WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#30900 closed defect (bug) (fixed)

Admin menu is flickering when scrolling left or right

Reported by: mindrun Owned by: azaozz
Milestone: 4.2 Priority: low
Severity: normal Version: 4.1
Component: Administration Keywords: has-patch dev-feedback commit
Focuses: ui, administration Cc:

Description

If you open WordPress' backend in the latest Safari version (8.0.2) on OS X you'll notice that the admin menu looks a bit buggy if you're trying to scroll left or right. So I started digging into the CSS code and found out that this is caused by some attributes on the #adminmenuback-element.

Currently, the element has the following attributes:

#adminmenuback {
     position: absolute;
     top: 0;
     bottom: 0;
     z-index: -1;
}

But If we would change the position to 'fixed' (like #adminmenuwrap already has it) and the z-index to '9980', the menu won't look buggy anymore if someone scrolls to the left or right. - I also tested the element's new behavior on mobile and in different browsers like Chrome, Opera and Firefox and everything looks fine.

Therefore this will be the new declaration for the named element:

#adminmenuback {
     position: fixed;
     top: 0;
     bottom: 0;
     z-index: 9989;
}

I would very much welcome it if someone could look after this problem. Thanks in advance!

Attachments (6)

30900.diff (368 bytes) - added by joemcgill 5 years ago.
old-z-index.mp4 (1.8 MB) - added by mindrun 5 years ago.
Quick shot without the new z-index property
30900.2.diff (367 bytes) - added by joemcgill 5 years ago.
added z-index
screen capture.mov (6.9 MB) - added by mindrun 4 years ago.
Bug still present
30900.3.diff (457 bytes) - added by joemcgill 4 years ago.
Add overflow to bottom of #adminmenuback to account for iOS overscroll
30900.4.diff (425 bytes) - added by helen 4 years ago.

Change History (36)

@joemcgill
5 years ago

#1 follow-up: @joemcgill
5 years ago

  • Component changed from Menus to Administration
  • Keywords has-patch added

Confirmed that this is a bug and changing the position property from absolute to fixed fixes the issue. There's no need, as far as I can tell, to change the z-index for this to work, so I've left that alone in 30900.diff.

#2 in reply to: ↑ 1 ; follow-ups: @mindrun
5 years ago

Replying to joemcgill:

Confirmed that this is a bug and changing the position property from absolute to fixed fixes the issue. There's no need, as far as I can tell, to change the z-index for this to work, so I've left that alone in 30900.diff.

Oh, sorry. I forgot to explain, why I also changed the z-index property to 9980. This change is required to avoid that the content is pushed between #adminmenuback and #adminmenuwrap if the user scrolls to the right.

I've attached another quick shot without the new z-index.

Last edited 5 years ago by mindrun (previous) (diff)

@mindrun
5 years ago

Quick shot without the new z-index property

@joemcgill
5 years ago

added z-index

#3 in reply to: ↑ 2 @joemcgill
5 years ago

Replying to mindrun:

Oh, sorry. I forgot to explain, why I also changed the z-index property to 9980. This change is required to avoid that the content is pushed between #adminmenuback and #adminmenuwrap if the user scrolls to the right.

Thanks for following up. I thought I had checked for that, but guess I didn't scroll far enough to see it. 30900.2.diff updates the needed z-index property accordingly.

#4 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 4.2

#5 follow-ups: @DrewAPicture
4 years ago

  • Keywords reporter-feedback added

I can't seem to reproduce this problem in Safari 8.0.3. Can you please reconfirm this bug is still present?

30900.2.diff still applies.

@mindrun
4 years ago

Bug still present

#6 in reply to: ↑ 5 @mindrun
4 years ago

Replying to DrewAPicture:

I can't seem to reproduce this problem in Safari 8.0.3. Can you please reconfirm this bug is still present?

30900.2.diff still applies.

That's curious. To me, it's still present (see latest attachment). I had also cleared the cache and updated to 4.1.1 ...

#7 in reply to: ↑ 5 @joemcgill
4 years ago

Replying to DrewAPicture:

I can't seem to reproduce this problem in Safari 8.0.3. Can you please reconfirm this bug is still present?

30900.2.diff still applies.

I can also confirm that this is still an issue running 4.2-alpha-31484.

#8 @mindrun
4 years ago

  • Keywords reporter-feedback removed

#9 follow-up: @azaozz
4 years ago

  • Keywords needs-testing added

Needs testing to confirm that #adminmenuback { position: fixed; } doesn't break in iOS Safari when scrolling and the keyboard is open.

#10 in reply to: ↑ 9 @joemcgill
4 years ago

Replying to azaozz:

Needs testing to confirm that #adminmenuback { position: fixed; } doesn't break in iOS Safari when scrolling and the keyboard is open.

Thanks for the question about mobile Safari. I tested the patch in iOS Safari and didn't notice any issues. In this case, I'm fairly certain we're safe because the position: fixed and z-index rules are both overridden by the media query rules (max-width: 782px) found in admin-menu.css, line 757. Also, the #adminmenuback component is set to display:none; on any viewport that is smaller than 600px wide (line 912).

Maybe @helen could give additional insight into whether the use of absolute vs. fixed positioning was intentional here.

This ticket was mentioned in IRC in #wordpress-dev by skbly7. View the logs.


