cpvalente/ontime
1
0
Fork 0
mirror of https://github.com/cpvalente/ontime.git synced 2026-08-15 12:23:51 +00:00
Code Issues Packages Projects Releases 10 Wiki Activity

fix(cuesheet): restore keyboard focus flow for lazy text cells

Browse Source 
The lazy editor (Exp A) unmounts on exit, so the editor's own deferred
focus-return (useReactiveTextInput) targeted a detached node and focus was
lost to <body>, breaking table keyboard navigation after submit/escape.

Return focus to the parent cell deterministically in a layout effect once the
display element is back in the DOM (rather than a setTimeout that races the
next keypress), mirroring the time/duration cells.

Update the e2e tests to the lazy interaction model (click the cell to open the
on-demand editor, assert it unmounts on exit) — matching the existing timer
cell test. Verified end-to-end: 202-cuesheet (3/3) and 206-url-preset (5/5).
This commit is contained in:
Claude
2026-06-19 06:23:51 +00:00
parent d17a7e9321
commit dcdfd9ca85
3 changed files with 49 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableCell.tsx
+22 -7
View File
@@ -1,4 +1,4 @@
import { memo, useCallback, useEffect, useRef, useState } from 'react'; import { memo, useCallback, useLayoutEffect, useRef, useState } from 'react';
import MultiLineCell from './MultiLineCell'; import MultiLineCell from './MultiLineCell';
import SingleLineCell from './SingleLineCell'; import SingleLineCell from './SingleLineCell';
@@ -17,6 +17,10 @@ interface FocusableEditor {
select?: () => void; select?: () => void;
} }
interface FocusableDisplay {
focusParentElement: () => void;
}
/** /**
* Lazily mounts the text editor for a cell. * Lazily mounts the text editor for a cell.
* *
@@ -24,20 +28,31 @@ interface FocusableEditor {
* cell is expensive when many rows mount at once during virtualised scroll. While the cell is not * cell is expensive when many rows mount at once during virtualised scroll. While the cell is not
* being edited we render a lightweight, focusable display element and only mount the real editor * being edited we render a lightweight, focusable display element and only mount the real editor
* when the user clicks/focuses the cell — mirroring how the time/duration cells already behave. * when the user clicks/focuses the cell — mirroring how the time/duration cells already behave.
*
* On exit we return focus to the parent cell (through the display element, in a layout effect once
* it is back in the DOM) so the table keyboard navigation keeps working — the editor is unmounted
* by then, so we cannot rely on its own ref.
*/ */
function EditableCell({ initialValue, multiline, fieldId, fieldLabel, handleUpdate }: EditableCellProps) { function EditableCell({ initialValue, multiline, fieldId, fieldLabel, handleUpdate }: EditableCellProps) {
const [isEditing, setIsEditing] = useState(false); const [isEditing, setIsEditing] = useState(false);
const wasEditing = useRef(false);
const editorRef = useRef<FocusableEditor | null>(null); const editorRef = useRef<FocusableEditor | null>(null);
const displayRef = useRef<FocusableDisplay | null>(null);
// focus the editor once it mounts on entering edit mode useLayoutEffect(() => {
useEffect(() => {
if (isEditing) { if (isEditing) {
// focus the editor once it mounts on entering edit mode
editorRef.current?.focus(); editorRef.current?.focus();
editorRef.current?.select?.(); editorRef.current?.select?.();
} else if (wasEditing.current) {
// returning from edit: hand focus back to the cell so table keyboard navigation continues
displayRef.current?.focusParentElement();
} }
wasEditing.current = isEditing;
}, [isEditing]); }, [isEditing]);
const enterEdit = useCallback(() => setIsEditing(true), []); const enterEdit = useCallback(() => setIsEditing(true), []);
const exitEdit = useCallback(() => setIsEditing(false), []);
const onSubmit = useCallback( const onSubmit = useCallback(
(newValue: string) => { (newValue: string) => {
@@ -47,11 +62,10 @@ function EditableCell({ initialValue, multiline, fieldId, fieldLabel, handleUpda
[handleUpdate], [handleUpdate],
); );
const onCancel = useCallback(() => setIsEditing(false), []);
if (!isEditing) { if (!isEditing) {
return ( return (
<TextLikeInput <TextLikeInput
ref={displayRef}
onClick={enterEdit} onClick={enterEdit}
onFocus={enterEdit} onFocus={enterEdit}
multiline={multiline} multiline={multiline}
@@ -69,7 +83,7 @@ function EditableCell({ initialValue, multiline, fieldId, fieldLabel, handleUpda
fieldId={fieldId} fieldId={fieldId}
fieldLabel={fieldLabel} fieldLabel={fieldLabel}
handleUpdate={onSubmit} handleUpdate={onSubmit}
handleCancelUpdate={onCancel} handleCancelUpdate={exitEdit}
/> />
) : ( ) : (
<SingleLineCell <SingleLineCell
@@ -78,9 +92,10 @@ function EditableCell({ initialValue, multiline, fieldId, fieldLabel, handleUpda
fieldId={fieldId} fieldId={fieldId}
fieldLabel={fieldLabel} fieldLabel={fieldLabel}
handleUpdate={onSubmit} handleUpdate={onSubmit}
handleCancelUpdate={onCancel} handleCancelUpdate={exitEdit}
/> />
); );
} }
export default memo(EditableCell); export default memo(EditableCell);
e2e/tests/features/202-cuesheet.spec.ts
+23 -15
View File
@@ -57,46 +57,54 @@ test('cuesheet datagrid keeps keyboard focus flow while editing text cells', asy
const firstEvent = page.getByTestId('cuesheet-event').first(); const firstEvent = page.getByTestId('cuesheet-event').first();
await expect(firstEvent).toBeVisible(); await expect(firstEvent).toBeVisible();
const cueCell = firstEvent.getByTestId('cuesheet-cell-cue');
const titleCell = firstEvent.getByTestId('cuesheet-cell-title');
const noteCell = firstEvent.getByTestId('cuesheet-cell-note');
const cueEditor = firstEvent.getByTestId('cuesheet-editor-cue'); const cueEditor = firstEvent.getByTestId('cuesheet-editor-cue');
const titleEditor = firstEvent.getByTestId('cuesheet-editor-title'); const titleEditor = firstEvent.getByTestId('cuesheet-editor-title');
const noteEditor = firstEvent.getByTestId('cuesheet-editor-note'); const noteEditor = firstEvent.getByTestId('cuesheet-editor-note');
/** /**
* 1. focus a cell in the datagrid single line text * 1. clicking a single line text cell opens the editor (mounted on demand)
* submitting the data returns the focus to the parent * submitting with Enter closes the editor and returns focus to the parent cell
*/ */
await titleEditor.click(); await titleCell.click();
await expect(titleEditor).toBeFocused(); await expect(titleEditor).toBeFocused();
const updatedTitle = `focus-title-${Date.now()}`; const updatedTitle = `focus-title-${Date.now()}`;
await titleEditor.fill(updatedTitle); await titleEditor.fill(updatedTitle);
await titleEditor.press('Enter'); await titleEditor.press('Enter');
await expect(titleEditor).not.toBeFocused(); await expect(titleEditor).toHaveCount(0);
await expect(titleEditor).toHaveValue(updatedTitle); await expect(titleCell).toContainText(updatedTitle);
await expect(titleCell).toBeFocused();
/** /**
* 2. navigate and modify multiline text cell * 2. navigate to the multiline text cell with the keyboard and open it with Enter
* submitting works with ctrl/cmd + enter and the focus returns to the parent * submitting works with ctrl/cmd + enter and the focus returns to the parent
*/ */
await page.keyboard.press('ArrowRight'); await page.keyboard.press('ArrowRight');
await expect(noteCell).toBeFocused();
await page.keyboard.press('Enter'); await page.keyboard.press('Enter');
await expect(noteEditor).toBeFocused(); await expect(noteEditor).toBeFocused();
const updatedNote = `focus-note-${Date.now()}`; const updatedNote = `focus-note-${Date.now()}`;
await noteEditor.fill(updatedNote); await noteEditor.fill(updatedNote);
await noteEditor.press('ControlOrMeta+Enter'); await noteEditor.press('ControlOrMeta+Enter');
await expect(noteEditor).not.toBeFocused(); await expect(noteEditor).toHaveCount(0);
await expect(noteEditor).toHaveValue(updatedNote); await expect(noteCell).toContainText(updatedNote);
await expect(noteCell).toBeFocused();
/** /**
* 2. navigate and modify single line text cell again * 3. navigating back returns focus to the title cell
* pressing escape cancels the edit and the focus returns to the parent * opening the cue cell and pressing escape cancels the edit and reverts the value
*/ */
await page.keyboard.press('ArrowLeft'); await page.keyboard.press('ArrowLeft');
await page.keyboard.press('Enter'); await expect(titleCell).toBeFocused();
await expect(titleEditor).toBeFocused();
await cueCell.click();
await expect(cueEditor).toBeFocused();
const cueBeforeCancel = await cueEditor.inputValue(); const cueBeforeCancel = await cueEditor.inputValue();
await cueEditor.click();
await cueEditor.fill(`${cueBeforeCancel} temporary`); await cueEditor.fill(`${cueBeforeCancel} temporary`);
await cueEditor.press('Escape'); await cueEditor.press('Escape');
await expect(cueEditor).not.toBeFocused(); await expect(cueEditor).toHaveCount(0);
await expect(cueEditor).toHaveValue(cueBeforeCancel); await expect(cueCell).toContainText(cueBeforeCancel);
await expect(cueCell).toBeFocused();
}); });
e2e/tests/features/206-url-preset.spec.ts
+4 -3
View File
@@ -231,9 +231,10 @@ test.describe('Sharing from cuesheet', () => {
// Verify that the title is visible and editable // Verify that the title is visible and editable
await expect(page.getByTestId('cuesheet-event').getByRole('cell', { name: 'title' })).toBeVisible(); await expect(page.getByTestId('cuesheet-event').getByRole('cell', { name: 'title' })).toBeVisible();
const titleEditor = page.getByTestId('cuesheet-event').getByTestId('cuesheet-editor-title'); // the editor mounts on demand: clicking the cell opens it
await titleEditor.click(); const firstEvent = page.getByTestId('cuesheet-event').first();
await expect(titleEditor).toBeEditable(); await firstEvent.getByTestId('cuesheet-cell-title').click();
await expect(firstEvent.getByTestId('cuesheet-editor-title')).toBeEditable();
// other elements are not there // other elements are not there
await expect(page.getByRole('cell', { name: 'Duration' })).toBeHidden(); await expect(page.getByRole('cell', { name: 'Duration' })).toBeHidden();