-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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: Spinbutton aria-valuenow vs native value update timing fix #33923
base: master
Are you sure you want to change the base?
Conversation
Pull request demo site: URL |
📊 Bundle size reportUnchanged fixtures
|
@@ -0,0 +1,7 @@ | |||
{ |
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.
🕵🏾♀️ visual regressions to review in the fluentuiv9 Visual Regression Report
Drawer 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Drawer.overlay drawer full - High Contrast.chromium.png | 2220 | Changed |
SpinButton Converged 92 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
SpinButton Converged.Appearance- filled-darker - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - Dark Mode.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - High Contrast.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker - RTL.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-darker.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - Dark Mode.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - High Contrast.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter - RTL.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- filled-lighter.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - Dark Mode.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - High Contrast.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default) - RTL.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default).focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default).mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- outline (default).mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - Dark Mode.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - High Contrast.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline - RTL.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline.focused.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Appearance- underline.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Max Bound.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - Dark Mode.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - High Contrast.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound - RTL.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound.focused.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.At Min Bound.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - Dark Mode.focused.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - Dark Mode.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - Dark Mode.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - High Contrast.focused.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - High Contrast.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - High Contrast.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - RTL.focused.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - RTL.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value - RTL.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value.focused.chromium.png | 0 | Removed |
SpinButton Converged.Display Value.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Display Value.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- filled-darker.focused.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- filled-darker.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- filled-darker.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- filled-lighter.focused.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- filled-lighter.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- filled-lighter.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- outline.focused.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- outline.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- outline.mouseDownIncrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- underline.focused.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- underline.mouseDownDecrement.chromium.png | 0 | Removed |
SpinButton Converged.Invalid- underline.mouseDownIncrement.chromium.png | 0 | Removed |
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.
Hey @smhigley , do you know why those screenshots are removed?
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.
I'm guessing it was just a screenshot tool issue, I'll update the branch and see if it fixes just by running again
6fc6550
to
c77e7cb
Compare
Previous Behavior
The basic problem is that we use
<input type="text" role="spinbutton">
, which creates tension between the native inputvalue
and the spinbutton role's expectation foraria-valuenow
. Some screen readers will usevalue
, and others will usearia-valuenow
. However, if we setaria-valuenow
with React, the timing of the update is one cycle after the native input/change happens. The result is that some screen readers (Narrator and VoiceOver) will read the previous value instead of the new value on value changes.I previously removed the default
aria-valuetext
(which is similar tovaluenow
for the purposes of this bug) to avoid having two separate sources of providing the value, but both VO and Narrator will not use the nativevalue
to calculate a spinbutton value, and will default to saying 0.New Behavior
To get around the timing problem that comes with using DOM attribute updates to provide
aria-valuenow
, I instead updated our code to set the IDL attribute directly (i.e.el.ariaValueNow = newValue
).Testing with VoiceOver, Narrator, NVDA, and JAWS, this seems to now work as expected.
Related Issue(s)