-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Overlapping editor notifications over admin submenus. #1630
Conversation
@@ -13,8 +13,8 @@ $z-layers: ( | |||
'.editor-post-visibility__dialog': 30, | |||
'.editor-post-schedule__dialog': 30, | |||
'.editor-block-mover': 30, | |||
'.components-notice-list': 100000, | |||
'.components-popover': 100000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think popovers should show up over admin submenus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so - the notice is part of the Gutenberg UI, not something that should block you from using other wp-admin functions.
The design of traditional WP notices is very different (inline with the page, rather than popovers) but they don't suffer from this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree about the notices, I was talking about the popovers (They can be full width and have a grayed background to cover the rest of the page)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call this a modal. The gray background should prohibit expanding the admin menus in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if you open the inserter (which is a popover on a small screen), Should the inserter show up above or under the admin bar? I think it should be above
'.components-notice-list': 100000, | ||
'.components-popover': 100000, | ||
'.components-notice-list': 9989, | ||
'.components-popover': 9989, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 9989
? How high does this value actually need to be in order to make it work, and why? Let's avoid unnecessarily high values here, and if a specific value is needed, add a comment describing why.
See previous discussion about z-index
values on #637 - these are more complicated than many people realize.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9989
was the exact value for the notification to fall exactly under the dropdown admin navigation menu, nothing more than that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if a specific value is needed, add a comment describing why.
Something like this:
// Admin submenus (.wp-some-selector) have z-indez 9990
Added a comment indicating what z-index is used by admin submenus (including their selectors). |
Continuing this one at #1884. |
As shown in the above screenshot, I think that having notifications to overlap admin submenus is bad user experience, since that would force the user to close the notification first, before being able to access the submenu content.
Bringing down the
z-index
value of the.components-notice-list
and.components-popover
to9989
should fix this issue. On the other hand I don't know if the previous value of100000
was there for a reason or just a randomly "high" number and this modification would break something else in the UI.