-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: numberfield announce pasted value #8604
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
base: main
Are you sure you want to change the base?
Conversation
Build successful! 🎉 |
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.
how best should we go about testing that this works with all kinds of locales/number formats? I was able to confirm the behavior with values like 3.000.000,25
and 1.000
, but I guess it might be good enough that those pass since the parsing/formatting/validating is tested elsewhere
let value = state.parser.parse(pastedText); | ||
let reformattedValue = numberFormatter.format(value); | ||
if (state.validate(reformattedValue)) { |
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.
Does there exist a case where the pastedText
is considered valid but is unable to be parsed? The description of parse
seems to suggest that might be the case, but digging to see if that is the case. Just concerned if there are any cases where we make it here but end up not announcing that we couldn't parse the value
"numberField": "حقل رقمي" | ||
"numberField": "حقل رقمي", | ||
"pastedValue": "Pasted value: {value}", | ||
"couldNotParseValue": "Could not understand value: {value}, try another format perhaps" |
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.
mega nit for more assertive message haha:
"couldNotParseValue": "Could not understand value: {value}, try another format perhaps" | |
"couldNotParseValue": "Could not understand value: {value}, please try another number format" |
Couldn't the original problem from #4967 still exist? If you typed the value (rather than pasted) and then blurred, and our parsing resulted in something different, it wouldn't get announced. Should we just announce the formatted value on blur instead of paste? |
Yeah, I was debating this, as the changes also exposed parse on the state object, which I wasn't thrilled with, in case the parser suddenly changed one render to another announcing in onBlur isn't quite right either, we don't know if the value is going to be accepted by a controlled component, so in that case the during paste was more "correct" we could do it in an effect after, but then it gets hard to know if we're just announcing typing (based on inputValue) or if we base it on value, then we may get no announcement if it failed to write a new number and reverted |
# Conflicts: # packages/@react-aria/numberfield/package.json # yarn.lock
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:NumberFieldState NumberFieldState {
canDecrement: boolean
canIncrement: boolean
commit: () => void
commitValidation: () => void
decrement: () => void
decrementToMin: () => void
defaultNumberValue: number
displayValidation: ValidationResult
increment: () => void
incrementToMax: () => void
inputValue: string
maxValue?: number
minValue?: number
numberValue: number
+ parser: NumberParser
realtimeValidation: ValidationResult
resetValidation: () => void
setInputValue: (string) => void
setNumberValue: (number) => void
validate: (string) => boolean
} @react-stately/color/@react-stately/color:ColorChannelFieldState ColorChannelFieldState {
canDecrement: boolean
canIncrement: boolean
colorValue: Color
commit: () => void
commitValidation: () => void
decrement: () => void
decrementToMin: () => void
defaultColorValue: Color | null
defaultNumberValue: number
displayValidation: ValidationResult
increment: () => void
incrementToMax: () => void
inputValue: string
maxValue?: number
minValue?: number
numberValue: number
+ parser: NumberParser
realtimeValidation: ValidationResult
resetValidation: () => void
setColorValue: (Color | null) => void
setInputValue: (string) => void
updateValidation: (ValidationResult) => void
validate: (string) => boolean
} @react-stately/numberfield/@react-stately/numberfield:NumberFieldState NumberFieldState {
canDecrement: boolean
canIncrement: boolean
commit: () => void
commitValidation: () => void
decrement: () => void
decrementToMin: () => void
defaultNumberValue: number
displayValidation: ValidationResult
increment: () => void
incrementToMax: () => void
inputValue: string
maxValue?: number
minValue?: number
numberValue: number
+ parser: NumberParser
realtimeValidation: ValidationResult
resetValidation: () => void
setInputValue: (string) => void
setNumberValue: (number) => void
validate: (string) => boolean
} |
Closes #4967
Split out of #6520
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: