From e2d74b7bf4eae673c12305ad7ec7d22cd1ac7e72 Mon Sep 17 00:00:00 2001 From: Marian Moldovan Date: Wed, 29 Jul 2026 16:55:15 +0200 Subject: [PATCH] fix(core): allow typing decimals in numeric column filters 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 --- .../filters/StringNumberFilterField.tsx | 40 +++- .../filters/filter_text_input.test.ts | 184 ++++++++++++++++++ .../filters/filter_text_input.ts | 71 +++++++ 3 files changed, 288 insertions(+), 7 deletions(-) create mode 100644 packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.test.ts create mode 100644 packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.ts diff --git a/packages/firecms_core/src/components/SelectableTable/filters/StringNumberFilterField.tsx b/packages/firecms_core/src/components/SelectableTable/filters/StringNumberFilterField.tsx index 5ee72ebd8..a6df87417 100644 --- a/packages/firecms_core/src/components/SelectableTable/filters/StringNumberFilterField.tsx +++ b/packages/firecms_core/src/components/SelectableTable/filters/StringNumberFilterField.tsx @@ -11,6 +11,7 @@ import { TextField } from "@firecms/ui"; import { EnumValueConfig } from "../../../types"; +import { filterValueToText, parseFilterTextInput } from "./filter_text_input"; interface StringNumberFilterFieldProps { name: string, @@ -60,8 +61,15 @@ export function StringNumberFilterField({ const [fieldOperation, fieldValue] = value || [possibleOperations[0], undefined]; const [operation, setOperation] = useState(fieldOperation === "==" && fieldValue === null ? "is-null" : fieldOperation); const [internalValue, setInternalValue] = useState(fieldValue); + // Raw text of the text input, kept apart from `internalValue` so that intermediate + // inputs like "-", "0." or ".0" are not destroyed while the user is typing. + const [inputText, setInputText] = useState(filterValueToText(fieldValue)); + // Value the current `inputText` was built from, used to tell updates coming from + // outside apart from the ones triggered by the user typing. + const syncedValueRef = React.useRef(fieldValue); React.useEffect(() => { + const newValue = value ? value[1] : undefined; if (value) { const [op, val] = value; setOperation(op === "==" && val === null ? "is-null" : op); @@ -70,15 +78,25 @@ export function StringNumberFilterField({ setOperation(possibleOperations[0]); setInternalValue(undefined); } + // only reset the text when the value was changed somewhere else, so we don't + // interfere with what the user is currently typing + if (syncedValueRef.current !== newValue) { + syncedValueRef.current = newValue; + setInputText(filterValueToText(newValue)); + } }, [value, possibleOperations[0]]); const isNullOperation = operation === "is-null"; - function updateFilter(op: VirtualTableWhereFilterOp | "is-null", val: string | number | string[] | number[] | null | undefined) { + // `keepInputText` is used when the update originates from the text input, where the + // text typed by the user is authoritative and must not be overwritten with `val` + function updateFilter(op: VirtualTableWhereFilterOp | "is-null", val: string | number | string[] | number[] | null | undefined, keepInputText = false) { // Handle "is null" operation if (op === "is-null") { setOperation(op); setInternalValue(null); + syncedValueRef.current = null; + setInputText(""); setValue(["==", null]); return; } @@ -96,6 +114,9 @@ export function StringNumberFilterField({ setOperation(op); setInternalValue(newValue); + syncedValueRef.current = newValue; + if (!keepInputText) + setInputText(filterValueToText(newValue)); const hasNewValue = newValue !== null && Array.isArray(newValue) ? newValue.length > 0 @@ -111,6 +132,14 @@ export function StringNumberFilterField({ } } + function updateFilterFromText(text: string) { + setInputText(text); + const { updateValue, value: parsedValue } = parseFilterTextInput(text, dataType); + // intermediate inputs like "-" or "0." keep the filter as it is + if (updateValue) + updateFilter(operation, parsedValue, true); + } + const multiple = multipleSelectOperations.includes(operation); return ( @@ -138,16 +167,13 @@ export function StringNumberFilterField({ {!enumValues && { - const val = dataType === "number" - ? parseFloat(evt.target.value) - : evt.target.value; - updateFilter(operation, val); + updateFilterFromText(evt.target.value); }} - endAdornment={internalValue !== undefined && internalValue != null && updateFilter(operation, undefined)}> } diff --git a/packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.test.ts b/packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.test.ts new file mode 100644 index 000000000..7bf481222 --- /dev/null +++ b/packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.test.ts @@ -0,0 +1,184 @@ +import { describe, expect, it } from "@jest/globals"; +import { filterValueToText, parseFilterTextInput } from "./filter_text_input"; + +/** + * Type `text` one character at a time, the way the filter field does, and keep track + * of the filter value left after every keystroke. + */ +function typeCharByChar(text: string, dataType: "string" | "number" = "number") { + let value: string | number | undefined; + const valueAfterEachKeystroke: (string | number | undefined)[] = []; + for (let i = 1; i <= text.length; i++) { + const result = parseFilterTextInput(text.slice(0, i), dataType); + if (result.updateValue) + value = result.value; + valueAfterEachKeystroke.push(value); + } + return { + value, + valueAfterEachKeystroke + }; +} + +describe("parseFilterTextInput with numbers", () => { + + it("should parse complete numbers", () => { + expect(parseFilterTextInput("0.01", "number")).toEqual({ + updateValue: true, + value: 0.01 + }); + expect(parseFilterTextInput("1", "number")).toEqual({ + updateValue: true, + value: 1 + }); + expect(parseFilterTextInput("1.5", "number")).toEqual({ + updateValue: true, + value: 1.5 + }); + expect(parseFilterTextInput("-0.5", "number")).toEqual({ + updateValue: true, + value: -0.5 + }); + expect(parseFilterTextInput(".5", "number")).toEqual({ + updateValue: true, + value: 0.5 + }); + expect(parseFilterTextInput("1e5", "number")).toEqual({ + updateValue: true, + value: 100000 + }); + expect(parseFilterTextInput("0.0100", "number")).toEqual({ + updateValue: true, + value: 0.01 + }); + }); + + it("should keep 0 as a valid filter value", () => { + expect(parseFilterTextInput("0", "number")).toEqual({ + updateValue: true, + value: 0 + }); + expect(parseFilterTextInput("0.0", "number")).toEqual({ + updateValue: true, + value: 0 + }); + expect(parseFilterTextInput("00", "number")).toEqual({ + updateValue: true, + value: 0 + }); + expect(parseFilterTextInput("-0", "number").updateValue).toBe(true); + }); + + it("should clear the filter for an empty input", () => { + expect(parseFilterTextInput("", "number")).toEqual({ + updateValue: true, + value: undefined + }); + expect(parseFilterTextInput(" ", "number")).toEqual({ + updateValue: true, + value: undefined + }); + }); + + it("should not touch the filter value for intermediate inputs", () => { + // these are states the user goes through while typing, the text must be kept + // in the input without the filter value being changed + expect(parseFilterTextInput("0.", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput(".", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("1.", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("-", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("-.", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("+", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("-0.", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("1e", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("1e-", "number")).toEqual({ updateValue: false }); + }); + + it("should not touch the filter value for text that is not a number", () => { + expect(parseFilterTextInput("abc", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("12abc", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("1.2.3", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("--1", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("0x10", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("Infinity", "number")).toEqual({ updateValue: false }); + expect(parseFilterTextInput("1e999", "number")).toEqual({ updateValue: false }); + }); + + it("should allow typing 0.01 character by character", () => { + const { value, valueAfterEachKeystroke } = typeCharByChar("0.01"); + expect(value).toBe(0.01); + // "0" -> 0, "0." -> unchanged, "0.0" -> 0, "0.01" -> 0.01 + expect(valueAfterEachKeystroke).toEqual([0, 0, 0, 0.01]); + }); + + it("should allow typing other decimals character by character", () => { + expect(typeCharByChar("0.001").value).toBe(0.001); + expect(typeCharByChar("-0.5").value).toBe(-0.5); + expect(typeCharByChar(".5").value).toBe(0.5); + expect(typeCharByChar(".01").value).toBe(0.01); + expect(typeCharByChar("1.05").value).toBe(1.05); + expect(typeCharByChar("10.00001").value).toBe(10.00001); + expect(typeCharByChar("-.5").value).toBe(-0.5); + }); + + it("should keep the previous value while typing a trailing decimal separator", () => { + // typing "1." leaves the value at 1, the text "1." stays in the input + expect(typeCharByChar("1.").value).toBe(1); + expect(typeCharByChar("1.").valueAfterEachKeystroke).toEqual([1, 1]); + }); + + it("should not commit a value until a leading sign is followed by digits", () => { + expect(typeCharByChar("-").value).toBeUndefined(); + expect(typeCharByChar("-1").valueAfterEachKeystroke).toEqual([undefined, -1]); + }); + + it("should clear the value when the input is emptied", () => { + // deleting "0.01" one character at a time, from the end + const texts = ["0.0", "0.", "0", ""]; + let value: string | number | undefined = 0.01; + for (const text of texts) { + const result = parseFilterTextInput(text, "number"); + if (result.updateValue) + value = result.value; + } + expect(value).toBeUndefined(); + }); +}); + +describe("parseFilterTextInput with strings", () => { + + it("should use the text as the value", () => { + expect(parseFilterTextInput("abc", "string")).toEqual({ + updateValue: true, + value: "abc" + }); + expect(parseFilterTextInput("0.", "string")).toEqual({ + updateValue: true, + value: "0." + }); + expect(parseFilterTextInput("", "string")).toEqual({ + updateValue: true, + value: "" + }); + expect(parseFilterTextInput(" spaced ", "string")).toEqual({ + updateValue: true, + value: " spaced " + }); + }); +}); + +describe("filterValueToText", () => { + + it("should render values as text", () => { + expect(filterValueToText(0)).toBe("0"); + expect(filterValueToText(0.01)).toBe("0.01"); + expect(filterValueToText(-0.5)).toBe("-0.5"); + expect(filterValueToText("abc")).toBe("abc"); + expect(filterValueToText("")).toBe(""); + }); + + it("should render empty text for no value", () => { + expect(filterValueToText(undefined)).toBe(""); + expect(filterValueToText(null)).toBe(""); + }); +}); diff --git a/packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.ts b/packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.ts new file mode 100644 index 000000000..7d861b014 --- /dev/null +++ b/packages/firecms_core/src/components/SelectableTable/filters/filter_text_input.ts @@ -0,0 +1,71 @@ +/** + * Matches text that is a complete number, as opposed to an intermediate state + * that the user may be in the middle of typing, like "-", "0." or ".". + * A decimal separator must be followed by at least one digit. + */ +const completeNumberRegex = /^[+-]?(?:\d+|\d*\.\d+)(?:[eE][+-]?\d+)?$/; + +export type FilterTextInputParseResult = { + /** + * Whether the filter value should be updated with `value`. + * It is `false` for intermediate inputs that are not a valid value yet, + * like "-" or "0.", so that the text typed by the user can be kept as it is + * while the current filter value is left untouched. + */ + updateValue: boolean; + + /** + * Parsed value, only relevant when `updateValue` is `true`. + * `undefined` means the filter should be cleared. + */ + value?: string | number; +}; + +/** + * Turn the raw text of a filter input into the value used by the filter. + * + * The text shown to the user is never derived back from the parsed value, so + * intermediate states like "0.", ".0" or "-" survive while typing. Numbers are + * only committed to the filter once the text is a complete number, which makes + * values such as 0.01 reachable by typing them character by character. + * + * Note that `0` is a perfectly valid filter value, only an empty input clears + * the filter. + */ +export function parseFilterTextInput(text: string, dataType: "string" | "number"): FilterTextInputParseResult { + + if (dataType !== "number") + return { + updateValue: true, + value: text + }; + + const trimmedText = text.trim(); + + if (trimmedText === "") + return { + updateValue: true, + value: undefined + }; + + if (!completeNumberRegex.test(trimmedText)) + return { updateValue: false }; + + const parsedValue = Number(trimmedText); + if (!isFinite(parsedValue)) + return { updateValue: false }; + + return { + updateValue: true, + value: parsedValue + }; +} + +/** + * Text a filter input should display for a given filter value. + */ +export function filterValueToText(value: unknown): string { + if (value === undefined || value === null) + return ""; + return String(value); +}