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