fix: #4967 useNumberField mutates number so screenreader users won’t know that their value has changed #6520

majornista · 2024-06-10T23:16:40Z

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue #4967.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Improves behavior when pasting a value from a different locale into a NumberField.

A pasted string containing groups and a decimal, like "3 000 000.25", "3,000,000.25" or "3.000.000,25", will now attempt to parse using supported locales rather than only parsing using the current locale.

Where there is ambiguity, as with "1,000" or "1.000", the pasted value will be parsed using the current locale. For example, "1.000" will be interpreted as "1" in en-US, and "1,000" will be "1" in de-DE.

If the pasted text is different from the resulting input text, the screen reader should announce "Pasted value: {value}" politely, so that the user hears the new value.

🧢 Your Project:

Adobe/Accessibility

packages/@react-stately/numberfield/src/useNumberFieldState.ts

rspbot · 2024-06-11T17:42:22Z

Build successful! 🎉

packages/@react-stately/numberfield/src/useNumberFieldState.ts

rspbot · 2024-06-19T19:25:48Z

Build successful! 🎉

snowystinger

Not quite sure it's working, we should be able to thoroughly test this using fast-check
code taken mostly from 'NumberParser.test', I'm not positive I have it set up correctly, but right now it doesn't appear to be making the round trip. General idea is

