Skip to content

feat(ui): Add thousands separator formatting to item amount input#1927

Merged
thelindat merged 4 commits into
overextended:mainfrom
Acc-Off:feature/inventory_control_input_format
May 2, 2026
Merged

feat(ui): Add thousands separator formatting to item amount input#1927
thelindat merged 4 commits into
overextended:mainfrom
Acc-Off:feature/inventory_control_input_format

Conversation

@Acc-Off

@Acc-Off Acc-Off commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Replace the type="number" input with a type="text" input for the item
amount field in InventoryControl, and apply comma-separated thousands
formatting (e.g. 1,000, 10,000) using toLocaleString('en-us').

Changes

  • Changed input type from number to text
  • Format the entered value with thousands separators on each keystroke
  • Preserve cursor position after formatting using useRef and useEffect
  • Strip non-numeric characters before dispatching the value to the Redux store

Motivation

The default number input does not display thousand separators, making it
difficult to read large quantities at a glance. This change improves
readability without affecting the underlying numeric value stored in state.

Testing

  • Type a large number (e.g. 1000000) and verify it displays as 1,000,000
  • Verify the cursor does not jump to the end while editing mid-value
  • Verify that the dispatched amount is correct (no commas included)
  • Verify that empty input resets the amount to 0

Demo

563879358-03f1a7ad-6307-42d4-9742-64dd8cd9633d

@Maximus7474

Copy link
Copy Markdown
Member

There's an issue with the implementation, when pressing backspace or delete around a comma it resets the cursor to the end of line.

issue

@Acc-Off

Acc-Off commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback! I've pushed a fix that addresses this.

Previously, pressing Backspace or Delete adjacent to a comma only moved the cursor without modifying the value, causing it to reset to the end. The updated behavior now deletes the comma together with the adjacent digit:

Backspace after a comma (e.g. "12,|345") → removes 2, → "1,345"
Delete before a comma (e.g. "12|,345") → removes ,3 → "1,245"

@Acc-Off

Acc-Off commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

@Maximus7474
Not sure if GitHub notified you automatically, so mentioning you just in case!
I've pushed a fix for the backspace/delete issue you mentioned. Thanks!

@unitysync unitysync left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be done with significantly less code. something like this would be good. (ive truncated the unchanged code, however it should be a simply copy and paste into the file)

const formatAmount = (n: number) => (n ? n.toLocaleString('en-US') : '')
const digitsOnly = (s: string) => s.replace(/\D/g, '')
const countDigitsBefore = (s: string, index: number) => digitsOnly(s.substring(0, index)).length

const itemAmount = useAppSelector(selectItemAmount)
const dispatch = useAppDispatch()

const [infoVisible, setInfoVisible] = useState(false)
const [value, setValue] = useState(formatAmount(itemAmount))

const inputRef = useRef<HTMLInputElement>(null)
const cursorRef = useRef<number | null>(null)

const commitValue = (raw: string, cursorIndex: number) => {
  const digitsBefore = countDigitsBefore(raw, cursorIndex)
  const num = parseInt(digitsOnly(raw), 10) || 0

  setValue(formatAmount(num))
  dispatch(setItemAmount(num))
  cursorRef.current = digitsBefore
}

// use `handleChange` instead of `inputHandler` for naming consistency
const handleChange = (event: React.ChangeEvent<HTMLInputElement>) =>
  commitValue(event.target.value, event.target.selectionStart ?? 0)

const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
  const el = event.currentTarget
  const pos = el.selectionStart ?? 0

  if (pos !== el.selectionEnd) return

  if (event.key === 'Backspace' && el.value[pos - 1] === ',') {
    event.preventDefault()
    commitValue(el.value.slice(0, pos - 2) + el.value.slice(pos), pos - 2)
  } else if (event.key === 'Delete' && el.value[pos] === ',') {
    event.preventDefault()
    commitValue(el.value.slice(0, pos) + el.value.slice(pos + 2), pos)
  }
}

useEffect(() => {
  if (!inputRef.current || cursorRef.current === null) return
  let newPos = 0
  let count = 0

  for (let i = 0; i < value.length && count < cursorRef.current; i++) {
    if (/\d/.test(value[i])) count++
    newPos++
  }

  inputRef.current.setSelectionRange(newPos, newPos)
  cursorRef.current = null
}, [value])```

@Acc-Off

Acc-Off commented May 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the clean refactor suggestion! Applied it — much simpler now with commitValue centralizing the logic. Pushed!

@Acc-Off Acc-Off requested a review from unitysync May 1, 2026 06:10

@unitysync unitysync left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good so far, except i noticed something in testing.

Comment thread web/src/components/inventory/InventoryControl.tsx Outdated
@Acc-Off Acc-Off requested a review from unitysync May 1, 2026 10:57

@unitysync unitysync left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @thelindat

@thelindat thelindat merged commit fb8f347 into overextended:main May 2, 2026
@Acc-Off Acc-Off deleted the feature/inventory_control_input_format branch May 2, 2026 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants