Lite: misc#13330
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Lite workspace UX by improving inline editors’ exit behavior, clarifying panel focus, and refining hover/selection visuals.
Changes:
- Switch inline commit/branch editors to a shared
submithandler and attempt to auto-submit on blur. - Add panel “focus” handling in the preview layout (state-driven, click-to-focus) and style focused panels.
- Refine diff hunk hover/selected styling and adjust native context menu positioning to avoid accidental hover.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/lite/ui/src/routes/project/$id/workspace/route.tsx | Updates inline editors to submit/exit via a shared handler and adds blur-to-submit behavior. |
| apps/lite/ui/src/routes/project/$id/workspace/route.module.css | Changes preview hunk styling from outlines to bordered hover/selected states with radius. |
| apps/lite/ui/src/routes/project/$id/shared.tsx | Adjusts getRelative behavior for navigation to stop at bounds instead of wrapping. |
| apps/lite/ui/src/routes/project/$id/shared.module.css | Adds focused panel styling via a bottom border. |
| apps/lite/ui/src/routes/project/$id/ProjectPreviewLayout.tsx | Tracks panel focus from layout state and updates focus on pointer interaction. |
| apps/lite/ui/src/native-menu.ts | Nudges cursor-based context menu positioning to avoid pre-hovering the first item. |
| const submit = (event: React.SyntheticEvent<HTMLFormElement>) => { | ||
| event.preventDefault(); | ||
| const formData = new FormData(event.currentTarget); | ||
| onExit(); | ||
| onSubmit(formData.get("branchName") as string); | ||
| }; | ||
| return ( | ||
| <form ref={formRef} className={styles.editorForm} onSubmit={submit} onBlur={submit}> |
There was a problem hiding this comment.
Same issue as the commit-message editor: onBlur={submit} combined with the Escape-cancel shortcut (which unmounts the editor) can trigger a blur-driven submit during cancel/unmount, and can also lead to duplicate submissions when saving via Enter. Recommend separating blur behavior from submit (or guarding blur submits) so cancel cannot persist changes and the submit handler can’t run twice.
| const submit = (event: React.SyntheticEvent<HTMLFormElement>) => { | |
| event.preventDefault(); | |
| const formData = new FormData(event.currentTarget); | |
| onExit(); | |
| onSubmit(formData.get("branchName") as string); | |
| }; | |
| return ( | |
| <form ref={formRef} className={styles.editorForm} onSubmit={submit} onBlur={submit}> | |
| const localFormRef = useRef<HTMLFormElement | null>(null); | |
| const hasSubmittedRef = useRef(false); | |
| const assignFormRef = (el: HTMLFormElement | null) => { | |
| localFormRef.current = el; | |
| if (!formRef) return; | |
| if (typeof formRef === "function") { | |
| formRef(el); | |
| return; | |
| } | |
| formRef.current = el; | |
| }; | |
| const handleSubmit = (event: React.SyntheticEvent<HTMLFormElement>) => { | |
| event.preventDefault(); | |
| if (hasSubmittedRef.current) return; | |
| hasSubmittedRef.current = true; | |
| const formData = new FormData(event.currentTarget); | |
| onExit(); | |
| onSubmit(formData.get("branchName") as string); | |
| }; | |
| const handleBlur = (event: React.FocusEvent<HTMLFormElement>) => { | |
| const nextFocusedElement = event.relatedTarget; | |
| if ( | |
| nextFocusedElement instanceof Node && | |
| event.currentTarget.contains(nextFocusedElement) | |
| ) { | |
| return; | |
| } | |
| const form = event.currentTarget; | |
| window.setTimeout(() => { | |
| if (hasSubmittedRef.current) return; | |
| if (localFormRef.current !== form || !form.isConnected) return; | |
| hasSubmittedRef.current = true; | |
| const formData = new FormData(form); | |
| onExit(); | |
| onSubmit(formData.get("branchName") as string); | |
| }, 0); | |
| }; | |
| return ( | |
| <form | |
| ref={assignFormRef} | |
| className={styles.editorForm} | |
| onSubmit={handleSubmit} | |
| onBlur={handleBlur} | |
| > |
| const submit = (event: React.SyntheticEvent<HTMLFormElement>) => { | ||
| event.preventDefault(); | ||
| const formData = new FormData(event.currentTarget); | ||
| onExit(); | ||
| onSubmit(formData.get("message") as string); | ||
| }; | ||
| return ( | ||
| <form ref={formRef} className={styles.editorForm} onSubmit={submit} onBlur={submit}> |
There was a problem hiding this comment.
The form is wired to call submit on both onSubmit and onBlur. Because the cancel shortcut dispatches exitMode (which unmounts this editor), the focused textarea will blur during cancel/unmount and onBlur will still run, causing an unintended save even when the user pressed Escape. This can also double-submit on Enter (submit -> onExit unmount -> blur -> submit again). Consider removing the form-level onBlur auto-submit, or make blur handling distinct (e.g., only exit without saving on blur, or guard with a ref/flag so blur doesn’t run during cancel/unmount, and ensure the submit path is invoked only once).
| const submit = (event: React.SyntheticEvent<HTMLFormElement>) => { | |
| event.preventDefault(); | |
| const formData = new FormData(event.currentTarget); | |
| onExit(); | |
| onSubmit(formData.get("message") as string); | |
| }; | |
| return ( | |
| <form ref={formRef} className={styles.editorForm} onSubmit={submit} onBlur={submit}> | |
| const isExitingRef = useRef(false); | |
| useLayoutEffect(() => { | |
| return () => { | |
| isExitingRef.current = true; | |
| }; | |
| }, []); | |
| const submitForm = (form: HTMLFormElement) => { | |
| if (isExitingRef.current) return; | |
| isExitingRef.current = true; | |
| const formData = new FormData(form); | |
| onExit(); | |
| onSubmit(formData.get("message") as string); | |
| }; | |
| const handleSubmit = (event: React.SyntheticEvent<HTMLFormElement>) => { | |
| event.preventDefault(); | |
| submitForm(event.currentTarget); | |
| }; | |
| const handleBlur = (event: React.FocusEvent<HTMLFormElement>) => { | |
| if (isExitingRef.current) return; | |
| const nextFocused = event.relatedTarget; | |
| if (nextFocused instanceof Node && event.currentTarget.contains(nextFocused)) return; | |
| submitForm(event.currentTarget); | |
| }; | |
| return ( | |
| <form ref={formRef} className={styles.editorForm} onSubmit={handleSubmit} onBlur={handleBlur}> |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50dac6eb2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (itemCount === 0) return null; | ||
| return items[(index + offset + itemCount) % itemCount] ?? null; | ||
| const targetIndex = index + offset; | ||
| return items[targetIndex] ?? items[index] ?? null; |
There was a problem hiding this comment.
Return no item when relative target is out of bounds
Falling back to items[index] changes boundary navigation semantics for callers like getAdjacentSection(), which passes sectionStartIndexes into this helper. At the first/last section, a “previous/next section” action now resolves to the current section’s start index, so selecting next/previous can jump from a child row to the current section header instead of being a no-op; this also triggers selectItem and clears any selected hunk even though there is no adjacent section. Returning null for out-of-range targets avoids this regression.
Useful? React with 👍 / 👎.
50dac6e to
15a2812
Compare
No description provided.