-
Notifications
You must be signed in to change notification settings - Fork 862
[euiScreenReaderOnly] Use clip to fix scrolling issues caused by absolute positioning
#5130
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
Conversation
- this prevents issues with absolute positioning and scrolling containers, and works on all modern browsers supported by EUI
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5130/ |
cchaos
left a comment
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 love the research that you put into this and specifically trust the A11y project's solution. And definitely a good one to share in the newsletter since I've seen some "hacks" in Kibana around this issue.
I only tested the Accessibility page (in Chrome, FF & Safari). 👍
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5130/ |
…ed by absolute positioning (elastic#5130)" This reverts commit bca85fb.
Summary
Problem
@qn895 reported an issue with their usage of
EuiInMemoryTablewithin a scrolling container causing strange scrolling on the rest of their page/body. I was able to reproduce this behavior in our own docs page as well as in a CodeSandbox:Notice the page scrollbar jumping/increasing every time the table rows (and consequently table height) increases.
CSS-tricks put me on the right track of thinking it was a hidden absolutely positioned element that was causing the overflow issue. I also noticed when I had a table with plain text and no external link, I didn't have the same scroll issues. Well, I put 2 and 2 together, and it turns out it's the
.euiScreenReaderOnlypart of the table content that's causing the scroll/overflow shenanigans. In above screencap, the external link icon has SR text to indicate the link opens in a new window, and in the table @qn895 had issues with, the actions column icons have SR text to indicate their behavior.Reproducing
You can check out this commit on my fork to see the issue locally on http://localhost:8030/#/tabular-content/in-memory-tables. You can also cherry-pick that commit to this fixed branch to see the fix.
Solution
Adding to
clipprevents the scrolling shenanigans mentioned above, and works on all modern browsers supported by EUI.This should be fairly safe change in terms of browser support - additionally, WebAIM, WordPress, 18f, and a11yproject all have snippets using
clip.Some fun asides I stumbled upon during testing:
clipis apparently going to be deprecated, which is why I includedclip-path(per the above links)clip-pathdidn't solve the above scroll issue by itself - onlyclipdidtopandleftpositioning properties totally and the fix worked. However, on Chromium/webkit, justclipalone didn't fix the scrolling issue: I neededleft: -10000pxtoo 😖 The only other place I could find where someone else had this issue was this filed bug, but they seemed to come to the conclusion the issue was with FontAwesome, not Chromium (which I sorta disagree with, but c'est la vie). Maybe someday someone will google around and come across this issue, and the cycle of the internet will be complete :)QA
@include euiScreenReaderOnlymixin still work as expected:Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for the any docs examples- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately