Remove aria attributes in order to read card body content #90
Conversation
jomcarvajal
requested review from
bethshook and
jivey
and removed request for
a team
| @@ -103,7 +103,7 @@ export const AnswersTable = (props: AnswersTableProps) => { | |||
| }); | |||
|
|
|||
| return ( | |||
| <div role="radiogroup" aria-label="Answer choices" className="answers-table"> | |||
There was a problem hiding this comment.
@jivey using NVDA I noticed that Answer choices appears always, no matter the content. Removing this aria attributes NVDA is capable to read inside card's content. I didn't see any other a11y issue after removing role="radiogroup" but let me know if you're fine with this approach
There was a problem hiding this comment.
The label and role here were recently added for accessibility so they probably need to stay here... can you share a screenshot of the NVDA speech that you are seeing? When I test with focus first off, I get this speech which is reading the options:
Answer choices grouping radio button unavailable not checked clickable A Option 1 radio button unavailable not checked clickable B Option 2 radio button unavailable not checked clickable C Option 3 radio button unavailable not checked clickable D Option 4
Additionally, I'm seeing we have a new issue here I didn't see before: those are being read as clickable, can we disable them while in preview mode?
There was a problem hiding this comment.
Using VoiceOver with the label and role I get this:
There was a problem hiding this comment.
Ah okay yes that seems like a problem, at least for our purposes. It's a newer change that I'm not familiar with: #78 My initial thought is that an aria label shouldn't be on that, as it has a role, but I'm not sure. Can you look into that? And if you determine the label needs to be there, we may have to make it conditional for preview cards...
There was a problem hiding this comment.
For some reason role is removing Answers from screen reader. I removed the label just to test but Answers still missing as you can see in my previous screenshot. @TylerZeroMaster maybe you can help us to understand this a11y changes. Are we expecting that answers from a card only appears as "Answer choices" in screen readers?
There was a problem hiding this comment.
@jomcarvajal I think the aria-label and role can be removed from the AnswersTable if all the radio buttons have the same name. When they have the same name, they should implicitly be in a radiogroup.
When using HTML's native input radio button, , the radio buttons are grouped when each of input radio buttons in the group is given the same name.
I may have followed the original issue, which suggested adding the radiogroup role, too closely when I implemented those changes. After further reading:
It is recommended to create radio groups by using same-named HTML input radio buttons, but, if you must use ARIA roles and attributes instead of semantic HTML form controls, custom radio buttons can and should act like native HTML radio input buttons.
So if it works better to create an implicit radiogroup by using same-named radio buttons, then I am all for that solution.
|
@jivey @TylerZeroMaster Let's keep it disabled just for preview cards for now |
|
@jomcarvajal The conditional look good, thanks for working on this. Before I approve can we make one more change, I see |
|
@jivey I already added |
I forget to pass the previewMode prop my bad |
* card styles * Overlay option added in exercise component and new component IncludeRemoveQuestion created (#86) * overlay option added in exercise component and new component includeremovequestion created * resolve comments * update snapshots * move overlay logic into Card component * remove focus on card when overlay is up and add autoFocus for first interactive element on overlay * Add export for IncludRemoveQuestion (#87) add export of IncludRemoveQuestion * Modify preview-card styles (#88) * modify preview card styles * change color of card when is included * resolve comments - fix style issues * change props in storybook of Exercise * Add prop method for details button * Refactor props of ExercisePreview * export ExercisePreview * remove focus first for screenreaders, style tweaks * Remove aria attributes in order to read card body content (#90) * remove aria attributes in order to read card body content * conditional aria attributes for cardbody * previewMode in storybook * missing previewMode prop * fix some type issues * disable eslint rule for ts-ignore just for this case * Core 675 refactor exercise preview (#92) * remove includeRemove logic from preview exercise card * print stories deleted by mistake * test description fixed * added new story for preview exercise, hide all feedback propertly * revert print delete * remove questionState prop, added feedback inside answers for stories purposes * is_completed added into ExercisePreview component * update snapshots * New prop showCorrectAnswer (#93) * new prop showCorrectAnswer * fix exercise stories * ExercisePreview now shows question detailed (#94) * ExercisePreview now shows question detailed * Restore print * hide footer with new condition * Restore print * fix indentation in ExerciseQuestion * set answer_id as empty * set new margin for preview card * update snaps * resolve test duplicity --------- Co-authored-by: jomcarvajal <[email protected]> Co-authored-by: jomcarvajal <[email protected]>
With NVDA, the card content wasn’t announced. The focus going to the include button causes that to be read, instead of reading “Question 1: option A, option B” etc.
Solution: remove role and aria-label from card content to allow Screen Reader to watch card's content.