WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#24914 closed defect (bug) (fixed)

Twenty Fourteen: Space above the fixed bar in FF

Reported by: DrewAPicture Owned by: lancewillett
Milestone: 3.8 Priority: normal
Severity: normal Version: 3.8
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

Seems like we're getting some extra space above the fixed bar when scrolling in Firefox. It doesn't do it when in the Customizer, and seems OK in Chrome, so this may be localized to this browser.

Screencast for your viewing displeasure: http://screencast.com/t/9ywPneKvD

Attachments (3)

chrome.jpg (162.4 KB) - added by MikeHansenMe 2 years ago.
screenshot of space using chrome
24914.diff (364 bytes) - added by MikeHansenMe 2 years ago.
Patch to remove the space
24914.2.diff (411 bytes) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 @nacin2 years ago

  • Milestone changed from Awaiting Review to 3.8

comment:2 @lancewillett2 years ago

Thanks, this was bugging me at WCSF Contributor Day, glad you've tracked it for us.

comment:3 @obenland2 years ago

Have you tried it with MP6 activated?

comment:4 @MikeHansenMe2 years ago

  • Cc mdhansen@… added

I noticed this as well. It does not happen with MP6 activated.

@MikeHansenMe2 years ago

screenshot of space using chrome

@MikeHansenMe2 years ago

Patch to remove the space

comment:5 @MikeHansenMe2 years ago

This problem seems to affect more than just chrome. The patch I submitted fixes the problem for me on FF23 and Chromium28 on Ubuntu. This patch will need to be tested because the problem does not seem to affect all browser OS combos.

comment:6 @obenland2 years ago

To explain the difference: In MP6 the toolbar is 4px taller than in pre-MP6 and Further was optimized for MP6.

comment:7 @MikeHansenMe2 years ago

So it seems we need to wait this one out to see if MP6 makes it into core in 3.8. If not I think patching it would be a good idea so it will look right with default core toolbar.

comment:8 @markoheijnen2 years ago

We can easily apply the patch and add this for now:
.admin-bar.mp6 #masthead.masthead-fixed { top:32px; }

comment:9 @MikeHansenMe2 years ago

  • Keywords has-patch added; needs-patch removed

comment:10 follow-up: @georgestephanis2 years ago

Honestly, as the plan is for them to ship together, I think we should just test them together. If MP6 drops (unlikely) then we can re-address it.

comment:11 in reply to: ↑ 10 @nacin2 years ago

Replying to georgestephanis:

Honestly, as the plan is for them to ship together, I think we should just test them together. If MP6 drops (unlikely) then we can re-address it.

Replying to markoheijnen:

We can easily apply the patch and add this for now:
.admin-bar.mp6 #masthead.masthead-fixed { top:32px; }

I prefer the latter approach.

comment:12 @georgestephanis2 years ago

That being said, we should account for the 12px gap between the 782px breakpoint where the MP6 header goes to 46px high http://plugins.trac.wordpress.org/browser/mp6/trunk/components/responsive/css/admin-bar.css#L15, and the 770px breakpoint where the nav stops being fixed http://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentyfourteen/js/theme.js#L95

Version 0, edited 2 years ago by georgestephanis (next)

comment:13 @obenland2 years ago

If we're backwards compatible with 3.6 (I assume we are) we'd have to accommodate for both toolbars, MP6 and non-MP6.

@SergeyBiryukov2 years ago

comment:14 @lancewillett2 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 25063:

Twenty Fourteen: ensure toolbar doesn't have a gap at the top when you start scrolling, including back compat for 3.6. Props MikeHansenMe and SergeyBiryukov, fixes #24914.

Note: See TracTickets for help on using tickets.