Add vim key bindings for navigation#3072
Add vim key bindings for navigation#3072cianswords wants to merge 2 commits intorust-lang:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Vim-style keyboard shortcuts to mdBook’s HTML output to improve navigation and scrolling from the keyboard.
Changes:
- Document Vim-style shortcuts in the user guide.
- Update the in-app help popup to list the new shortcuts.
- Implement
h/lchapter navigation andj/k/d/uscrolling in the frontend key handler.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| guide/src/guide/reading.md | Documents the new Vim-style key bindings. |
| crates/mdbook-html/front-end/templates/index.hbs | Updates the keyboard shortcut help text shown in the UI. |
| crates/mdbook-html/front-end/js/book.js | Adds key handling for h/l/j/k/d/u to navigate/scroll. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| The **left and right arrow keys** on the keyboard can be used to navigate to the previous or the next chapter. | ||
|
|
||
| Additionally, Vim-style key bindings (<kbd>H</kbd>, <kbd>J</kbd>, <kbd>K</kbd>, <kbd>L</kbd>, <kbd>D</kbd> and <kbd>U</kbd>) can be used for navigation. |
There was a problem hiding this comment.
This line says the Vim-style keys are used for “navigation”, but in the implementation j/k/d/u scroll rather than navigate between chapters. Also, the doc displays uppercase keys, while the key handler matches lowercase ('h', 'j', etc.), which may mislead users into holding Shift. Recommend updating the text to distinguish chapter navigation (h/l) vs scrolling (j/k and d/u), and either show lowercase in the guide or add uppercase cases in the handler.
| Additionally, Vim-style key bindings (<kbd>H</kbd>, <kbd>J</kbd>, <kbd>K</kbd>, <kbd>L</kbd>, <kbd>D</kbd> and <kbd>U</kbd>) can be used for navigation. | |
| Additionally, Vim-style key bindings can be used: <kbd>h</kbd> and <kbd>l</kbd> navigate to the previous and next chapter, while <kbd>j</kbd>, <kbd>k</kbd>, <kbd>d</kbd>, and <kbd>u</kbd> scroll the current page. |
| case 'j': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: 80, behavior: 'smooth' }); | ||
| break; | ||
| case 'k': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: -80, behavior: 'smooth' }); | ||
| break; | ||
| case 'd': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: window.innerHeight, behavior: 'smooth' }); | ||
| break; | ||
| case 'u': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: -window.innerHeight, behavior: 'smooth' }); | ||
| break; |
There was a problem hiding this comment.
These scroll actions always use behavior: 'smooth'. For users who prefer reduced motion, this can be an accessibility issue. Consider detecting prefers-reduced-motion: reduce and using behavior: 'auto' (or omitting behavior) when reduced motion is requested.
| case 'j': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: 80, behavior: 'smooth' }); | ||
| break; | ||
| case 'k': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: -80, behavior: 'smooth' }); | ||
| break; | ||
| case 'd': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: window.innerHeight, behavior: 'smooth' }); | ||
| break; | ||
| case 'u': | ||
| e.preventDefault(); | ||
| window.scrollBy({ top: -window.innerHeight, behavior: 'smooth' }); | ||
| break; |
There was a problem hiding this comment.
The scroll behavior is duplicated across four cases and introduces a magic number (80). This will be harder to tune consistently later. Consider extracting a small helper (e.g., scroll(delta)) and defining named constants for the line-step and page-step to centralize the behavior.
| <div> | ||
| <p>Press <kbd>←</kbd> or <kbd>→</kbd> to navigate between chapters</p> | ||
| <p>Press <kbd>←</kbd>/<kbd>→</kbd> or <kbd>h</kbd>/<kbd>l</kbd> to navigate between chapters</p> | ||
| <p>Press <kbd>j</kbd>/<kbd>k</kbd> or <kbd>d</kbd>/<kbd>u</kbd> to scroll the page up and down</p> |
There was a problem hiding this comment.
The second line lumps j/k (small increments) and d/u (page-sized increments) together as “scroll up and down”. To avoid confusion, consider clarifying that j/k scroll by a small step and d/u scroll by roughly one page.
| <p>Press <kbd>j</kbd>/<kbd>k</kbd> or <kbd>d</kbd>/<kbd>u</kbd> to scroll the page up and down</p> | |
| <p>Press <kbd>j</kbd>/<kbd>k</kbd> to scroll a small amount, or <kbd>d</kbd>/<kbd>u</kbd> to scroll roughly one page</p> |
This change adds Vim-style key bindings for easier navigation without leaving the home row.
Implemented are:
handlfor going to the previous and next pagejandkfor scrolling up and downdandufor scrolling up and down by one page(Note: This is my first ever PR, please tell me if I did something wrong)
closes #2936