fix(cuesheet): prevent timer cells from submitting on tab-out or escape#2083
fix(cuesheet): prevent timer cells from submitting on tab-out or escape#2083cpvalente wants to merge 1 commit into
Conversation
Tab-out and Escape on time inputs (start, end, duration) were triggering a submit that destructively changed the timer strategy. Added isEscaping and isTabbing guards to useReactiveTextInput; timer cells opt out of tab-submit via submitOnTab=false and supply a handleTabCancel that exits edit mode without stealing focus. E2e tests added first (TDD). https://claude.ai/code/session_01LJnSMxNiMz8DWZ8eXH8krJ
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the cuesheet timer-cell editing behavior so that leaving a time field via Tab or Escape does not submit the in-progress value, aligning cuesheet keyboard navigation with expected spreadsheet-like interactions.
Changes:
- Added a Playwright e2e regression test covering timer-cell Tab-out and Escape behavior.
- Extended
useReactiveTextInputwith Tab/Escape tracking to avoid blur-driven submission in those cases. - Wired timer inputs (
DurationInput,TimeInputDuration) throughSingleLineCellto opt out of Tab-submit and to cancel cleanly on Tab-out.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| e2e/tests/features/202-cuesheet.spec.ts | Adds e2e coverage ensuring timer cells don’t submit on Tab-out or Escape. |
| apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/TimeInput.tsx | Disables Tab-submit for time inputs and exits edit mode on Tab-out without refocusing parent. |
| apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/DurationInput.tsx | Disables Tab-submit for duration inputs and exits edit mode on Tab-out without refocusing parent. |
| apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/SingleLineCell.tsx | Plumbs submitOnTab + handleTabCancel into the reactive input hook. |
| apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx | Implements Tab/Escape flags to prevent blur-triggered submits in those flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| value: text, | ||
| onChange: (event: ChangeEvent) => handleChange((event.target as HTMLInputElement).value), | ||
| onBlur: (event: ChangeEvent) => { | ||
| 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); | ||
| } |
| setIsEditing(false); | ||
| setTimeout(() => textRef.current?.focusParentElement()); // Immediate timeout to ensure state change takes place first | ||
| }; | ||
| const handleTabOut = useCallback(() => { |
There was a problem hiding this comment.
I there a reason for this to be a callback when handleFakeBlur is not
| interface UseReactiveTextInputReturn { | ||
| value: string; | ||
| onChange: (event: ChangeEvent) => void; | ||
| onBlur: (event: ChangeEvent) => void; | ||
| onKeyDown: (event: KeyboardEvent<HTMLElement>) => void; | ||
| } |
There was a problem hiding this comment.
| interface UseReactiveTextInputReturn { | |
| value: string; | |
| onChange: (event: ChangeEvent<HTMLInputElement |HTMLTextAreaElement>) => void; | |
| onBlur: (event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => void; | |
| onKeyDown: (event: KeyboardEvent<HTMLElement>) => void; | |
| } |
| // for cells that opt out of tab-submit, track tab-out without preventing navigation | ||
| if (event.key === 'Tab' && options?.submitOnTab === false) { | ||
| isTabbing.current = true; | ||
| } |
There was a problem hiding this comment.
| 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.value); | |
| } |
Disable submit on cuesheet when tabbing through time fields
Pressing Escape should in no case submit a value