From 199dea5df63ad0164559f936a5adf6f45ffb2bee Mon Sep 17 00:00:00 2001 From: Carlos Valente <34649812+cpvalente@users.noreply.github.com> Date: Sun, 7 Jun 2026 12:59:18 +0200 Subject: [PATCH] fix(cuesheet): prevent timer cells from submitting on tab-out or escape (#2083) --- .../input/text-input/useReactiveTextInput.tsx | 33 +++++++++++--- .../cuesheet-table-elements/DurationInput.tsx | 6 +++ .../SingleLineCell.tsx | 15 ++++++- .../cuesheet-table-elements/TimeInput.tsx | 6 +++ e2e/tests/features/202-cuesheet.spec.ts | 45 +++++++++++++++++++ 5 files changed, 98 insertions(+), 7 deletions(-) diff --git a/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx b/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx index 8558a2c29..59f314ce7 100644 --- a/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx +++ b/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx @@ -3,8 +3,8 @@ import { ChangeEvent, KeyboardEvent, RefObject, useCallback, useEffect, useMemo, interface UseReactiveTextInputReturn { value: string; - onChange: (event: ChangeEvent) => void; - onBlur: (event: ChangeEvent) => void; + onChange: (event: ChangeEvent) => void; + onBlur: (event: ChangeEvent) => void; onKeyDown: (event: KeyboardEvent) => void; } @@ -15,7 +15,9 @@ export default function useReactiveTextInput( options?: { submitOnEnter?: boolean; submitOnCtrlEnter?: boolean; + submitOnTab?: boolean; onCancelUpdate?: () => void; + onTabCancel?: () => void; allowSubmitSameValue?: boolean; allowKeyboardNavigation?: boolean; }, @@ -23,6 +25,10 @@ export default function useReactiveTextInput( const [text, setText] = useState(initialText); // track whether we are submitting via a submit key (eg enter) and avoid submitting again on blur const isKeyboardSubmitting = useRef(false); + // track escape to prevent the subsequent blur from submitting + const isEscaping = useRef(false); + // track tab-out to prevent blur from submitting (used by timer cells) + const isTabbing = useRef(false); useEffect(() => { if (typeof initialText === 'undefined') { @@ -78,6 +84,7 @@ export default function useReactiveTextInput( * @param {string} valueToSubmit */ const handleEscape = useCallback(() => { + isEscaping.current = true; // No need to update if it hasn't changed setText(initialText); // force the text to be the initial value @@ -141,16 +148,30 @@ export default function useReactiveTextInput( event.stopPropagation(); } + // for cells that opt out of tab-submit, track tab-out without preventing navigation + if (event.key === 'Tab' && options?.submitOnTab === false) { + isTabbing.current = true; + } + hotKeyHandler(event); }; - }, [handleEscape, handleSubmit, options?.submitOnCtrlEnter, options?.submitOnEnter, text]); + }, [handleEscape, handleSubmit, options?.submitOnCtrlEnter, options?.submitOnEnter, options?.submitOnTab, text]); return { value: text, - onChange: (event: ChangeEvent) => handleChange((event.target as HTMLInputElement).value), - onBlur: (event: ChangeEvent) => { + onChange: (event) => handleChange(event.target.value), + onBlur: (event) => { + if (isTabbing.current) { + isTabbing.current = false; + (options?.onTabCancel ?? options?.onCancelUpdate)?.(); + return; + } + if (isEscaping.current) { + isEscaping.current = false; + return; + } if (!isKeyboardSubmitting.current) { - handleSubmit((event.target as HTMLInputElement).value); + handleSubmit(event.target.value); } }, onKeyDown: keyHandler, diff --git a/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/DurationInput.tsx b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/DurationInput.tsx index 4882da9d3..de5f84997 100644 --- a/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/DurationInput.tsx +++ b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/DurationInput.tsx @@ -49,6 +49,10 @@ function DurationInput({ setIsEditing(false); setTimeout(() => textRef.current?.focusParentElement()); // Immediate timeout to ensure state change takes place first }; + const handleTabOut = useCallback(() => { + setIsEditing(false); + // Tab moves focus naturally; do not steal it back to parent + }, []); const handleUpdate = useCallback( (newValue: string) => { @@ -95,8 +99,10 @@ function DurationInput({ ref={inputRef} initialValue={timeString} allowSubmitSameValue={!lockedValue} // if the value is not locked, submitting will lock the value + submitOnTab={false} handleUpdate={handleUpdate} handleCancelUpdate={handleFakeBlur} + handleTabCancel={handleTabOut} /> ) : ( void; handleCancelUpdate?: () => void; + handleTabCancel?: () => void; } const SingleLineCell = forwardRef( ( - { initialValue, fieldId, fieldLabel, allowSubmitSameValue, handleUpdate, handleCancelUpdate }: SingleLineCellProps, + { + initialValue, + fieldId, + fieldLabel, + allowSubmitSameValue, + submitOnTab, + handleUpdate, + handleCancelUpdate, + handleTabCancel, + }: SingleLineCellProps, inputRef, ) => { const ref = useRef(null); @@ -25,7 +36,9 @@ const SingleLineCell = forwardRef( allowKeyboardNavigation: true, submitOnEnter: true, // single line should submit on enter submitOnCtrlEnter: true, + submitOnTab, onCancelUpdate: handleCancelUpdate, + onTabCancel: handleTabCancel, }); // expose a subset of the methods to the parent diff --git a/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/TimeInput.tsx b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/TimeInput.tsx index 29e03f898..9a4c19392 100644 --- a/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/TimeInput.tsx +++ b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/TimeInput.tsx @@ -50,6 +50,10 @@ function TimeInputDuration({ setIsEditing(false); setTimeout(() => textRef.current?.focusParentElement()); // Immediate timeout to ensure state change takes place first }; + const handleTabOut = useCallback(() => { + setIsEditing(false); + // Tab moves focus naturally; do not steal it back to parent + }, []); const handleUpdate = useCallback( (newValue: string) => { @@ -96,8 +100,10 @@ function TimeInputDuration({ ref={inputRef} initialValue={timeString} allowSubmitSameValue={!lockedValue} // if the value is not locked, submitting will lock the value + submitOnTab={false} handleUpdate={handleUpdate} handleCancelUpdate={handleFakeBlur} + handleTabCancel={handleTabOut} /> ) : ( { await expect(page.getByTestId('cuesheet-event').first()).toBeVisible(); }); +test('cuesheet datagrid does not submit timer cells on tab-out or escape', async ({ page }) => { + await page.goto('/cuesheet'); + + const firstEvent = page.getByTestId('cuesheet-event').first(); + await expect(firstEvent).toBeVisible(); + + const durationCell = firstEvent.getByTestId('cuesheet-cell-duration'); + + /** + * 1. Tab out of a timer cell should not submit the typed value + */ + await durationCell.click(); + const durationInput = durationCell.locator('input'); + await expect(durationInput).toBeVisible(); + const originalDuration = await durationInput.inputValue(); + + await durationInput.fill('01:00:00'); + await durationInput.press('Tab'); + + // cell should exit edit mode without submitting + await expect(durationInput).not.toBeVisible(); + + // re-enter edit mode: original value should be unchanged + await durationCell.click(); + await expect(durationCell.locator('input')).toHaveValue(originalDuration); + await durationCell.locator('input').press('Escape'); + + /** + * 2. Escape on a timer cell should not submit, should revert to original value + */ + await durationCell.click(); + await expect(durationCell.locator('input')).toBeVisible(); + + await durationCell.locator('input').fill('02:00:00'); + await durationCell.locator('input').press('Escape'); + + // cell should exit edit mode without submitting + await expect(durationCell.locator('input')).not.toBeVisible(); + + // re-enter edit mode: original value should be unchanged + await durationCell.click(); + await expect(durationCell.locator('input')).toHaveValue(originalDuration); + await durationCell.locator('input').press('Escape'); +}); + test('cuesheet datagrid keeps keyboard focus flow while editing text cells', async ({ page }) => { await page.goto('/cuesheet');