fix(number-input): backspace behavior with formatted numbers#5719
Merged
Conversation
🦋 Changeset detectedLatest commit: f937dc9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughAdds a patch changeset for @heroui/number-input, implements locale-aware Backspace handling when the caret sits between a digit and the first grouping separator, and adds tests verifying backspace behavior with grouped/formatted numbers. No public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant NI as NumberInput (DOM)
participant H as useNumberInput.handleKeyDown
participant S as Internal State
U->>NI: Press Backspace
NI->>H: handleKeyDown(event)
rect rgb(230,245,255)
note over H: Caret between digit and first grouping separator
H-->>NI: event.preventDefault()
H->>H: compute newValue (remove preceding digit)
H->>H: clean/parse numeric string
H->>S: setNumberValue / setInputValue
H-->>NI: setTimeout(0) to restore caret position
end
NI-->>U: Updated displayed value and caret position
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/code
@heroui/date-input
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-aria-overlay
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-form-reset
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/use-viewport-size
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/number-input/src/use-number-input.ts (1)
245-292: Make number-input locale-aware (RTL & non‑Latin digits)Confirmed: packages/components/number-input/src/use-number-input.ts (~lines 245–292) uses ASCII-only cleanup (checks for ','; value.replace(/[^\d.-]/g, '') and parseFloat(cleanValue)) which strips Arabic‑Indic/Eastern digits and mishandles locale group/decimal separators. Action: normalize input in a locale-aware way — get locale decimal/group chars via Intl.NumberFormat, map Unicode digits (e.g. \p{Nd} ranges) to ASCII before parsing, and adjust cursor-position logic to compute the new caret after normalization. Note: apps/docs/utils/number.ts also uses parseFloat (docs only — review if UI-facing); packages/components/toast/src/use-toast.ts uses parseFloat for CSS margins (acceptable).
🧹 Nitpick comments (4)
packages/components/number-input/src/use-number-input.ts (3)
275-281: Cursor restore timingsetTimeout(0) works but can be racy under concurrent renders. Consider requestAnimationFrame or queueMicrotask for tighter ordering.
Example:
- setTimeout(() => { + queueMicrotask(() => { // set the new cursor position const pos = Math.max(0, selectionStart - 1); inputElement.setSelectionRange(pos, pos); - }, 0); + });
291-292: Dependency array should include localehandleKeyDown now depends on locale; add it to avoid stale grouping char.
Apply this diff:
- [inputValue, state, onClear, isClearable, originalProps.isReadOnly], + [inputValue, state, onClear, isClearable, originalProps.isReadOnly, locale],
245-292: Optional: simplify the conditionThe guard
value[selectionStart - 1] !== groupCharis unnecessary given real formats never have consecutive group chars. You can drop it to reduce branching.packages/components/number-input/__tests__/number-input.test.tsx (1)
596-652: Add i18n/readOnly/decimal coverageGreat regression tests. Please add:
- A locale with non-comma grouping (e.g., de-DE or fr-FR narrow NBSP).
- readOnly scenario to ensure value doesn’t change on Backspace.
- A case with decimals and a negative number.
- (Optional) Verify Delete key remains default (no special handling).
Example (sketch):
import {I18nProvider} from "@react-aria/i18n"; it("handles backspace with de-DE grouping", () => { const {container} = render( <I18nProvider locale="de-DE"> <NumberInput defaultValue={1234} formatOptions={{style: "decimal", useGrouping: true}} /> </I18nProvider>, ); const input = container.querySelector("input[type='text']") as HTMLInputElement; expect(input.value).toBe("1.234"); act(() => { input.focus(); input.setSelectionRange(1, 1); // 1|.234 fireEvent.keyDown(input, {key: "Backspace"}); }); expect(input.value).toBe("234"); }); it("does not modify value on Backspace when readOnly", () => { const {container} = render( <NumberInput isReadOnly defaultValue={1234} formatOptions={{style: "decimal", useGrouping: true}} /> ); const input = container.querySelector("input[type='text']") as HTMLInputElement; act(() => { input.focus(); input.setSelectionRange(1, 1); fireEvent.keyDown(input, {key: "Backspace"}); }); expect(input.value).toBe("1,234"); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/gentle-owls-arrive.md(1 hunks)packages/components/number-input/__tests__/number-input.test.tsx(1 hunks)packages/components/number-input/src/use-number-input.ts(2 hunks)
🔇 Additional comments (1)
.changeset/gentle-owls-arrive.md (1)
1-6: Changeset looks goodPatch scope and message are appropriate and reference the bug ID. No action needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/components/number-input/src/use-number-input.ts (3)
296-297: Include missing dependencies in useCallback
localeandoriginalProps.isDisabledare captured but not in deps, leading to stale behavior when they change.Apply this diff:
- [inputValue, state, onClear, isClearable, originalProps.isReadOnly], + [inputValue, state, onClear, isClearable, locale, originalProps.isReadOnly, originalProps.isDisabled],
251-253: Fix comment typo and scopeComment ends with a stray “if (” and says “first” though condition applies to any group separator.
Apply this diff:
- // handle backspace when cursor is between a digit and the first group separator - // e.g. 1|,234 (en-US) or 1|.234 (de-DE) -> backspace removes the preceding digit if ( + // Handle Backspace when the cursor is between a digit and a group separator. + // e.g., 1|,234 (en-US) or 1|.234 (de-DE) -> Backspace removes the preceding digit.
248-250: Optional: memoize the grouping characterAvoid recomputing per keydown and shrink the handler. Compute once per
locale.You can hoist and memoize:
const groupChar = useMemo(() => { const nf = new Intl.NumberFormat(locale, {useGrouping: true}); return nf.formatToParts(1000).find((p) => p.type === "group")?.value ?? ","; }, [locale]);Then remove the per-call computation inside
handleKeyDown.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/components/number-input/src/use-number-input.ts(2 hunks)
🔇 Additional comments (2)
packages/components/number-input/src/use-number-input.ts (2)
248-250: Locale-aware grouping separator detection looks goodGood use of Intl.NumberFormat + formatToParts with a safe fallback.
268-278: Locale parsing bug: replace/parseFloat breaks decimal-comma and non-ASCII digitsRegex cleanup + parseFloat is not locale-safe (e.g., de-DE: ".234,56" → 0.234). Delegate parsing/formatting to the NumberField state by setting the localized string directly.
Apply this diff:
- // e.g. ,234 -> 234 - const cleanValue = newValue.replace(/[^\d.-]/g, ""); - - if (cleanValue === "" || cleanValue === "-") { - state.setInputValue(""); - } else { - const numberValue = parseFloat(cleanValue); - - if (!isNaN(numberValue)) { - state.setNumberValue(numberValue); - } - } + // Normalize leading group char so the localized parser can handle it. + // Examples: ",234" -> "234", "-,234" -> "-234" + let normalized = newValue; + if (normalized.startsWith(groupChar)) { + normalized = normalized.slice(groupChar.length); + } else if (normalized.startsWith("-" + groupChar)) { + normalized = "-" + normalized.slice(1 + groupChar.length); + } + state.setInputValue(normalized);
This was referenced Nov 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5712
📝 Description
The problem occurred when the cursor was positioned between the first digit and the first comma in a formatted number (e.g., between "1" and "," in "1,234"). Pressing backspace in this position would do nothing.
This PR is to add the handling for such case.
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
Bug Fixes
Tests
Chores