-
Notifications
You must be signed in to change notification settings - Fork 138
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(APIKeyModal): improve screen reader announcement #6481
base: main
Are you sure you want to change the base?
fix(APIKeyModal): improve screen reader announcement #6481
Conversation
✅ Deploy Preview for ibm-products-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6481 +/- ##
==========================================
+ Coverage 79.85% 79.86% +0.01%
==========================================
Files 394 394
Lines 12877 12886 +9
Branches 4258 4263 +5
==========================================
+ Hits 10283 10292 +9
Misses 2594 2594
|
@matthewgallo , @oliviaflory, |
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.
Before | After |
---|---|
So I see the design changes and understand why we want a design opinion on them. Can you and @davidmenendez provide more clarity on what you tried before changing the design (1. move to helper text, 2. repeated success message)?
I would have expected us to use a combination of aria-live
and aria-relevant
1. I do think it’d be nice if we’re able to prevent redundancy in the pattern rather than repeating the success message twice.
Footnotes
-
aria-live
andaria-relevant
↩
Is it typical for the heading or message to change in the modal and the screen reader not announce it? In that case would we want to keep the heading the same and then include a success message at the bottom? I agree with Elysia that avoiding the duplicate message would be nice and I'd be interested to hear if there were alternative solutions. If it does require a design change I wonder if it should use the success status that the file uploader uses? I'm also curious what this "more content menu" is for? Is this unique to voice over on mac? I don't have JAWS so a little difficult for me to compare! When I open it it just repeats the helper text which seems even more repetitive than the double success messaging. (Sorry the video is long, start at 0:13 seconds in) generate-api-key.mov |
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.
waiting for design feedback & approval.
lets bring this up with michael gower at the next carbon accessibility guild 👍 |
Hey all, thanks for reviewing! Regarding Issue 4 to 6 in the description: Then I tried to show the succesMessage at the bottom just like in edit flow and error flow. But then it will duplicating the successMessage by displaying it both in the title and at the bottom of the modal. So for now as I mentioned in the Engineering huddle, the alternative solution I proposed is to keep the Issue 2 in the description: Regarding the helper text, it was previously being passed as the body of As we discussed in call, lets bring this up with Michael Gower for more clarity! @oliviaflory The "more content menu" you mentioned is part of the VoiceOver narration, we can see the same behaviour for Carbon TextInput aswell. Proposed Solution: Available in deployment preview also API.Key.Modal.movDemo of title announcement by jaws when APIKeyModal_readingTitle.mov |
Thank you @davidmenendez for discussing this with Michael Gower! I updated the style of status message with a |
Closes #6253 #6248
Improved the screen reader announcement.
Issue 1 : This issue is occur in PasswordInput component from Carbon, attaching the recording of PasswordInput.
Issue 2 : The information "This is your unique API key…" was changed from body of APIKeyDownloader to helperText of PasswordInput. Had a discussion with @davidmenendez on this.
Issue 3: Working as expected
Issue 4 to 6 : The success message is currently showing in heading, in that case it won't read success message immediately if we give aria-live or alert ( Also if we follow reading heading approach, in Custom Edit variant we have both Heading change & success message at a time so it will ignore reading heading since giving more importance to success message)
Had a discussion with @davidmenendez and Fix done : Added a successMessage below
Issue 7 : Divya Vani Bollayyagari from Raghu's team confirmed that it is working as expected now
Required design input on issue 2 and 4-6
Carbon's PasswordInput https://github.com/user-attachments/assets/163f4d12-e6d5-478b-a254-a7df3f6f907a
What did you change?
packages/ibm-products/src/components/APIKeyModal/APIKeyDownloader.js
packages/ibm-products/src/components/APIKeyModal/APIKeyModal.stories.jsx
packages/ibm-products/src/components/APIKeyModal/APIKeyModal.test.js
packages/ibm-products/src/components/APIKeyModal/APIKeyModal.tsx
packages/ibm-products/src/components/APIKeyModal/APIKeyModal.types.ts
How did you test and verify your work? storybook, Jaws