fix(desktop): restore native text editing shortcuts in chat and auto-focus on pane nav#2676
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughProgrammatic focus was added to move the prompt textarea cursor to the end; modifier+ArrowLeft/ArrowRight keystrokes now stop propagation. A new hook ties pane focus state to focusing the prompt input, and relevant ChatInputFooter components invoke that hook. Changes
Sequence Diagram(s)sequenceDiagram
participant Pane as Pane / isFocused
participant Footer as ChatInputFooter
participant Hook as useFocusPromptOnPane
participant Controller as PromptInputController
participant Textarea as Prompt Textarea
Pane->>Footer: isFocused changes (true)
Footer->>Hook: call useFocusPromptOnPane(isFocused)
Hook->>Controller: read textInput ref
Hook->>Textarea: invoke textInput.focus()
Textarea-->>Hook: focused (caret moved to end via setSelectionRange)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
58a20fe to
3aea57f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx (1)
337-344: Consider extracting the editable-element guard to a reusable helper.The guard logic is duplicated across both
PREV_PANEandNEXT_PANEhandlers. Given the context that each component usinguseAppHotkeymust implement its own checks for editable elements, extracting this to a utility would improve consistency and reusability.♻️ Proposed refactor
Add a helper function near the top of the file or in a shared utilities module:
function isEditableElement(target: EventTarget | null): boolean { if (!target || !(target instanceof Element)) return false; const tagName = target.tagName; return ( tagName === "TEXTAREA" || tagName === "INPUT" || (target as HTMLElement).isContentEditable ); }Then simplify both handlers:
useAppHotkey( "PREV_PANE", (event) => { - const target = event.target as Element; - if ( - target?.tagName === "TEXTAREA" || - target?.tagName === "INPUT" || - (target as HTMLElement)?.isContentEditable - ) - return; + if (isEditableElement(event.target)) return; if (!activeTabId || !activeTab?.layout || !focusedPaneId) return;Also applies to: 357-364
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx around lines 337 - 344, Extract the duplicated editable-element guard used in the PREV_PANE and NEXT_PANE handlers into a reusable helper (e.g., isEditableElement) and use it from both handlers: create isEditableElement(target: EventTarget | null) that returns true when target is an Element and its tagName is "TEXTAREA" or "INPUT" or isContentEditable is true, place it near the top of the file or in a shared util, and replace the inline guard checks inside the PREV_PANE and NEXT_PANE callbacks (the handlers passed to useAppHotkey) with calls to this helper to ensure consistent behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx:
- Around line 337-344: Extract the duplicated editable-element guard used in the
PREV_PANE and NEXT_PANE handlers into a reusable helper (e.g.,
isEditableElement) and use it from both handlers: create
isEditableElement(target: EventTarget | null) that returns true when target is
an Element and its tagName is "TEXTAREA" or "INPUT" or isContentEditable is
true, place it near the top of the file or in a shared util, and replace the
inline guard checks inside the PREV_PANE and NEXT_PANE callbacks (the handlers
passed to useAppHotkey) with calls to this helper to ensure consistent behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba082e76-7bdf-4aeb-92cf-f5dc52bc8ff7
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
|
I don't think this is exactly right since the hotkeys can be rebound which would make this stale. wonder if we need to fix hotkeys at the api level or if we should fix the chat input to prevent bubbling to hotkeys instead ? |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx">
<violation number="1">
P1: Pane navigation hotkeys lost editable-element guards, so PREV_PANE/NEXT_PANE can now trigger while typing in inputs/contentEditable fields.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Cmd+Shift+Left/Right should allow text selection in chat prompts, not hijack focus to the adjacent pane.
Fix pane-nav hotkey hijacking at the source — the prompt input now calls stopPropagation for Cmd/Ctrl+Arrow keys so they perform native text selection rather than switching panes.
fc39ab2 to
26c7ecb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsx (1)
85-85: Implementation is correct; consider extracting shared focus logic.The
useEffectandusePromptInputControllerusage is correct. However, this code is nearly identical toapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx. The focus-on-pane logic could be extracted into a small custom hook (e.g.,useFocusPromptOnPane) to reduce duplication and ensure both components stay in sync.♻️ Example hook extraction
Create a shared hook:
// e.g., in a shared hooks directory export function useFocusPromptOnPane(isFocused: boolean) { const { textInput } = usePromptInputController(); useEffect(() => { if (isFocused) { textInput.focus(); } }, [isFocused, textInput]); return { textInput }; }Then both
ChatInputFootercomponents can simply call:const { textInput } = useFocusPromptOnPane(isFocused);Also applies to: 93-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsx at line 85, Duplicate focus-on-pane logic in WorkspaceChatInterface's ChatInputFooter (useEffect + usePromptInputController usage) should be extracted into a shared custom hook (e.g., useFocusPromptOnPane) so both ChatInputFooter implementations stay in sync; create a small hook that accepts isFocused, calls usePromptInputController() inside, and runs the existing useEffect to call textInput.focus() when isFocused is true, then replace the local textInput/useEffect usage in WorkspaceChatInterface's ChatInputFooter and the other ChatInputFooter (also referenced around the 93-97 block) to simply call const { textInput } = useFocusPromptOnPane(isFocused).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsx:
- Line 85: Duplicate focus-on-pane logic in WorkspaceChatInterface's
ChatInputFooter (useEffect + usePromptInputController usage) should be extracted
into a shared custom hook (e.g., useFocusPromptOnPane) so both ChatInputFooter
implementations stay in sync; create a small hook that accepts isFocused, calls
usePromptInputController() inside, and runs the existing useEffect to call
textInput.focus() when isFocused is true, then replace the local
textInput/useEffect usage in WorkspaceChatInterface's ChatInputFooter and the
other ChatInputFooter (also referenced around the 93-97 block) to simply call
const { textInput } = useFocusPromptOnPane(isFocused).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adf5ed90-7941-40ba-b8fe-24a1d2c55ff5
📒 Files selected for processing (3)
apps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsxpackages/ui/src/components/ai-elements/prompt-input.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/components/ai-elements/prompt-input.tsx
|
Hey @Kitenite this is ready for review now. I opted to prevent bubbling to hotkeys — good call! |
What does this PR do? (required)
Three related fixes to keyboard/focus behavior in the chat prompt textarea:
Skip pane-nav shortcuts when input is focused —
Cmd+Shift+Left/Rightwas being intercepted by thePREV_PANE/NEXT_PANEhotkey handlers when the cursor was in a textarea or input. Added an early-return guard so native text selection is preserved.Stop modifier+arrow propagation in chat textarea — Moved the fix to the source: the prompt textarea now calls
stopPropagationonCmd/Ctrl+Arrowkeys so they perform native text navigation/selection rather than bubbling to pane-navigation handlers.Auto-focus chat textarea on pane nav — When navigating to a chat pane via keyboard shortcut, the prompt textarea is now automatically focused (mirroring how the terminal auto-focuses on pane activation). The cursor is placed at the end of any existing draft text rather than selecting it all.
Link to Basecamp to-do, Trello card, New Relic or Honeybadger (required)
To be added by developer
QA
What platforms should be included in QA?
QA steps
Text editing shortcuts in chat textarea:
Cmd+Left— verify it moves the cursor to the start of the line (native behavior), not switching to the previous pane.Cmd+Right— verify it moves the cursor to the end of the line, not switching to the next pane.Cmd+Shift+Left— verify it selects text to the left of the cursor, not switching panes.Cmd+Shift+Right— verify it selects text to the right, not switching panes.Pane navigation still works outside inputs:
8. Click on an empty area (not a textarea/input) and press
Cmd+Shift+Left/Right— verify pane navigation still works as expected.Auto-focus chat textarea on pane nav:
9. With a chat pane and terminal pane open, click into the terminal so it has focus.
10. Press the pane navigation shortcut (
Cmd+Shift+LeftorCmd+Shift+Right) to move focus to the chat pane.11. Verify the chat prompt textarea is automatically focused (you can start typing immediately).
12. If there is existing draft text in the textarea, verify the cursor is placed at the end of the text (not selecting all).
Screenshots (if appropriate)
N/A
Docs
Update changelog after deployment if the changes in Admin are valuable for Sales, Support or Success teams.
Summary by CodeRabbit