fix(cuesheet): prevent timer cells from submitting on tab-out or escape (#2083)

This commit is contained in:
Carlos Valente
2026-06-07 12:59:18 +02:00
committed by GitHub
parent 5a3ea9f7d0
commit 199dea5df6
5 changed files with 98 additions and 7 deletions
@@ -3,8 +3,8 @@ import { ChangeEvent, KeyboardEvent, RefObject, useCallback, useEffect, useMemo,
interface UseReactiveTextInputReturn { interface UseReactiveTextInputReturn {
value: string; value: string;
onChange: (event: ChangeEvent) => void; onChange: (event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => void;
onBlur: (event: ChangeEvent) => void; onBlur: (event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => void;
onKeyDown: (event: KeyboardEvent<HTMLElement>) => void; onKeyDown: (event: KeyboardEvent<HTMLElement>) => void;
} }
@@ -15,7 +15,9 @@ export default function useReactiveTextInput(
options?: { options?: {
submitOnEnter?: boolean; submitOnEnter?: boolean;
submitOnCtrlEnter?: boolean; submitOnCtrlEnter?: boolean;
submitOnTab?: boolean;
onCancelUpdate?: () => void; onCancelUpdate?: () => void;
onTabCancel?: () => void;
allowSubmitSameValue?: boolean; allowSubmitSameValue?: boolean;
allowKeyboardNavigation?: boolean; allowKeyboardNavigation?: boolean;
}, },
@@ -23,6 +25,10 @@ export default function useReactiveTextInput(
const [text, setText] = useState<string>(initialText); const [text, setText] = useState<string>(initialText);
// track whether we are submitting via a submit key (eg enter) and avoid submitting again on blur // track whether we are submitting via a submit key (eg enter) and avoid submitting again on blur
const isKeyboardSubmitting = useRef(false); 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(() => { useEffect(() => {
if (typeof initialText === 'undefined') { if (typeof initialText === 'undefined') {
@@ -78,6 +84,7 @@ export default function useReactiveTextInput(
* @param {string} valueToSubmit * @param {string} valueToSubmit
*/ */
const handleEscape = useCallback(() => { const handleEscape = useCallback(() => {
isEscaping.current = true;
// No need to update if it hasn't changed // No need to update if it hasn't changed
setText(initialText); setText(initialText);
// force the text to be the initial value // force the text to be the initial value
@@ -141,16 +148,30 @@ export default function useReactiveTextInput(
event.stopPropagation(); 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); hotKeyHandler(event);
}; };
}, [handleEscape, handleSubmit, options?.submitOnCtrlEnter, options?.submitOnEnter, text]); }, [handleEscape, handleSubmit, options?.submitOnCtrlEnter, options?.submitOnEnter, options?.submitOnTab, text]);
return { return {
value: text, value: text,
onChange: (event: ChangeEvent) => handleChange((event.target as HTMLInputElement).value), onChange: (event) => handleChange(event.target.value),
onBlur: (event: ChangeEvent) => { onBlur: (event) => {
if (isTabbing.current) {
isTabbing.current = false;
(options?.onTabCancel ?? options?.onCancelUpdate)?.();
return;
}
if (isEscaping.current) {
isEscaping.current = false;
return;
}
if (!isKeyboardSubmitting.current) { if (!isKeyboardSubmitting.current) {
handleSubmit((event.target as HTMLInputElement).value); handleSubmit(event.target.value);
} }
}, },
onKeyDown: keyHandler, onKeyDown: keyHandler,
@@ -49,6 +49,10 @@ function DurationInput({
setIsEditing(false); setIsEditing(false);
setTimeout(() => textRef.current?.focusParentElement()); // Immediate timeout to ensure state change takes place first 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( const handleUpdate = useCallback(
(newValue: string) => { (newValue: string) => {
@@ -95,8 +99,10 @@ function DurationInput({
ref={inputRef} ref={inputRef}
initialValue={timeString} initialValue={timeString}
allowSubmitSameValue={!lockedValue} // if the value is not locked, submitting will lock the value allowSubmitSameValue={!lockedValue} // if the value is not locked, submitting will lock the value
submitOnTab={false}
handleUpdate={handleUpdate} handleUpdate={handleUpdate}
handleCancelUpdate={handleFakeBlur} handleCancelUpdate={handleFakeBlur}
handleTabCancel={handleTabOut}
/> />
) : ( ) : (
<TextLikeInput <TextLikeInput
@@ -8,13 +8,24 @@ interface SingleLineCellProps {
fieldId?: string; fieldId?: string;
fieldLabel?: string; fieldLabel?: string;
allowSubmitSameValue?: boolean; allowSubmitSameValue?: boolean;
submitOnTab?: boolean;
handleUpdate: (newValue: string) => void; handleUpdate: (newValue: string) => void;
handleCancelUpdate?: () => void; handleCancelUpdate?: () => void;
handleTabCancel?: () => void;
} }
const SingleLineCell = forwardRef( const SingleLineCell = forwardRef(
( (
{ initialValue, fieldId, fieldLabel, allowSubmitSameValue, handleUpdate, handleCancelUpdate }: SingleLineCellProps, {
initialValue,
fieldId,
fieldLabel,
allowSubmitSameValue,
submitOnTab,
handleUpdate,
handleCancelUpdate,
handleTabCancel,
}: SingleLineCellProps,
inputRef, inputRef,
) => { ) => {
const ref = useRef<HTMLInputElement | null>(null); const ref = useRef<HTMLInputElement | null>(null);
@@ -25,7 +36,9 @@ const SingleLineCell = forwardRef(
allowKeyboardNavigation: true, allowKeyboardNavigation: true,
submitOnEnter: true, // single line should submit on enter submitOnEnter: true, // single line should submit on enter
submitOnCtrlEnter: true, submitOnCtrlEnter: true,
submitOnTab,
onCancelUpdate: handleCancelUpdate, onCancelUpdate: handleCancelUpdate,
onTabCancel: handleTabCancel,
}); });
// expose a subset of the methods to the parent // expose a subset of the methods to the parent
@@ -50,6 +50,10 @@ function TimeInputDuration({
setIsEditing(false); setIsEditing(false);
setTimeout(() => textRef.current?.focusParentElement()); // Immediate timeout to ensure state change takes place first 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( const handleUpdate = useCallback(
(newValue: string) => { (newValue: string) => {
@@ -96,8 +100,10 @@ function TimeInputDuration({
ref={inputRef} ref={inputRef}
initialValue={timeString} initialValue={timeString}
allowSubmitSameValue={!lockedValue} // if the value is not locked, submitting will lock the value allowSubmitSameValue={!lockedValue} // if the value is not locked, submitting will lock the value
submitOnTab={false}
handleUpdate={handleUpdate} handleUpdate={handleUpdate}
handleCancelUpdate={handleFakeBlur} handleCancelUpdate={handleFakeBlur}
handleTabCancel={handleTabOut}
/> />
) : ( ) : (
<TextLikeInput <TextLikeInput
+45
View File
@@ -6,6 +6,51 @@ test('cuesheet displays events', async ({ page }) => {
await expect(page.getByTestId('cuesheet-event').first()).toBeVisible(); 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 }) => { test('cuesheet datagrid keeps keyboard focus flow while editing text cells', async ({ page }) => {
await page.goto('/cuesheet'); await page.goto('/cuesheet');