From 7ec0be0d12bf547daa548d08603c9c6e95a5a5f1 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 26 Aug 2026 09:18:46 -0400 Subject: [PATCH] refactor(web): lift the enlarge key handler out of JSX The handler carried four guards, a preventDefault and a state transition inline in the TextInput's props, which is what AGENTS.md rules out ("NEVER use inline code; instead extract to functions in the same file"). Name it handleEnlargeKeyDown and put it beside enlargeWithNewline, where the comment can enumerate why each condition is there rather than sitting between two JSX props. No behavior change. Signed-off-by: cliffhall --- .../groups/SchemaForm/SchemaForm.tsx | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 6ab6e7c5..1fdd748b 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -19,6 +19,7 @@ import { useRef, useState, type ChangeEvent, + type KeyboardEvent, } from "react"; import { ClearButton } from "../../elements/ClearButton/ClearButton"; import { EnlargeButton } from "../../elements/EnlargeButton/EnlargeButton"; @@ -774,43 +775,46 @@ export function SchemaForm({ enlarge(); }; + // The keyboard's way into multiline mode, now that the enlarge button is + // out of the tab order (#2138). Enter is inert in this field — nothing + // renders SchemaForm inside a `
`, so there is no implicit + // submission to displace — and it is the very key a user presses trying + // to enter the newline a single-line input swallows, which is what #2042 + // exists to fix. So the gesture that fails is the one that enlarges, + // rather than a shortcut nobody would guess. + // + // Every condition below is load-bearing; none is incidental: + // + // - Shift+Enter is the other newline gesture, so it enlarges too. + // - `isComposing` is the IME guard. Enter is also how a Japanese, Chinese + // or Korean input method commits the candidate being composed, and that + // keystroke means "accept this word", not "new line". Acting on it + // would enlarge the field and insert a stray newline every time such a + // user finished a word, making the field unusable for them. + // - The Ctrl/Cmd/Alt chords are deliberately left alone: those read as + // "submit" in a form, and a consumer binding one (run the tool) must + // not be overridden into enlarging a field instead. + const handleEnlargeKeyDown = (event: KeyboardEvent) => { + if ( + event.key !== "Enter" || + event.nativeEvent.isComposing || + event.ctrlKey || + event.metaKey || + event.altKey + ) { + return; + } + event.preventDefault(); + enlargeWithNewline(event.currentTarget); + }; + return ( `, so there is no - // implicit submission to displace — and it is the very key a user - // presses trying to enter the newline a single-line input swallows, - // which is what #2042 exists to fix. So the gesture that fails is the - // one that enlarges, rather than a shortcut nobody would guess. - onKeyDown={(event) => { - // Shift+Enter is the other newline gesture, so it enlarges too. The - // Ctrl/Cmd/Alt chords are deliberately left alone: those read as - // "submit" in a form, and a consumer binding one (run the tool) - // must not be overridden into enlarging a field instead. - // - // `isComposing` guards the IME case: Enter is also how a Japanese, - // Chinese or Korean input method commits the candidate being - // composed. That keystroke means "accept this word", not "new - // line", so acting on it would enlarge the field and insert a - // newline every time such a user finished a word — the shortcut - // would make the field unusable for them. - if ( - event.key !== "Enter" || - event.nativeEvent.isComposing || - event.ctrlKey || - event.metaKey || - event.altKey - ) { - return; - } - event.preventDefault(); - enlargeWithNewline(event.currentTarget); - }} + onKeyDown={handleEnlargeKeyDown} // Announces that the field carries a shortcut at all. A keyboard user // no longer meets the button by tabbing, so without this the binding // is undiscoverable rather than merely unlabelled.