fix(core): allow typing decimals in numeric column filters - #950
Open
marianmoldovan wants to merge 1 commit into
Open
fix(core): allow typing decimals in numeric column filters#950marianmoldovan wants to merge 1 commit into
marianmoldovan wants to merge 1 commit into
Conversation
The numeric column filter would not let you type a value like `0.01`: as soon as
a `0` was typed after the decimal separator the whole field collapsed to `0`, and
typing `.01` lost the separator entirely.
Root cause: the filter's `TextField` was controlled from the parsed number
(`value={String(internalValue)}`) while `onChange` ran `parseFloat` on the raw
text. Every keystroke round-tripped text -> number -> text, which destroys any
intermediate state that is not a canonical number:
"0." -> parseFloat -> 0 -> String -> "0" (separator gone)
"0.0" -> parseFloat -> 0 -> String -> "0"
so `0.01` was unreachable when typed left to right, while pasting worked because
the text was already canonical. The form field binding is unaffected because it
renders the value held in form state instead of re-deriving it from a parse.
Fix: hold the raw input text in its own `inputText` state and render that, so
what the user typed is never rewritten. A new pure helper,
`parseFilterTextInput`, converts the text into a filter value and only commits a
number once the text is a complete number, otherwise leaving the current filter
value untouched. Intermediate inputs such as "", "-", "0.", ".0" and "1." are now
preserved in the input without corrupting the filter.
`0` remains a valid filter value; only an empty input clears the filter. The
props sync effect still updates the displayed text when the value is changed
elsewhere, but compares the incoming value against the one the text was built
from so it cannot clobber an in-progress edit. The `is-null` operation and the
`enumValues` select paths are unchanged.
Parsing is also stricter than `parseFloat` was: "1.2.3", "12abc", "0x10" and
"1e999" no longer sneak a bogus number into the filter.
Fixes #715
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes numeric column filter typing by decoupling the rendered input text from the parsed filter value, preventing intermediate keystrokes (e.g. 0., -) from being clobbered during controlled re-renders.
Changes:
- Update
StringNumberFilterFieldto render from a dedicatedinputTextstate and parse/commit values only when the text is a “complete” number. - Add a pure
filter_text_input.tshelper to centralize parsing and value→text rendering. - Add Jest unit tests covering per-keystroke behavior and edge cases for numeric parsing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/firecms_core/src/components/SelectableTable/filters/StringNumberFilterField.tsx | Switches numeric/string filter input to preserve raw typing while committing parsed values safely. |
| packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.ts | Introduces strict parsing + value-to-text helper for filter inputs. |
| packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.test.ts | Adds unit tests validating parsing and per-keystroke commit behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+176
to
179
| endAdornment={inputText !== "" && <IconButton | ||
| onClick={(e) => updateFilter(operation, undefined)}> | ||
| <CloseIcon /> | ||
| </IconButton>} |
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
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.
Fixes #715
The bug
You could not type
0.01into a numeric column filter. TheTextFieldwas controlled from the parsed number (String(internalValue)) whileonChangeranparseFloat, so every keystroke round-tripped text → number → text and destroyed intermediate states:parseFloat("0.")andparseFloat("0.0")are both0, which renders as"0", so0.01was unreachable left-to-right. Pasting worked because the text was already canonical. The form/inline editors were unaffected becauseTextFieldBindingrendersvaluefrom form state rather than re-deriving it.The fix
Raw input text lives in its own state and is rendered directly; a
syncedValueRefstops the props-sync effect clobbering in-progress typing; parsing is extracted into a purefilter_text_input.tshelper.is-nullstill disables the input; theenumValuesSelect/MultiSelect branches are untouched.Tests
13 tests on the pure helper, asserting the value after every keystroke for
0.01,1.,-1; final values for0.001,-0.5,.5,.01,1.05,10.00001,-.5; reverse deletion to empty; and that0stays a valid filter value.@firecms/core: 250/253 passing (was 237/240) — 3 pre-existing failures unchanged.Two judgement calls to review
internalValue != nulltoinputText !== "". The earlier0-shows-the-X fix (c570dd02f) is preserved since"0"is non-empty. Side effect: an emptied string filter no longer shows a dangling X on an empty field.parseFloat:"1.2.3","12abc","0x10","1e999"no longer commit bogus values (previously1.2,12,0,Infinity). An improvement, but a behaviour change.Separate pre-existing quirk found, left alone
Switching the operation away from
is-nulldoesn't work —updateFilter("==", null)re-emits["==", null]and the sync effect maps it straight back tois-null. Identical onmain, unrelated to this issue.Not verified
Typing behaviour needs a browser. The package's jest
testEnvironmentisnodeand the one existing component test already fails to run, so no DOM test was added. Verified instead with a faithful simulation of the new component state machine (bothuseStates, the ref guard, and the sync effect re-firing after everysetValue).🤖 Generated with Claude Code