feat: add terminal shortcuts panel for mobile#411
feat: add terminal shortcuts panel for mobile#411blackmammoth merged 4 commits intositeboon:mainfrom
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 infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds localized terminalShortcuts entries for four locales, exposes Changes
Sequence DiagramsequenceDiagram
participant User
participant Panel as TerminalShortcutsPanel
participant WS as WebSocket
participant Terminal as Terminal Instance
User->>Panel: Open panel / select shortcut
Panel->>Panel: validate isConnected and handle focus
alt send input shortcut
Panel->>WS: sendSocketMessage(shortcut payload)
WS->>Terminal: deliver input
Terminal-->>User: render output
Panel->>Panel: start/clear auto-close timeout
else navigation shortcut (scroll)
Panel->>Terminal: terminal.scrollToBottom()
Terminal-->>User: view scrolled
Panel->>Panel: start/clear auto-close timeout
end
Possibly related PRs
Suggested reviewers
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 |
2467736 to
ec3f879
Compare
|
hey @PaloSP ! I like this PR. Can you resolve the merge conflict so that it can be reviewed? |
Slide-out panel providing touch-friendly shortcut buttons (Esc, Tab, Shift+Tab, Arrow Up/Down) and scroll-to-bottom for the terminal. Integrates into the new modular shell architecture by exposing terminalRef and wsRef from useShellRuntime hook and reusing the existing sendSocketMessage utility. Includes localization keys for en, ja, ko, and zh-CN.
ec3f879 to
4472135
Compare
|
Merge conflict resolved — rebased onto latest main and adapted to the new modular shell architecture. Note: would appreciate if native speakers could verify the ja, ko, and zh-CN translations. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx (1)
49-55: Clear any pending close timer before scheduling a new one.This avoids stale timer overlap during rapid interactions.
♻️ Small safety improvement
const handleShortcutAction = useCallback((action: () => void) => { action(); if (document.activeElement instanceof HTMLElement) { document.activeElement.blur(); } + if (closeTimeoutRef.current) { + clearTimeout(closeTimeoutRef.current); + } closeTimeoutRef.current = setTimeout(() => setIsOpen(false), 50); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx` around lines 49 - 55, handleShortcutAction currently schedules a new close timeout without clearing any existing timer, which can cause overlapping/stale timers; before assigning closeTimeoutRef.current = setTimeout(...), call clearTimeout(closeTimeoutRef.current) (and ensure closeTimeoutRef is the ref holding the timeout id) so any pending timeout is canceled, then set the new timeout and call setIsOpen(false) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx`:
- Around line 90-91: The component currently attaches duplicate handlers
(onTouchEnd and onClick) that fire twice on touch devices; replace these pairs
with single pointer handlers (onPointerUp) and consolidated handler functions
that accept a PointerEvent. Create new handlers (e.g., handlePointerToggle to
replace handleToggleTouch/handleToggle, handlePointerShortcut to replace
handleShortcutTouch/handleShortcut, and similar for copy/other shortcut actions)
that call event.preventDefault(), optionally inspect event.pointerType if
needed, then invoke the existing toggle/send logic; update the JSX to use
onPointerUp={handlePointerX} and remove the onTouchEnd/onClick props at the
occurrences around the current handlers (the ones referenced as
handleToggleTouch/handleToggle and the shortcut handlers near lines 137–142,
164–165, 185–186).
---
Nitpick comments:
In `@src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx`:
- Around line 49-55: handleShortcutAction currently schedules a new close
timeout without clearing any existing timer, which can cause overlapping/stale
timers; before assigning closeTimeoutRef.current = setTimeout(...), call
clearTimeout(closeTimeoutRef.current) (and ensure closeTimeoutRef is the ref
holding the timeout id) so any pending timeout is canceled, then set the new
timeout and call setIsOpen(false) as before.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/shell/hooks/useShellRuntime.tssrc/components/shell/types/types.tssrc/components/shell/view/Shell.tsxsrc/components/shell/view/subcomponents/TerminalShortcutsPanel.tsxsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/ja/settings.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/zh-CN/settings.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/i18n/locales/ja/settings.json
- src/i18n/locales/en/settings.json
- src/i18n/locales/ko/settings.json
src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx
Outdated
Show resolved
Hide resolved
Prevents double-fire on touch devices by using onPointerUp instead of separate onTouchEnd + onClick pairs. Also clears pending close timer before scheduling a new one to avoid stale timeout overlap. Addresses CodeRabbit review feedback on PR siteboon#411.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx`:
- Around line 60-74: The pointer-only handlers block keyboard activation;
restore keyboard support by adding equivalent click/keyboard handlers that
invoke the same actions: attach onClick (and/or onKeyDown handling Enter/Space)
to the same elements that currently use onPointerUp and call handleToggle or
handleShortcutAction respectively; update or reuse handlePointerToggle and
handlePointerShortcut logic by adding small wrappers (e.g., handleClickToggle ->
call handleToggle, handleClickShortcut -> call handleShortcutAction(action)) so
keyboard-generated clicks activate the buttons and keep existing pointer
handlers intact.
src/components/shell/view/subcomponents/TerminalShortcutsPanel.tsx
Outdated
Show resolved
Hide resolved
Prevents double-fire on touch devices by removing onTouchEnd handlers and using a single onClick for all interactions (mouse, touch, keyboard). onPointerDown with preventDefault handles focus steal prevention. Also clears pending close timer before scheduling a new one to avoid stale timeout overlap. Addresses CodeRabbit review feedback on PR siteboon#411.
e986074 to
09c791e
Compare
|
@blackmammoth CodeRabbit feedback addressed — replaced dual touch/click handlers with unified pointer events and added timer cleanup. Ready for review. |
blackmammoth
left a comment
There was a problem hiding this comment.
Tested and works well!
* feat: add terminal shortcuts panel for mobile users Slide-out panel providing touch-friendly shortcut buttons (Esc, Tab, Shift+Tab, Arrow Up/Down) and scroll-to-bottom for the terminal. Integrates into the new modular shell architecture by exposing terminalRef and wsRef from useShellRuntime hook and reusing the existing sendSocketMessage utility. Includes localization keys for en, ja, ko, and zh-CN. * fix: replace dual touch/click handlers with unified pointer events Prevents double-fire on touch devices by removing onTouchEnd handlers and using a single onClick for all interactions (mouse, touch, keyboard). onPointerDown with preventDefault handles focus steal prevention. Also clears pending close timer before scheduling a new one to avoid stale timeout overlap. Addresses CodeRabbit review feedback on PR siteboon#411. --------- Co-authored-by: Haileyesus <118998054+blackmammoth@users.noreply.github.com>
Summary
Test plan
Summary by CodeRabbit