we have a random number
we format the number to a string so we know it's valid
we send that string into the NumberField via paste
we should see an onChange with the value of the number we formatted earlier and we should see the input reflect the number as formatted our current locale


  // for some reason hu-HU isn't supported in jsdom/node
  let locales = Object.keys(messages).filter(locale => locale !== 'hu-HU');
  describe.only('round trips', function () {
    // Locales have to include: 'de-DE', 'ar-EG', 'fr-FR' and possibly others
    // But for the moment they are not properly supported
    const localesArb = fc.constantFrom(...locales);
    const styleOptsArb = fc.oneof(
      {withCrossShrink: true},
      fc.record({style: fc.constant('decimal')}),
      // 'percent' should be part of the possible options, but for the moment it fails for some tests
      fc.record({style: fc.constant('percent')}),
      fc.record(
        {style: fc.constant('currency'), currency: fc.constantFrom('USD', 'EUR', 'CNY', 'JPY'), currencyDisplay: fc.constantFrom('symbol', 'code', 'name')},
        {requiredKeys: ['style', 'currency']}
      ),
      fc.record(
        {style: fc.constant('unit'), unit: fc.constantFrom('inch', 'liter', 'kilometer-per-hour')},
        {requiredKeys: ['style', 'unit']}
      )
    );
    const genericOptsArb = fc.record({
      localeMatcher: fc.constantFrom('best fit', 'lookup'),
      unitDisplay: fc.constantFrom('narrow', 'short', 'long'),
      useGrouping: fc.boolean(),
      minimumIntegerDigits: fc.integer({min: 1, max: 21}),
      minimumFractionDigits: fc.integer({min: 0, max: 20}),
      maximumFractionDigits: fc.integer({min: 0, max: 20}),
      minimumSignificantDigits: fc.integer({min: 1, max: 21}),
      maximumSignificantDigits: fc.integer({min: 1, max: 21})
    }, {requiredKeys: []});

    // We restricted the set of possible values to avoid unwanted overflows to infinity and underflows to zero
    // and stay in the domain of legit values.
    const DOUBLE_MIN = Number.EPSILON;
    const valueArb = fc.tuple(
      fc.constantFrom(1, -1),
      fc.double({next: true, noNaN: true, min: DOUBLE_MIN, max: 1 / DOUBLE_MIN})
    ).map(([sign, value]) => sign * value);

    const inputsArb = fc.tuple(valueArb, localesArb, styleOptsArb, genericOptsArb)
      .map(([d, locale, styleOpts, genericOpts]) => ({d, opts: {...styleOpts, ...genericOpts}, locale}))
      .filter(({opts}) => opts.minimumFractionDigits === undefined || opts.maximumFractionDigits === undefined || opts.minimumFractionDigits <= opts.maximumFractionDigits)
      .filter(({opts}) => opts.minimumSignificantDigits === undefined || opts.maximumSignificantDigits === undefined || opts.minimumSignificantDigits <= opts.maximumSignificantDigits)
      .map(({d, opts, locale}) => {
        if (opts.style === 'percent') {
          opts.minimumFractionDigits = opts.minimumFractionDigits > 18 ? 18 : opts.minimumFractionDigits;
          opts.maximumFractionDigits = opts.maximumFractionDigits > 18 ? 18 : opts.maximumFractionDigits;
        }
        return {d, opts, locale};
      })
      .map(({d, opts, locale}) => {
        let adjustedNumberForFractions = d;
        if (Math.abs(d) < 1 && opts.minimumFractionDigits && opts.minimumFractionDigits > 1) {
          adjustedNumberForFractions = d * (10 ** (opts.minimumFractionDigits || 2));
        } else if (Math.abs(d) > 1 && opts.minimumFractionDigits && opts.minimumFractionDigits > 1) {
          adjustedNumberForFractions = d / (10 ** (opts.minimumFractionDigits || 2));
        }
        return {adjustedNumberForFractions, opts, locale};
      });

    it('should fully reverse NumberFormat', async function () {
      let onChange = jest.fn((val) => console.log(val));
      let {getByRole} = render(<TestNumberField onChange={onChange} />);
      await fc.assert(
        fc.asyncProperty(
          inputsArb,
          fc.scheduler({act}),
          async function ({adjustedNumberForFractions, locale, opts}, s) {
            s.scheduleSequence([{builder: async () => {
              const formatter = new Intl.NumberFormat(locale, opts);
              const parser = new NumberParser(locale, opts);

              const formattedOnce = formatter.format(adjustedNumberForFractions);
              let input = getByRole('textbox');
              act(() => {
                input.focus();
                input.setSelectionRange(0, input.value.length);
              });
              await userEvent.paste(formattedOnce);
            }, label: `entering ${adjustedNumberForFractions}`}]);
            // Assert
            while (s.count() !== 0) {
              await s.waitOne();
              console.log('mock call', onChange.mock.calls)
              expect(onChange).toHaveBeenLastCalledWith(adjustedNumberForFractions);
            }
          }
        ),
        {numRuns: 1000, timeout: 1000}
      );
    });

majornista · 2024-06-24T16:39:50Z

@snowystinger I think I may have fixed the NumberParser sanitize method so that should fully reverse NumberFormat test no longer needs to be skipped. Could you take a look?

rspbot · 2024-06-24T16:56:27Z

Build successful! 🎉

rspbot · 2024-06-24T19:16:46Z

Build successful! 🎉

snowystinger · 2024-06-26T08:16:46Z

Thanks, had a look, I think we still need to resolve a few things as I couldn't get the test I suggested in my comment above working. See https://github.com/adobe/react-spectrum/compare/Issue-4967-useNumberField-input-masking...numberfield-testing-and-questions?expand=1

rspbot · 2024-06-26T15:50:47Z

Build successful! 🎉

rspbot · 2024-06-26T17:59:44Z

Build successful! 🎉

rspbot · 2024-07-10T23:24:23Z

Build successful! 🎉

rspbot · 2024-07-10T23:24:32Z

Build successful! 🎉

rspbot · 2024-07-11T13:53:44Z

Build successful! 🎉

rspbot · 2024-07-15T14:46:29Z

Build successful! 🎉

majornista · 2024-07-15T16:57:42Z

It’s still not perfect, but it’s a bit better. It becomes difficult to round trip parsing values with units from other the locales, percent, currency (accounting), and negative values. Rather than fail the, I trace out any values that fail to match after parsing when pasting from one locale to another, in the test case en-US : https://github.com/adobe/react-spectrum/pull/6520/commits/3a44259268d3fe945fa59d10027709a88ba1297c#diff-7bf83599612e94f27f[…]02e2ea6515e426R244-R246.

rspbot · 2024-07-15T17:08:22Z

Build successful! 🎉

Addresses #6520 (comment)

rspbot · 2024-10-23T14:53:08Z

Build successful! 🎉

… know that their value has changed Improvements to behavior when pasting a value from a different locale into a NumberField. A pasted string containing groups and a decimal, like "3 000 000.25", "3,000,000.25" or "3.000.000,25", will now attempt to parse using supported locales rather than only parsing using the current locale. Where there is ambiguity, as with "1,000" or "1.000", the pasted value will be parsed using the current locale. For example, "1.000" will be interpreted as "1" in en-US, and "1,000" will be "1" in de-DE. If the pasted text is different from the resulting input text, the screen reader should announce "Pasted value: {value}" politely, so that the user hears the new value.

…mbering systems 1. If the value fails to parse using default parser and numbering system for the locale, try other locales and numbering systems. 2. Use a RegEx to capture the decimal part of the value. 3. Improve logic for replacing decimal symbol in the value with the decimal symbol for the current locale. Ambiguous values may still fail to round trip. For example 123,456, will be parsed as 123.456 in locales that use a comma as the decimal symbol, but 123456 in those that use a period as the decimal symbol. "Round trips should fully reverse NumberFormat" test will console.log the details of any of these failures. 3. Strip leading zeros from the integer part using a RegEx rather than parseInt. 4. Handle accounting formatting when handling literals after the last numeral in the value.

Addresses #6520 (comment)

rspbot · 2024-10-28T14:24:35Z

Build successful! 🎉

rspbot · 2024-11-12T17:13:59Z

Build successful! 🎉

rspbot · 2024-11-19T16:24:58Z

Build successful! 🎉

…ield-input-masking

rspbot · 2024-12-03T22:06:14Z

Build successful! 🎉

rspbot · 2024-12-04T17:18:20Z

Build successful! 🎉

rspbot · 2024-12-05T15:43:22Z

Build successful! 🎉

rspbot · 2025-01-02T14:59:08Z

Build successful! 🎉

rspbot · 2025-01-02T14:59:09Z

## API Changes

@react-stately/color

/@react-stately/color:ColorChannelFieldState

 ColorChannelFieldState {
   canDecrement: boolean
   canIncrement: boolean
   colorValue: Color
   commit: () => void
   commitValidation: () => void
   decrement: () => void
   decrementToMin: () => void
   displayValidation: ValidationResult
   increment: () => void
   incrementToMax: () => void
   inputValue: string
   maxValue?: number
   minValue?: number
   numberValue: number
+  parseValueInAnySupportedLocale: (string) => number
   realtimeValidation: ValidationResult
   resetValidation: () => void
   setInputValue: (string) => void
   setNumberValue: (number) => void
   validate: (string) => boolean
 }

@react-stately/numberfield

/@react-stately/numberfield:NumberFieldState

 NumberFieldState {
   canDecrement: boolean
   canIncrement: boolean
   commit: () => void
   commitValidation: () => void
   decrement: () => void
   decrementToMin: () => void
   displayValidation: ValidationResult
   increment: () => void
   incrementToMax: () => void
   inputValue: string
   maxValue?: number
   minValue?: number
   numberValue: number
+  parseValueInAnySupportedLocale: (string) => number
   realtimeValidation: ValidationResult
   resetValidation: () => void
   setInputValue: (string) => void
   setNumberValue: (number) => void
   validate: (string) => boolean
 }

majornista requested review from snowystinger, devongovett and LFDanLu June 10, 2024 23:16

snowystinger reviewed Jun 11, 2024

View reviewed changes

packages/@react-stately/numberfield/src/useNumberFieldState.ts Outdated Show resolved Hide resolved

snowystinger reviewed Jun 11, 2024

View reviewed changes

packages/@react-stately/numberfield/src/useNumberFieldState.ts Outdated Show resolved Hide resolved

packages/@react-stately/numberfield/src/useNumberFieldState.ts Outdated Show resolved Hide resolved

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 5a5e783 to f86672a Compare June 19, 2024 17:23

majornista requested a review from snowystinger June 19, 2024 19:19

snowystinger reviewed Jun 20, 2024

View reviewed changes

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 7b708f7 to 4f2457c Compare June 24, 2024 16:34

majornista requested a review from snowystinger June 24, 2024 16:39

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 4f2457c to 5fdf204 Compare June 24, 2024 18:52

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 5fdf204 to e196cf0 Compare June 26, 2024 15:32

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from d6bc520 to 3a44259 Compare July 11, 2024 13:37

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 3a44259 to 7b59721 Compare July 15, 2024 14:28

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 7b59721 to 3795e27 Compare July 15, 2024 16:52

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 3795e27 to 126765a Compare July 15, 2024 21:02

majornista added a commit that referenced this pull request Oct 23, 2024

fix(#4967): parse leading ambiguous decimal symbol as a decimal

28e8b10

Addresses #6520 (comment)

majornista added a commit that referenced this pull request Oct 23, 2024

fix(#4967): fix logic for filtering group symbol matches within value

d462ea4

Addresses #6520 (comment)

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from 955544e to dd7cb2a Compare October 23, 2024 14:42

majornista added 10 commits October 28, 2024 10:15

Refactor to use a static map of locale, group and decimal separator info

1a60245

Refactor logic into NumberParser's sanitize method.

6f548be

fix(#4967): NumberParser sanitize improvements

023118a

fix NUMERALS_REGEX

091f25f

more NumberParser improvements

86ac341

fix(#4967): parse leading ambiguous decimal symbol as a decimal

44055e3

Addresses #6520 (comment)

fix(#4967): fix logic for filtering group symbol matches within value

d4d3b33

Addresses #6520 (comment)

fix(#4967): update yarn.lock to include live-announcer

a287fb7

majornista force-pushed the Issue-4967-useNumberField-input-masking branch from dd7cb2a to a287fb7 Compare October 28, 2024 14:15

Rocss mentioned this pull request Oct 30, 2024

feat: add support for Devanagari & Bengali decimal numbering systems #6715

Merged

5 tasks

Merge branch 'main' into Issue-4967-useNumberField-input-masking

dd5bb88

Merge branch 'main' into Issue-4967-useNumberField-input-masking

a814918

Merge remote-tracking branch 'origin/main' into Issue-4967-useNumberF…

0c35e7b

…ield-input-masking

Merge branch 'main' into Issue-4967-useNumberField-input-masking

b768cee

Merge branch 'main' into Issue-4967-useNumberField-input-masking

5bdc9b6

Merge branch 'main' into Issue-4967-useNumberField-input-masking

ceb6c34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: #4967 useNumberField mutates number so screenreader users won’t know that their value has changed #6520

fix: #4967 useNumberField mutates number so screenreader users won’t know that their value has changed #6520

majornista commented Jun 10, 2024

rspbot commented Jun 11, 2024

rspbot commented Jun 19, 2024

snowystinger left a comment

majornista commented Jun 24, 2024

rspbot commented Jun 24, 2024

rspbot commented Jun 24, 2024

snowystinger commented Jun 26, 2024

rspbot commented Jun 26, 2024

rspbot commented Jun 26, 2024

rspbot commented Jul 10, 2024

rspbot commented Jul 10, 2024

rspbot commented Jul 11, 2024

rspbot commented Jul 15, 2024

majornista commented Jul 15, 2024

rspbot commented Jul 15, 2024

rspbot commented Oct 23, 2024

rspbot commented Oct 28, 2024

rspbot commented Nov 12, 2024

rspbot commented Nov 19, 2024

rspbot commented Dec 3, 2024

rspbot commented Dec 4, 2024

rspbot commented Dec 5, 2024

rspbot commented Jan 2, 2025

rspbot commented Jan 2, 2025

fix: #4967 useNumberField mutates number so screenreader users won’t know that their value has changed #6520

Are you sure you want to change the base?

fix: #4967 useNumberField mutates number so screenreader users won’t know that their value has changed #6520

Conversation

majornista commented Jun 10, 2024

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

rspbot commented Jun 11, 2024

rspbot commented Jun 19, 2024

snowystinger left a comment

Choose a reason for hiding this comment

majornista commented Jun 24, 2024

rspbot commented Jun 24, 2024

rspbot commented Jun 24, 2024

snowystinger commented Jun 26, 2024

rspbot commented Jun 26, 2024

rspbot commented Jun 26, 2024

rspbot commented Jul 10, 2024

rspbot commented Jul 10, 2024

rspbot commented Jul 11, 2024

rspbot commented Jul 15, 2024

majornista commented Jul 15, 2024

rspbot commented Jul 15, 2024

rspbot commented Oct 23, 2024

rspbot commented Oct 28, 2024

rspbot commented Nov 12, 2024

rspbot commented Nov 19, 2024

rspbot commented Dec 3, 2024

rspbot commented Dec 4, 2024

rspbot commented Dec 5, 2024

rspbot commented Jan 2, 2025

rspbot commented Jan 2, 2025

@react-stately/color

/@react-stately/color:ColorChannelFieldState

@react-stately/numberfield

/@react-stately/numberfield:NumberFieldState