4 years ago

#12 follow-up: @helen
4 years ago

I'm not too sure about the history of the CSS here - there's a looooot of stuff and cruft. The only thing that I would note is that there's some JS that changes the "stickiness" of the admin menu based on window height + menu height.

#13 in reply to: ↑ 12 @joemcgill
4 years ago

Replying to helen:

I'm not too sure about the history of the CSS here - there's a looooot of stuff and cruft. The only thing that I would note is that there's some JS that changes the "stickiness" of the admin menu based on window height + menu height.

Tested this again for any weirdness that might happen when JS changes the positioning of #adminmenuwrap from absolute to fixed when the viewport height is shorter than the menu height and everything looks clean. I'm fairly confident that this is ready to go in.

Last edited 4 years ago by joemcgill (previous) (diff)

#14 @samuelsidler
4 years ago

  • Priority changed from normal to low

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


4 years ago

#16 @DrewAPicture
4 years ago

  • Owner set to azaozz
  • Status changed from new to reviewing

#17 follow-up: @helen
4 years ago

  • Keywords needs-testing removed

I noticed something a little wonky with the patch on an iPad in landscape, uncollapsed and visible menu - it doesn't repaint the background of the wrap until the scrolling is complete, so it momentarily has no background at the bottom and then it pops in. Everything else in both desktop and mobile Safari looked fine, I'm just not sure of the balance of users between a touch device that's wide enough to have the expanded admin menu (not sure if the repaint issue appears other than in mobile Safari, my Nexus 7 isn't wide enough) and desktop Safari. Any thoughts here?

@joemcgill
4 years ago

Add overflow to bottom of #adminmenuback to account for iOS overscroll

#18 in reply to: ↑ 17 @joemcgill
4 years ago

Replying to helen:

Good catch there. iOS overscrolling makes the frame container act strange here. 30900.3.diff adds a 30px overflow to the bottom of the #adminmenuback div to account for the overscroll, which seems to do the trick.

This ticket was mentioned in Slack in #core by drew. View the logs.


4 years ago

#20 @azaozz
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 32089:

Fix flickering of the admin menu on over-scrolling.
Props joemcgill, mindrun. Fixes #30900.

#21 @nerrad
4 years ago

Note, this change resulted in #31941

#22 in reply to: ↑ 2 ; follow-up: @azaozz
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to mindrun:

I forgot to explain, why I also changed the z-index property to 9980. This change is required to avoid that the content is pushed between #adminmenuback and #adminmenuwrap if the user scrolls to the right.

Don't see that happening if the z-index is removed. Tested in Safari on MacOS 10.10.3 and iOS 8.3.

Going to remove the z-index for now. That will fix cases like #31941. Feel free to reopen if that z-index value is required.

#23 @azaozz
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 32099:

Remove z-index from #adminmenuback.
Fixes #30900.

#25 in reply to: ↑ 22 ; follow-up: @mindrun
4 years ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to azaozz:

Replying to mindrun:

I forgot to explain, why I also changed the z-index property to 9980. This change is required to avoid that the content is pushed between #adminmenuback and #adminmenuwrap if the user scrolls to the right.

Don't see that happening if the z-index is removed. Tested in Safari on MacOS 10.10.3 and iOS 8.3.

Going to remove the z-index for now. That will fix cases like #31941. Feel free to reopen if that z-index value is required.

Hmm... For me, the z-index is still required to avoid the fact that the editor slides between the two elements. I also tested this on a few other WordPress sites (all running on 4.1.1) with Safari 8.0.5 on the latest OS X.

Maybe we have different results because you don't have 'Enable full-height editor ...' enabled? Because if I turn this option off, it works fine (even without the z-index).

#26 in reply to: ↑ 25 @DrewAPicture
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to mindrun:

Replying to azaozz:

Replying to mindrun:

I forgot to explain, why I also changed the z-index property to 9980. This change is required to avoid that the content is pushed between #adminmenuback and #adminmenuwrap if the user scrolls to the right.

Don't see that happening if the z-index is removed. Tested in Safari on MacOS 10.10.3 and iOS 8.3.

Going to remove the z-index for now. That will fix cases like #31941. Feel free to reopen if that z-index value is required.

Hmm... For me, the z-index is still required to avoid the fact that the editor slides between the two elements. I also tested this on a few other WordPress sites (all running on 4.1.1) with Safari 8.0.5 on the latest OS X.

Maybe we have different results because you don't have 'Enable full-height editor ...' enabled? Because if I turn this option off, it works fine (even without the z-index).

The z-index was re-added in [32100].

This ticket was mentioned in Slack in #core by helen. View the logs.


4 years ago

#28 @helen
4 years ago

The high z-index also makes the admin menu disappear in IE7 on various screens, particularly post edit. It seems fine to me with #adminmenuback just having a positive z-index at all, rather than relating it to #adminmenuwrap.

@helen
4 years ago

#29 @azaozz
4 years ago

  • Keywords commit added
  • Resolution fixed deleted
  • Status changed from closed to reopened

30900.4.diff looks good here.

#30 @helen
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 32268:

Admin menu: prevent (most) lock outs caused by plugins or IE7.

fixes #30900.

Note: See TracTickets for help on using tickets.