From 4f19ee32387d24f575649da8b7b199a6a4d09bb2 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 26 Aug 2026 09:04:01 -0400 Subject: [PATCH] fix(web): insert the enlarge newline at the caret, not at the end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enter appended its newline to the end of the value regardless of where the caret was, so pressing it in the middle of `abc|def` produced `abcdef` with a trailing blank line rather than splitting the text, and a selected range was kept rather than replaced. That is a rewritten value, not just a misplaced caret, and no editor behaves that way. Split at the field's own selection and replace any selected range, then mount the text area with the caret just after the inserted newline — EnlargedStringField takes an explicit caretAt for this. Pointer activation passes none and keeps falling back to end-of-value, since a click carries no meaningful position. The maxLength check now prices the resulting value, so replacing a selection can free the room the newline needs. The two maxLength tests were asserting against a stub onChange, which froze the value and made "it did not change" pass whatever the component did. They now run on a seeded stateful harness, with a no-maxLength control so the constraint is shown doing the work. Signed-off-by: cliffhall --- .../groups/SchemaForm/SchemaForm.test.tsx | 97 +++++++++++++++++-- .../groups/SchemaForm/SchemaForm.tsx | 76 +++++++++++---- 2 files changed, 142 insertions(+), 31 deletions(-) diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index db245900..992bf4ca 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -1999,6 +1999,25 @@ describe("SchemaForm enlarge keyboard access (#2138)", () => { ); } + // Seeds a value and holds it in real state. A stub `onChange` would leave the + // field's value frozen, which makes "the value did not change" assertions + // pass whatever the component does. + function SeededHarness({ maxLength }: { maxLength?: number }) { + const [values, setValues] = useState>({ + note: "abc", + }); + return ( + + ); + } + const noteField = () => screen.getByRole("textbox", { name: /Note/ }) as HTMLTextAreaElement; @@ -2138,20 +2157,55 @@ describe("SchemaForm enlarge keyboard access (#2138)", () => { expect(noteField().selectionStart).toBe("before\n".length); }); + // The newline goes in where the caret is, exactly as it would in a text area. + // Appending at the end instead silently rewrites the value whenever the caret + // was not already there — `abc|def` would become `abcdef` with a trailing + // blank line (#2139 review). + it("inserts the newline at the caret, not at the end", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.type(noteField(), "abcdef"); + noteField().setSelectionRange(3, 3); + fireEvent.keyDown(noteField(), { key: "Enter" }); + + expect(noteField().value).toBe("abc\ndef"); + expect(noteField().selectionStart).toBe(4); + }); + + // And a selected range is replaced, not kept — again matching what typing + // Enter into a real text area does. + it("replaces a selected range with the newline", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.type(noteField(), "abcdef"); + noteField().setSelectionRange(3, 6); + fireEvent.keyDown(noteField(), { key: "Enter" }); + + expect(noteField().value).toBe("abc\n"); + expect(noteField().selectionStart).toBe(4); + }); + + // Splitting at the very start is the boundary the slice arithmetic is most + // likely to get wrong. + it("splits at the start of the value", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.type(noteField(), "abc"); + noteField().setSelectionRange(0, 0); + fireEvent.keyDown(noteField(), { key: "Enter" }); + + expect(noteField().value).toBe("\nabc"); + expect(noteField().selectionStart).toBe(1); + }); + // A newline is a character, so a field with no room for one is enlarged // without it rather than pushed past a constraint its schema states. it("enlarges without a newline when the field is at its maxLength", async () => { const user = userEvent.setup(); - renderWithMantine( - , - ); + renderWithMantine(); noteField().focus(); await user.keyboard("{Enter}"); @@ -2160,6 +2214,29 @@ describe("SchemaForm enlarge keyboard access (#2138)", () => { expect(noteField().value).toBe("abc"); }); + // Replacing a selection frees room, so the same full field does take the + // newline when the keystroke is removing something to make space for it. + it("enters the newline at maxLength when a selection makes room", () => { + renderWithMantine(); + + noteField().setSelectionRange(1, 3); + fireEvent.keyDown(noteField(), { key: "Enter" }); + + expect(noteField().value).toBe("a\n"); + }); + + // The control: the same seeded field with no maxLength does take the newline, + // so the test above is showing the constraint at work rather than a field + // that never accepts one. + it("enters the newline when the field has no maxLength", () => { + renderWithMantine(); + + noteField().setSelectionRange(3, 3); + fireEvent.keyDown(noteField(), { key: "Enter" }); + + expect(noteField().value).toBe("abc\n"); + }); + // An empty field still has room, so the newline is entered there too. it("enters a newline in an empty field", async () => { const user = userEvent.setup(); diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index ae149861..6ab6e7c5 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -65,16 +65,23 @@ const MultilineStringInput = Textarea.withProps({ * A string field that has just been enlarged (#2042). * * Exists only to take focus on mount. This component mounts as a direct - * consequence of the user activating the enlarge button — which unmounts in the - * same commit, taking the focused element with it. Without this, a keyboard user - * who activates it is left with focus on the document body, and the next Tab - * restarts from the top of the page rather than continuing through the form. + * consequence of the user enlarging the field — and whichever control they used + * unmounts in the same commit, taking the focused element with it. Without this, + * the user is left with focus on the document body, and the next Tab restarts + * from the top of the page rather than continuing through the form. * - * The caret is placed at the end of whatever was already typed, since focusing a - * pre-filled text control does not agree across browsers on where it lands, and - * the one answer that is never right is "before the text the user just wrote". + * `caretAt` says where the caret goes. The keyboard route passes one, because + * Enter inserts a newline at the user's selection and the caret belongs just + * after it (#2138). Pointer activation passes none: a click carries no + * meaningful position, so the caret falls to the end of whatever was already + * typed — focusing a pre-filled text control does not agree across browsers on + * where it lands, and the one answer that is never right is "before the text + * the user just wrote". */ -function EnlargedStringField(props: TextareaProps) { +function EnlargedStringField({ + caretAt, + ...props +}: TextareaProps & { caretAt?: number }) { const inputRef = useRef(null); // Layout, not passive: a passive effect runs after paint, so the browser would @@ -85,8 +92,9 @@ function EnlargedStringField(props: TextareaProps) { /* v8 ignore next -- an effect runs after mount, so the ref is always set */ if (!node) return; node.focus(); - node.setSelectionRange(node.value.length, node.value.length); - }, []); + const caret = caretAt ?? node.value.length; + node.setSelectionRange(caret, caret); + }, [caretAt]); return ; } @@ -590,7 +598,17 @@ export function SchemaForm({ // *name*, so it must not carry across to another entity's same-named field — // the same reasoning `resetKey` documents for the number field's draft. Reset // during render rather than in an effect so no frame paints the wrong shape. - useValueChange(resetKey, () => setEnlargedFields(new Set())); + // Where the caret goes when a field's text area mounts, for the fields + // enlarged by Enter (#2138). Absent for a field enlarged by the button, which + // carries no position — see EnlargedStringField. + const [enlargeCarets, setEnlargeCarets] = useState< + Readonly> + >({}); + + useValueChange(resetKey, () => { + setEnlargedFields(new Set()); + setEnlargeCarets({}); + }); // Stable so a field's reporting effect subscribes once, not per render. The // updater returns the previous set unchanged when nothing moved, which is @@ -712,6 +730,7 @@ export function SchemaForm({ @@ -726,17 +745,32 @@ export function SchemaForm({ // where it was, so the next thing typed runs on from the last word — the // user pressed the key that means "new line" and got a reshaped box. // - // The newline lands at the end rather than at the caret because that is - // where EnlargedStringField puts the caret regardless (see its comment); - // going anywhere else would separate the newline from the text that - // follows it. A field already at its maxLength is enlarged without one, - // since the alternative is breaching a constraint the schema states. - const enlargeWithNewline = () => { + // The newline goes in at the field's own selection, exactly as it would + // in a text area: split at the caret, and replace a selected range rather + // than keeping it. Appending at the end instead would silently rewrite + // the value whenever the caret was not already there — pressing Enter in + // the middle of `abc|def` would produce `abcdef` with a trailing blank + // line, which is not what any editor does and not what was asked for. + // + // A field with no room left for the newline is enlarged without one, + // since the alternative is breaching a maxLength the schema states. + const enlargeWithNewline = (input: HTMLInputElement) => { const current = (rawValue as string) ?? ""; - const room = + // A control that cannot report a selection (`selectionStart` is null + // for some input types) is treated as a caret at the end. + const start = input.selectionStart ?? current.length; + const end = input.selectionEnd ?? start; + const next = `${current.slice(0, start)}\n${current.slice(end)}`; + const fits = fieldSchema.maxLength === undefined || - current.length < fieldSchema.maxLength; - if (room) handleFieldChange(fieldName, `${current}\n`); + next.length <= fieldSchema.maxLength; + if (fits) { + handleFieldChange(fieldName, next); + setEnlargeCarets((previous) => ({ + ...previous, + [fieldName]: start + 1, + })); + } enlarge(); }; @@ -775,7 +809,7 @@ export function SchemaForm({ return; } event.preventDefault(); - enlargeWithNewline(); + enlargeWithNewline(event.currentTarget); }} // Announces that the field carries a shortcut at all. A keyboard user // no longer meets the button by tabbing, so without this the binding