Skip to content
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

Fix/18582 z index for snackbar #18690

Closed
wants to merge 2 commits into from
Closed

Fix/18582 z index for snackbar #18690

wants to merge 2 commits into from

Conversation

ajotka
Copy link
Contributor

@ajotka ajotka commented Nov 22, 2019

Description

Fix #18582 for Snackbar notices. It was showing up under the footer and under the horizontal scrollbar.

How has this been tested?

Tested locally, by manual tests.
Wordpress 5.3.0.
Steps:
I:

  • Add normal block
  • Click Save
  • Snackbar is showing up properly.
    II:
  • Add block which is more than 1920px
  • Click save
  • Snackbar is showing up properly.

Screenshots

Zrzut ekranu z 2019-11-22 14-33-25

Types of changes

  • add z-index 35 for editor layout in z-index file
  • add overflow-x: hidden for editor layout

Checklist:

  • [+] My code is tested.
  • [+] My code follows the WordPress code style.
  • [+] My code follows the accessibility standards.
  • [+] RWD checked.

@ajotka ajotka requested a review from talldan as a code owner November 22, 2019 13:43
@gziolo gziolo added the General Interface Parts of the UI which don't fall neatly under other labels. label Nov 22, 2019
@gziolo gziolo requested a review from youknowriad November 22, 2019 16:27
@gziolo gziolo added the [Type] Bug An existing feature does not function as intended label Nov 22, 2019
@gziolo gziolo requested a review from jasmussen November 26, 2019 11:44
Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the pull request 🎉

While it does seem to address the issue specifically, I left some comments to suggest we might need a different approach. Best I can tell the way you solved it here was to forcibly remove the horizontal scrollbar and change the z-index to be above any such scrollbars. But while I agree horizontal scrollbars are the worst (:D) every once in a while they are intentional, so we can't hide those.

All the snackbars are inside a container though, that you may be able to target instead, such as components-snackbar-list.

Thanks again for the PR!

@@ -24,6 +24,7 @@ $z-layers: (
".block-editor-warning": 5,
".block-library-gallery-item__inline-menu": 20,
".block-editor-url-input__suggestions": 30,
".edit-post-layout__content": 35,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the goal is to fix the snackbar position, I would consider changing the z-index of components-snackbar-list rather than the content container itself. It might have adverse effects otherwise.

Unfortunately this particular classname also changed in a recent refactor in #18044, so it no longer exists in master.

@@ -12,6 +12,11 @@
}
}


.edit-post-layout__content {
overflow-x: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add this. While we haven't seen it yet, there may be cases where you need to be able to scroll horizontally in the layout content. Most of the time the horizontal scrollbar is helpful to discover accidental horizontal scrollbars, i.e. when content is too wide, but you could even imagine a horizontally scrolling theme! :) https://wordpress.com/theme/shelf

@youknowriad
Copy link
Contributor

I think this is already fixed in master, right?

Base automatically changed from master to trunk March 1, 2021 15:42
@paaljoachim
Copy link
Contributor

I am closing this PR. As the issue has been fixed.

@gwwar
Copy link
Contributor

gwwar commented Apr 29, 2021

Thanks @ajotka for giving this a try!

In trunk I see the footer set to z-index 90 and the snackbar container set to 100000, with the same z-index stacking context so we should be good with current behavior

Screen Shot 2021-04-29 at 11 36 46 AM

Screen Shot 2021-04-29 at 11 37 01 AM

@ajotka
Copy link
Contributor Author

ajotka commented Apr 30, 2021

This PR is from 2019 and you fixed it in different branch a long time ago, but never closed it ;) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snackbar notices showing up under the editor footer (z-index)
6 participants