Update ColorPicker to announce changes to RGBA fields, even when field is not focused #15855
Conversation
msft-github-bot
added
the
needs cherry-pick
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 5ea01a0:
|
Perf AnalysisNo significant results to display. All results
|
Asset size changes
Baseline commit: fef6e99987f44a41ee46faf1db217d9c6d44109a (build) |
|
Oh shoot, didn't update the snapshots, will need to look into that. Let me know if you have any feedback on the code/approach. |
packages/office-ui-fabric-react/src/components/ColorPicker/ColorPicker.base.tsx
Outdated
Show resolved
Hide resolved
packages/office-ui-fabric-react/src/components/ColorPicker/ColorPicker.base.tsx
Outdated
Show resolved
Hide resolved
packages/office-ui-fabric-react/src/components/ColorPicker/ColorPicker.base.tsx
Outdated
Show resolved
Hide resolved
…orPicker.base.tsx Applying PR suggestion Co-authored-by: Elizabeth Craig <ecraig12345@gmail.com>
|
Updated to remove hex field from set of fields, originally thought it should be included, but now realize that it's duplicative information since the RGBA fields are enough to understand what the color is. Also screen readers don't do a great job reading the hex field. @ecraig12345 are you able to review this soon? Folks are waiting on this fix |
| @@ -0,0 +1,8 @@ | |||
| { | |||
| "type": "minor", | |||
There was a problem hiding this comment.
Not super important but this is probably patch since it's not an API change.
| "type": "minor", | |
| "type": "patch", |
There was a problem hiding this comment.
Let's keep it minor since it's a DOM structure addition
There was a problem hiding this comment.
I wouldn't generally say adding an attribute qualifies as a structure change (that's more about adding/removing/moving elements). But if you'd still rather keep it as a minor change that's fine.
There was a problem hiding this comment.
Sorry, yes you are correct on the terminology. It's not so much DOM structure as a general DOM change.
I guess the way I see it is that since aspire to treat DOM attribute removal as a major, we should treat DOM attribute addition as a minor. It's really a stretch, but also it seems better to be conservative on patch vs. minor.
Good to discuss these points though.
JustSlone
changed the title
|
🎉 Handy links: |
Update ColorPicker to announce clamping of RGBA values When a user types an invalid number in an RGBA field in ColorPicker then changes focus from that field ColorPicker will update the field to a valid value. However, this change was not being announced by screen readers, which is an issue as a non-sighted user is unable to know about the new value. This change adds aria-live="assertive" to announce the change to the user even when they are not focused on the field. Co-authored-by: Justin Slone <justin.slone@microsoft.com>
ecraig12345
removed
the
needs cherry-pick
…15855) (#16709) * Update ColorPicker to announce clamping of RGBA values (#15855) Update ColorPicker to announce clamping of RGBA values When a user types an invalid number in an RGBA field in ColorPicker then changes focus from that field ColorPicker will update the field to a valid value. However, this change was not being announced by screen readers, which is an issue as a non-sighted user is unable to know about the new value. This change adds aria-live="assertive" to announce the change to the user even when they are not focused on the field. Co-authored-by: Justin Slone <justin.slone@microsoft.com>
5 participants
Pull request checklist
$ yarn changeDescription of changes
Thanks to an suggestion from someone more familiar with this space, found a much better fix.
Simply applying aria-live="assertive" to the hex and RGBA fields results in the screenreader announcing any changes made to these fields.
Choices are polite or assertive. I choose assertive as it works a bit better with the flow, it still announces the change made, but then immediately announces the other changes. Also other ColorPickers use aria-live="assertive".
The core issue here is that when a user enters a value > 255 (or >100) in the RGBA fields of ColorPicker, screen readers make it seem like the invalid value was entered successfully, however when the user moves to the next field ColorPicker resets the value to 255, without announcing to the user.This fix adds role="spinbutton" and the appropriate spinbutton aria properties to the RGBA fields. The aria-valuenow attribute is updated with the correct clamped value resulting in the screen reader announcing the proper value even if the edit box is currently displaying an invalid value.Focus areas to test
ColorPicker with screen reader, need to try multiple incorrect values is in the RGBA textfields