-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Error messages don't block bottom of page clicks #12567
Conversation
The flash container was set to 100% width to center the messages on the screen. The messages were covering only part of the screen though. So the container beyond the actual message box was covering part of the page, blocking clicks on elements. A new way of centering the container with CSS translate means that the width of the container can be the same as the content, not covering anyting accidentally. And moving the whole container up instead of only moving the contained message allows us to interact with elements below the flash message as well.
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.
Great, thanks for solving this.
I haven't used translateX
much and wonder if we will lose the flexibility we had with flexbox (like trying to support stacking of multiple flash messages). But I don't think we support that yet anyways.
width: 100%; | ||
left: 50%; | ||
transform: translateX(-50%); | ||
bottom: 3.5rem; | ||
z-index: $flash-message-z-index; |
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 wonder if we could just move the z-index
rule to the child .flash
, so the container wouldn't cover anything?
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.
Maybe that could have been a solution, too.
By the way, I noticed that the flash content seems to be there twice. Once in the flash block and then in a notice block which is perfectly overlaying the first one. It doesn't really matter right now but I guess that we want to clean that up at some point.
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.
Oh dear, does that mean you have to click "dismiss" twice?
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.
Yes! I didn't think of that but just tested and you are right. You have to click twice. 🤦
That's another issue, low priority though.
@filipefurtad0 I'm not successful in reproducing the error 🙈 so I'm not sure how to test this one. Can you try on your side? |
Hi @mkllnk, I managed to reproduce the issue and test your solution. Positive
Negative
Both of the negative effects above are present in master already, so no regression here. I will open an issue for this. ConclusionIt's a clear improvement enabling the interaction with elements on the page. No regressions introduced by this PR. |
What? Why?
The flash container was set to 100% width to center the messages on the screen. The messages were covering only part of the screen though. So the container beyond the actual message box was covering part of the page, blocking clicks on elements.
A new way of centering the container with CSS translate means that the width of the container can be the same as the content, not covering anyting accidentally.
And moving the whole container up instead of only moving the contained message allows us to interact with elements below the flash message as well.
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates