[number field] Improve parsing logic #2725
Conversation
commit: |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Bundle size report
|
atomiks
force-pushed
the
fix/number-field-parsing
branch
from
f60460e to
05d3dfb
Compare
atomiks
force-pushed
the
fix/number-field-parsing
branch
from
05d3dfb to
c535a1c
Compare
| 六: '6', | ||
| 七: '7', | ||
| 八: '8', | ||
| 九: '9', |
There was a problem hiding this comment.
| 九: '9', | |
| 九: '9', | |
| 十: '10', |
@atomiks This is supposed to simply convert single digits?
There was a problem hiding this comment.
It should - let me know if that char is necessary though to handle another use case
There was a problem hiding this comment.
LLM review as this is out of my depth a bit:
We intentionally only normalize single-glyph Han digits (零/〇…九). Adding 十: '10' would break inputs like 三十 (30), which would become "310" under per-char substitution. If we need full ideographic number parsing, that should be a separate parser; this function is just digit normalization across scripts.
There was a problem hiding this comment.
OK it doesn't matter then if it's just converting each individual char into a digit
Though in practice this would only work for 0-9 (and not 10+) since there are characters for 10/100/1000, so the numbering system is similar to roman numerals
But this is indeed an incredibly exotic use-case so it's ok
| }); | ||
|
|
||
| it('collapses extra dots from mixed-locale inputs', () => { | ||
| // First '.' is decimal; subsequent '.' are removed |
There was a problem hiding this comment.
| // First '.' is decimal; subsequent '.' are removed | |
| // Last '.' is decimal; previous '.' are removed |
There was a problem hiding this comment.
This could be the culprit in onKeyDown as it doesn't pass the locale, although it uses your runtime locale. Is it in US/English for you when the bug occurs?
const { decimal, currency, percentSign } = getNumberLocaleDetails(
[],
formatOptionsRef.current,
);There was a problem hiding this comment.
Yup, but I see you fixed it. Seems to work OK, but I found another issue 😅 - when on "pl-PL' locale (where we use spaces as thousand separators), I can't type in space at all.
There was a problem hiding this comment.
@michaldudak I see, fixed
atomiks
force-pushed
the
fix/number-field-parsing
branch
from
7c72b98 to
43d3913
Compare
atomiks
force-pushed
the
fix/number-field-parsing
branch
from
0010ea2 to
632c655
Compare
Handles a few more edge cases correctly.
Fixes #2735