diff --git a/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableCell.tsx b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableCell.tsx index 2c7541e2e..5eb8a53d3 100644 --- a/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableCell.tsx +++ b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/EditableCell.tsx @@ -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 SingleLineCell from './SingleLineCell'; @@ -17,6 +17,10 @@ interface FocusableEditor { select?: () => void; } +interface FocusableDisplay { + focusParentElement: () => void; +} + /** * 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 * 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. + * + * 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) { const [isEditing, setIsEditing] = useState(false); + const wasEditing = useRef(false); const editorRef = useRef(null); + const displayRef = useRef(null); - // focus the editor once it mounts on entering edit mode - useEffect(() => { + useLayoutEffect(() => { if (isEditing) { + // focus the editor once it mounts on entering edit mode editorRef.current?.focus(); 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]); const enterEdit = useCallback(() => setIsEditing(true), []); + const exitEdit = useCallback(() => setIsEditing(false), []); const onSubmit = useCallback( (newValue: string) => { @@ -47,11 +62,10 @@ function EditableCell({ initialValue, multiline, fieldId, fieldLabel, handleUpda [handleUpdate], ); - const onCancel = useCallback(() => setIsEditing(false), []); - if (!isEditing) { return ( ) : ( ); } export default memo(EditableCell); + diff --git a/e2e/tests/features/202-cuesheet.spec.ts b/e2e/tests/features/202-cuesheet.spec.ts index ea1b76c51..bb186c6df 100644 --- a/e2e/tests/features/202-cuesheet.spec.ts +++ b/e2e/tests/features/202-cuesheet.spec.ts @@ -57,46 +57,54 @@ test('cuesheet datagrid keeps keyboard focus flow while editing text cells', asy const firstEvent = page.getByTestId('cuesheet-event').first(); 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 titleEditor = firstEvent.getByTestId('cuesheet-editor-title'); const noteEditor = firstEvent.getByTestId('cuesheet-editor-note'); /** - * 1. focus a cell in the datagrid single line text - * submitting the data returns the focus to the parent + * 1. clicking a single line text cell opens the editor (mounted on demand) + * submitting with Enter closes the editor and returns focus to the parent cell */ - await titleEditor.click(); + await titleCell.click(); await expect(titleEditor).toBeFocused(); const updatedTitle = `focus-title-${Date.now()}`; await titleEditor.fill(updatedTitle); await titleEditor.press('Enter'); - await expect(titleEditor).not.toBeFocused(); - await expect(titleEditor).toHaveValue(updatedTitle); + await expect(titleEditor).toHaveCount(0); + 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 */ await page.keyboard.press('ArrowRight'); + await expect(noteCell).toBeFocused(); await page.keyboard.press('Enter'); await expect(noteEditor).toBeFocused(); const updatedNote = `focus-note-${Date.now()}`; await noteEditor.fill(updatedNote); await noteEditor.press('ControlOrMeta+Enter'); - await expect(noteEditor).not.toBeFocused(); - await expect(noteEditor).toHaveValue(updatedNote); + await expect(noteEditor).toHaveCount(0); + await expect(noteCell).toContainText(updatedNote); + await expect(noteCell).toBeFocused(); /** - * 2. navigate and modify single line text cell again - * pressing escape cancels the edit and the focus returns to the parent + * 3. navigating back returns focus to the title cell + * opening the cue cell and pressing escape cancels the edit and reverts the value */ await page.keyboard.press('ArrowLeft'); - await page.keyboard.press('Enter'); - await expect(titleEditor).toBeFocused(); + await expect(titleCell).toBeFocused(); + + await cueCell.click(); + await expect(cueEditor).toBeFocused(); const cueBeforeCancel = await cueEditor.inputValue(); - await cueEditor.click(); await cueEditor.fill(`${cueBeforeCancel} temporary`); await cueEditor.press('Escape'); - await expect(cueEditor).not.toBeFocused(); - await expect(cueEditor).toHaveValue(cueBeforeCancel); + await expect(cueEditor).toHaveCount(0); + await expect(cueCell).toContainText(cueBeforeCancel); + await expect(cueCell).toBeFocused(); }); diff --git a/e2e/tests/features/206-url-preset.spec.ts b/e2e/tests/features/206-url-preset.spec.ts index 5a5edcade..ca9f3eb8c 100644 --- a/e2e/tests/features/206-url-preset.spec.ts +++ b/e2e/tests/features/206-url-preset.spec.ts @@ -231,9 +231,10 @@ test.describe('Sharing from cuesheet', () => { // Verify that the title is visible and editable await expect(page.getByTestId('cuesheet-event').getByRole('cell', { name: 'title' })).toBeVisible(); - const titleEditor = page.getByTestId('cuesheet-event').getByTestId('cuesheet-editor-title'); - await titleEditor.click(); - await expect(titleEditor).toBeEditable(); + // the editor mounts on demand: clicking the cell opens it + const firstEvent = page.getByTestId('cuesheet-event').first(); + await firstEvent.getByTestId('cuesheet-cell-title').click(); + await expect(firstEvent.getByTestId('cuesheet-editor-title')).toBeEditable(); // other elements are not there await expect(page.getByRole('cell', { name: 'Duration' })).toBeHidden();