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
♿ [Story a11y] Interactive components options to buttons #32957
Conversation
Hey @gmajoulet! These files were changed:
|
this.rootEl_.querySelectorAll('button, a').forEach((el) => { | ||
el.setAttribute('tabindex', toggle ? 0 : -1); | ||
// Disable tabbing through options if already selected. | ||
if (this.getOptionElements().includes(el) && this.hasUserSelection_) { |
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 do you need to check if it's an option element? What else could it be?
If you really need to check that, let's test for a class or something. Doing a querySelector + a loop within a loop should be improved
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.
The disclaimer icon + learn more link + close button are also buttons, and they should remain tabbable after the user has answered (so they need to be selected). Testing for a class SG
@@ -187,6 +190,12 @@ | |||
line-height: 1.25em !important; | |||
} | |||
|
|||
.i-amphtml-story-interactive-active:not(.i-amphtml-story-interactive-post-selection) | |||
.i-amphtml-story-interactive-quiz-option:focus::before { | |||
left: 0px !important; |
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.
This looks random?
Also, RTL support?
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.
Actually I can fix this on another place. Seems like if you don't set left: 0px
on an absolute element, it lines up with the padding of the parent so it wasn't being aligned with the button.
RTL support is the same since the width is 100%, it's the same to do left: 0
or right: 0
@@ -181,12 +184,18 @@ | |||
.i-amphtml-story-interactive-quiz-option::before { | |||
content: '' !important; | |||
position: absolute !important; | |||
left: 0px !important; |
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.
@gmajoulet this should have left: 0px
always since it's the hint element that needs to be aligned with the parent at all times
This is great Matias and covers essential a11y of the buttons nicely. I wonder how we could give users feedback on interactives that don't have percentages. |
Do you mean to announce when the option selected is correct or not on quizzes? The other "updates" don't seem to be that useful to announce, like the results component can be read when you get to the page but we wouldn't want to announce it every time that it gets updated |
Oh I can fix talkback in a followup PR since it's a bit complex |
Contributes to #32751
Make interactive options tabbable by using buttons instead of divs and updating the focus state of the buttons with the shade color (white background gets darker and dark background gets lighter, similar to the hinting animation).
Also setting
aria-live="polite"
on buttons so they read the percentages when they get added to the DOM (on user selection)Focus states: