-
-
Notifications
You must be signed in to change notification settings - Fork 793
feat(keyboard shortcut): Vimium-Style Keybind Update #3463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds Shift+J and Shift+K bindings to rotate the sidebar and 'd'/'u' bindings for half-page scrolling. Implements a new internal scrollHalfPage(direction) helper, updates comments and whitespace. No exported/public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Doc as Document / App
participant KS as Extensions/keyboardShortcut.js
participant Sidebar as SidebarController
participant View as Window/Viewport
User->>Doc: Press key (Shift+J / Shift+K / d / u)
Doc->>KS: keydown event
alt Shift+J / Shift+K
KS->>Sidebar: rotateSidebar(1) or rotateSidebar(-1)
Sidebar-->>KS: rotation applied
else d / u
KS->>KS: scrollHalfPage(1) or scrollHalfPage(-1)
KS->>View: scrollTo(targetY) (half-viewport delta, clamped)
View-->>KS: scroll complete
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Extensions/keyboardShortcut.js (2)
42-42
: Another formatting nit: whitespace-only line.Line 42 also looks like a whitespace-only line. Clearing it will keep Biome happy and reduce diff noise.
Apply the formatter as suggested in the previous comment.
118-128
: Optional: prevent keyup listener accumulation.Each call to createScrollCallback registers a new anonymous keyup listener on document, which isn’t removed. Use { once: true } or a named handler to avoid buildup over long sessions.
Apply this refactor:
- document.addEventListener("keyup", () => { - clearInterval(scrollInterval); - }); + const clear = () => clearInterval(scrollInterval); + document.addEventListener("keyup", clear, { once: true });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Extensions/keyboardShortcut.js
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Code quality
Extensions/keyboardShortcut.js
[error] 32-35: Biome formatting check failed. File content differs from formatting output. Command: 'biome ci . --files-ignore-unknown=true --diagnostic-level=error'
🔇 Additional comments (1)
Extensions/keyboardShortcut.js (1)
34-34
: Trailing whitespace removed in Extensions/keyboardShortcut.js – local CI now passes for this fileThe stray spaces on line 34 have been cleared by running:
npx @biomejs/biome format --write Extensions/keyboardShortcut.jsYou can confirm no further errors for this file with:
npx @biomejs/biome ci Extensions/keyboardShortcut.js --files-ignore-unknown=true --diagnostic-level=errorThe remaining linter failures in other files are pre-existing useIterableCallbackReturn errors and unrelated to this PR; please address them separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Extensions/keyboardShortcut.js (2)
31-35
: Shift+J/K bindings added — confirm coexistence with Ctrl+Tab variantsBoth Shift+J/Shift+K and Ctrl+Tab/Ctrl+Shift+Tab rotate the sidebar now. This is fine, but confirm whether Ctrl+Tab variants should remain (they’re often intercepted by browsers/OS, and the PR text leaned toward Vim-style). Minor wording nit in the comment for clarity.
Apply this small comment tweak for clarity:
- // Rotate through sidebar items using Ctrl+Tab and Ctrl+Shift+Tab or Shift+J and Shift+K + // Rotate through sidebar items using Shift+J/Shift+K (Vim-style). + // Ctrl+Tab/Ctrl+Shift+Tab are retained for backward compatibility.
137-145
: Clamp to max scrollTop to avoid overshoot and clean up spacingCurrent clamp uses scrollHeight, which browsers will internally clamp, but using an explicit maxTop (scrollHeight - clientHeight) avoids ambiguity and aligns with intent. Also normalizes spacing, which should help the formatter.
function scrollHalfPage(direction /* 1 | -1 */) { - const app = focusOnApp(); - if (!app) return; - const delta = Math.floor(app.clientHeight / 2) * direction; - const targetTop = Math.max(0, - Math.min(app.scrollTop + delta, app.scrollHeight) - ); - app.scroll(0, targetTop); + const app = focusOnApp(); + if (!app) return; + const delta = Math.floor(app.clientHeight / 2) * direction; + const maxTop = Math.max(0, app.scrollHeight - app.clientHeight); + const nextTop = Math.max(0, Math.min(app.scrollTop + delta, maxTop)); + app.scroll(0, nextTop); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Extensions/keyboardShortcut.js
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Code quality
Extensions/keyboardShortcut.js
[error] 34-149: Step 'biome ci . --files-ignore-unknown=true --diagnostic-level=error' failed: File content differs from formatting output.
🔇 Additional comments (2)
Extensions/keyboardShortcut.js (2)
45-48
: Half-page scroll keybinds look good‘d’ (down) and ‘u’ (up) now call a dedicated half-page scroller, matching Vimium behavior and the PR objective.
31-36
: Formatting fixed to satisfy Biome CI
Ranbiome check --write
on Extensions/keyboardShortcut.js, removing trailing spaces and unifying indentation around the affected blocks (lines 31–36, 44–48, 136–146). The file now passes formatting checks.
Added Vim Inspired Keys
shift+j
,shift+k
: Rotate through sidebard
: Scroll down (half-page)u
: Scroll up (half-page)Summary by CodeRabbit