Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smhigley
Copy link
Contributor

Previous Behavior

The basic problem is that we use <input type="text" role="spinbutton">, which creates tension between the native input value and the spinbutton role's expectation for aria-valuenow. Some screen readers will use value, and others will use aria-valuenow. However, if we set aria-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 to valuenow 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 native value 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)

@smhigley smhigley requested a review from a team as a code owner February 27, 2025 23:51
Copy link

Pull request demo site: URL

Copy link

github-actions bot commented Feb 28, 2025

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.174 MB
293.93 kB
1.174 MB
294.041 kB
654 B
111 B
react-spinbutton
SpinButton
34.965 kB
11.661 kB
35.221 kB
11.75 kB
256 B
89 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
69.638 kB
20.24 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
225.271 kB
65.211 kB
react-components
react-components: FluentProvider & webLightTheme
44.473 kB
14.597 kB
react-portal-compat
PortalCompatProvider
8.39 kB
2.64 kB
react-timepicker-compat
TimePicker
108.551 kB
36.094 kB
🤖 This report was generated against 5e9bdc096779a2c614a2b9f829621b48e1c59746

@@ -0,0 +1,7 @@
{
Copy link
Collaborator

@fabricteam fabricteam Feb 28, 2025

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

Copy link
Contributor

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?

Copy link
Contributor Author

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

@ValentinaKozlova ValentinaKozlova self-requested a review February 28, 2025 06:35
@smhigley smhigley force-pushed the spinbutton-value-update branch from 6fc6550 to c77e7cb Compare February 28, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants