Add keyboard shortcuts panel#595
Conversation
… into document-kb-shortcuts
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR refactors keyboard shortcuts documentation from an inline sidebar element to a separate modal panel. It removes the keyboard controls UI from index.html's info-panel, creates a new Changes
Sequence DiagramsequenceDiagram
actor User
participant Main as main.js
participant ShortcutsPanel
participant DOM
User->>Main: Press Ctrl+/
Main->>ShortcutsPanel: toggle()
ShortcutsPanel->>ShortcutsPanel: Check if visible
alt Panel hidden
ShortcutsPanel->>DOM: Render shortcuts table<br/>(categorized rows)
ShortcutsPanel->>DOM: Set display: visible
ShortcutsPanel->>DOM: Add global click/key listeners
else Panel visible
ShortcutsPanel->>DOM: Set display: hidden
ShortcutsPanel->>DOM: Remove global listeners
end
User->>DOM: Click close button or<br/>press Escape
DOM->>ShortcutsPanel: Trigger hide()
ShortcutsPanel->>DOM: Set display: hidden
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 30 minutes and 19 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main/accessibility.js`:
- Around line 373-392: When opening the shortcuts panel in show(), capture and
save the currently focused element (e.g. this._previousFocus =
document.activeElement), set focus into the panel (preferably to its close
button or first focusable element inside this.panel; add tabindex="-1" if
needed) and ensure the panel is focusable so keyboard users can tab the
contents; in hide(), restore focus to this._previousFocus (and clear the saved
reference) before hiding the panel. Update the show() and hide() methods on the
same object (references: show(), hide(), this.panel, and the panel’s close
button element) so focus is moved into .shortcuts-panel on open and returned on
close.
- Around line 408-431: The keydown listener on document currently traps all
Ctrl/Cmd+Arrow events even when focus is in editable fields; update the handler
(the document.addEventListener("keydown"...) that references this.panel and
calls this.setDock) to first ignore events when focus is outside the shortcuts
panel or when the event target is an editable control: bail out unless
this.panel.contains(document.activeElement) OR (event.target is inside the
panel). Also treat inputs, textareas, contentEditable elements and selects as
editable by checking tagName and el.isContentEditable (or using
event.target.closest to test), and only run the ArrowLeft/Right/Up/Down handling
when the focus/target is within the shortcuts panel.
In `@style.css`:
- Around line 1595-1599: The CSS rule for the kbd selector uses the keyword
"currentColor" with mixed case and triggers Stylelint's value-keyword-case;
update the property value in the kbd rule (border: 1px solid currentColor) to
the lowercase form "currentcolor" so it satisfies the linter. Locate the kbd
selector in style.css and replace the currentColor token used in border
declarations with currentcolor, keeping the rest of the declaration intact.
In `@ui/axis-keyboard.js`:
- Around line 17-26: The gizmo key handler currently short-circuits when the
event target t is inside "#codePanel" or "header" but doesn't exclude the
shortcuts panel; update the conditional that checks t?.closest (or add an
additional early-return) to also return when t.closest(".shortcuts-panel") is
true so keyboard interactions inside the shortcuts panel don't propagate to
gizmo handling; ensure you modify the selector used around t?.closest (or add a
separate check immediately before the tag/isContentEditable checks) in
ui/axis-keyboard.js so Enter and Ctrl/Cmd+Arrow keys inside the shortcuts panel
are not swallowed by the gizmo handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04263898-0ccc-47bd-afa3-767cc7beae83
📒 Files selected for processing (7)
index.htmlmain/accessibility.jsmain/blockhandling.jsmain/input.jsmain/main.jsstyle.cssui/axis-keyboard.js
Summary
Adds a keyboard shortcuts panel which can be docked left or right.
Details
/or ⌘ +/to toggle open or close shortcut panel - it deliberately does NOT hide onEscas you might be using Esc for something on the UIFurther work to do
Relates to #512 #343
AI usage
Claude Sonnet 4.6 used throughout, with all changes carefully approved by a human.
Summary by CodeRabbit