-
Notifications
You must be signed in to change notification settings - Fork 918
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 Animation button accessibility issues (fixes #15762, #15763, #15764) #15776
base: main
Are you sure you want to change the base?
Fix Animation button accessibility issues (fixes #15762, #15763, #15764) #15776
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15776 +/- ##
==========================================
+ Coverage 79.06% 79.07% +0.01%
==========================================
Files 160 160
Lines 8355 8359 +4
==========================================
+ Hits 6606 6610 +4
Misses 1749 1749 ☔ View full report in Codecov by Sentry. |
media/css/m24/flag.scss
Outdated
|
||
// Focused control is not hidden under sticky nav: https://bugzilla.mozilla.org/show_bug.cgi?id=1936862 | ||
&:focus { | ||
scroll-margin-top: var(--sticky-nav-height); |
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.
scroll margin experiment, don't think I'm going to use it, only seems to work with no banner and it doesn't seem worth adding up the nav & occasional banner heights
I think that leaves me with a z-index override option? open to other ideas
769fce1
to
99bde00
Compare
@@ -63,6 +69,16 @@ | |||
&:active { | |||
background-color: $m24-color-dark-mid-gray; | |||
} | |||
|
|||
// High Contrast Mode Support: https://bugzilla.mozilla.org/show_bug.cgi?id=1936865 | |||
@media (forced-colors) { |
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.
we could also make these changes site-wide for buttons in forced-colors
One-line summary
As discussed with Tess, we'd like to keep the text visible on smaller screens to provide toggle animation button label
The above decision also helps make the label more visible for Windows High Contrast Mode
We also want to ensure the button is not overlapped by the sticky nav when focused
Significant changes and points to review
Other approaches considered for icon-only toggle animation button label:
Overall, the visible text label felt simplest and most accessible
Other approaches considered for focused button visibility:
Issue / Bugzilla link
#15762
#15763
#15764
Testing
Go to http://localhost:8000/en-US/ and check animation toggle button in hero section
Text label is visible when large screen is zoomed to 200%
Text label is visible when on smaller tablet/mobile screens
Test in Windows High Contrast Mode to confirm button focus color is correct and border color is visible
(Polypane testing with forced colors)
Focused & not focused
Focused button above sticky nav
Not-focused button below sticky nav