adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.86k stars 1.11k forks source link

Unit in NumberField cannot be deleted (for some units/ unit-displays) #2806

Open ghost opened 2 years ago

ghost commented 2 years ago

🐛 Bug Report

I am using the useNumberField with the "formatOptions" option to add units to the number fields.

Using the following option works fine:

formatOptions={{
    style: "unit",
    unit: "gram",
}}

But when I am using the "milliliter" unit, the unit's letters cannot be deleted (by pressing the backspace or the delete key).

formatOptions={{
    style: "unit",
    unit: "milligram",
}}

When using unitDisplay="long only the last letter can be deleted.

🤔 Expected Behavior

Pressing the backspace or the delete key should delete unit's letters inside the input.

😯 Current Behavior

Pressing the backspace or the delete key does not always delete the unit's letters inside the input.

💁 Possible Solution

🔦 Context

💻 Code Sample

Codesandbox reproduction

Go to the codesandbox and focus one NumberField after another to see which letters of which units can be deleted and which cannot.

🌍 Your Environment

Software Version(s)
react-spectrum 3.9.0
Browser
Operating System
snowystinger commented 2 years ago

So the way this should work is that you can only delete the units if you delete the entire units. This is why you can delete 'g' for grams, but no part of 'mL' for milliliters. You can select the entire string 'mL' and delete that.

As for the "s" at the end of some of those units, that seems like a bug.

I'm guessing it's coming from us storing all possible permutations of the word, plural and singular in this case, since we can't know which will end up being the final number. I'm not sure there is anything we can do about it unless we started to allow deleting of the entire unit letter by letter. We tried doing this at one point, I don't recall if it was before or after one of the refactors. I recall it having issues with partial number validation due to a unit symbol being able to contain characters that might also be part of a number. For instance (a contrived example because I don't recall the combination) "1.000.000 ."

Where that trailing dot might be the start of the unit, or it might mean that they meant to add another 3 zeros on the end, so trying to parse that might be difficult.

ghost commented 2 years ago

@snowystinger Ah okay, I though not being able to delete the "mL" was a bug and not the expected behaviour. I am totally fine with that, so we could close this issue from my side.

snowystinger commented 2 years ago

Thanks, I'll leave it open so we can remember to try to improve this

Faithfinder commented 1 year ago

Thought about opening a new ticket, but I'll hijack this one instead. The experience of editing a number field with format is quite silly. I would call it "Not accessible to mouse users". You can try it out in the docs with the "Inches" example

1) Clicking into the "unit" part of the field basically makes the field uneditable. You can't type, delete or backspace. And clicking into a unit is much easier than into the number, while we're in single and double digits. 2) Putting the cursor at the end of the "unit" and typing numbers appends them on blur. I think that would be quite unexpected to a normal user.

cmawhorter commented 4 months ago

i was surprised by this as well. i'm not an expert but AFAICT, all locales display the unit as a suffix. if true, maybe the unit could be moved outside of the <input> into static text? or even treating the unit as a single token for backspace would be great, but i know how difficult that is.

coming up with something that does that outside of the useNumber hooks is problematic and i think i'll need to use style=decimal and then create a second instance of NumberFormat to get the localized unit label. however, i'm a little concerned there are rendering differences in amounts between decimal and unit i'm not aware of.

otherwise... holy crap the internationalized number stuffs you have are amazingly useful. the fact this isn't built into the spec and browsers is just crazy to me. why does accepting i18n number input have to be this hard 😢

snowystinger commented 3 months ago

Unfortunately, there are units where they are the prefix. It's mostly currencies. Counter example:

    CN¥024,663.430041683405 ko-KR {
      style: 'currency',
      currency: 'CNY',
      currencyDisplay: 'symbol',
      localeMatcher: 'best fit',
      useGrouping: true,
      minimumIntegerDigits: 6,
      minimumFractionDigits: 2,
      maximumFractionDigits: 9,
      minimumSignificantDigits: 1,
      maximumSignificantDigits: 18
    }

There's also the issue where the unit might be inside the number -EUR 20,30 pt-BR

cmawhorter commented 3 months ago

@snowystinger my comment was about the Intl.NumberFormat style "unit" specifically. but it should be noted, no style other than unit (currency, percent) has the issue with deleting the non-numeric part of the input, from what i've seen.

i was able to work around by using decimal and doing my own formatToParts. FWIW here's my implementation that adds the unit outside of the input, but be warned it makes some assumptions that i believe to be true, but are still assumptions (aria, last part.type=unit wins):

import React from 'react';
// NumberInput is useNumberField
import { NumberInput, NumberInputProps } from '../NumberInput/NumberInput';
import { useLocale } from '@react-aria/i18n';
import { useInputId } from '../../hooks/input_id';
import { Unit } from '../../data/units';

type InheritedNumberInputProps = Omit<NumberInputProps, 'formatOptions'>;

export interface UnitInputProps extends InheritedNumberInputProps {
  unit: Unit;
  /**
   * Default is "long" because it makes more sense here
   */
  unitDisplay?: 'short' | 'narrow' | 'long';
  minimumFractionDigits?: number;
  maximumFractionDigits?: number;
}

export const UnitInput = ({
  id: rawId,
  value,
  unit,
  unitDisplay = 'long',
  minimumFractionDigits,
  maximumFractionDigits,
  ...props
}: UnitInputProps) => {
  const {locale} = useLocale();
  // used to get localized unit via formatToParts
  const unitFormatter = React.useMemo(() => {
    return new Intl.NumberFormat(locale, {
      style: 'unit',
      unit,
      unitDisplay,
      minimumFractionDigits,
      maximumFractionDigits,
    });
  }, [locale, unit, unitDisplay, minimumFractionDigits, maximumFractionDigits]);
  const id = useInputId(rawId);
  const unitId = id.deriveId('unit').toString();
  return (
    <div className="inline-flex gap-2 items-center min-w-0 max-w-full w-auto">
      <NumberInput 
        {...props} 
        id={id}
        value={value}
        aria-labelledby={
          // NOTE: we use labelledby because this is probably the least-worst option i.e. readers say "Weight pounds" for an input with the label "Weight"
          `${props['aria-labelledby'] || ''} ${unitId}`.trim()
        }
        formatOptions={{
          style: 'decimal',
          minimumFractionDigits,
          maximumFractionDigits,
        } as Intl.NumberFormatOptions} />
      <span id={unitId} className="text-muted-foreground">{getUnitLabel(unitFormatter, value)}</span>
    </div>
  );
};

const getUnitLabel = (formatter: Intl.NumberFormat, value: undefined | number): null | string => {
  if (value === undefined || Number.isNaN(value)) return null;
  const parts = formatter.formatToParts(value);
  let unit = '';
  for (const part of parts) {
    // not sure if multiple are possible but we'll choose the last one as winner
    if (part.type === 'unit' && part.value) {
      unit = part.value;
    }
  }
  return unit ? unit : null;
};
snowystinger commented 3 months ago

Thanks for sharing that!