Add Google Slides parity keyboard shortcuts#238
Merged
Conversation
The slides editor already covered nudge / undo-redo / delete / clipboard / duplicate / z-order. The remaining shortcuts a Google Slides user reaches for on day one were missing: select all, Esc clear-selection, Tab cycle, F2/Enter enter-edit, Cmd+M new slide, Cmd+Shift+D duplicate slide, Page Up/Down navigate, Cmd+Enter present, Cmd+Shift+Enter present from start, Cmd+/ shortcut help, and the Cmd+K link request inside text-boxes. This change adds all of them as keyRules in the existing interactions/keyboard.ts array. The editor exposes three optional host callbacks (onStartPresentation, onShowShortcutsHelp, onLinkRequest) so the surrounding shell can route to its presentation entry, help modal, and link popover. Group/ungroup and find/replace are intentionally deferred — both require larger changes and are scoped to follow-up designs. A new shortcuts-catalog module exports the full list with a platform-neutral key notation; it's the source of truth that the help modal and any future doc pages will render from. Single rule-and- catalog drift is the trade-off; comments call out the dual-edit convention. Design: docs/design/slides/slides-keyboard-shortcuts.md Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The editor exposes onShowShortcutsHelp; this commit adds the React modal that consumes the SHORTCUTS catalog and renders categorized key combos. Without a host-side wiring, Cmd+/ no-ops — the modal is the only path users have to discover the parity shortcuts shipped in the previous commit. The onStartPresentation and onLinkRequest callbacks are left unwired. Present mode UI isn't yet implemented in the slides package, and a real link popover requires TextBoxEditorAPI to expose insertLink / getLinkAtCursor first. Comments in slides-view.tsx call this out so the next owner knows where to pick up. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The runtime + help modal are in place. Two items remain as documented deferrals: present-mode wiring (UI not yet implemented) and a real link popover (needs TextBoxEditorAPI expansion). Both have the editor-side hooks ready, so picking them up is contained. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The first round of selection-level shortcuts only checked INPUT/TEXTAREA/SELECT/contenteditable for the editable-target gate. That left a real UX bug: pressing Tab inside the shortcuts-help dialog (a focused button inside role="dialog") would fire the slides Tab-cycle rule and steal focus from the dialog. Enter on any focused button would enter text-edit mode on a selected text element instead of activating the button. Extend isEditableTarget to also gate when the focused element is a <button>, has a button/menuitem/option/tab role, or lives inside a dialog/menu/listbox/combobox/tree/grid. Two regression tests lock this in. Also tighten the present-mode and Cmd+/ rules to require their host callback be wired before matching — otherwise an unhandled shortcut consumed the event silently. Document the Cmd+Enter behaviour deviation (it respects the gate rather than bypassing it like the original design said) and the rationale in the design doc. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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 (1)
📝 WalkthroughWalkthroughThis PR implements keyboard shortcut parity with Google Slides by introducing a centralized shortcuts catalog, extending keyboard interaction rules and editor options, adding link-request callbacks throughout the text editing stack, and building a help modal UI integrated into the slides view. ChangesSlides Keyboard Shortcuts Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/frontend/src/app/slides/slides-shortcuts-help.tsx (1)
50-52: ⚡ Quick winRender uncategorized shortcut groups instead of dropping them silently.
The current loop only renders categories listed in
CATEGORY_ORDER, so new categories added toSHORTCUTSwon’t appear in the help modal.Proposed diff
- <div className="grid grid-cols-1 md:grid-cols-2 gap-x-8 gap-y-6 mt-2"> - {CATEGORY_ORDER.map((category) => { + <div className="grid grid-cols-1 md:grid-cols-2 gap-x-8 gap-y-6 mt-2"> + {[...CATEGORY_ORDER, ...Array.from(grouped.keys()).filter((c) => !CATEGORY_ORDER.includes(c))].map((category) => { const entries = grouped.get(category); if (!entries || entries.length === 0) return null; return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/app/slides/slides-shortcuts-help.tsx` around lines 50 - 52, The rendering loop currently iterates only CATEGORY_ORDER, dropping any other groups from grouped derived from SHORTCUTS; update the iteration in slides-shortcuts-help.tsx to include uncategorized groups by computing the remaining categories (e.g. Array.from(grouped.keys()).filter(k => !CATEGORY_ORDER.includes(k))) and concatenating them to CATEGORY_ORDER (or sort grouped entries using CATEGORY_ORDER as a priority) before mapping; ensure you still skip empty entries (the existing entries check) and reference CATEGORY_ORDER, grouped, and SHORTCUTS when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/tasks/active/20260514-slides-keyboard-shortcuts-todo.md`:
- Line 11: Change the task section headings from level 3 to level 2 to fix
markdownlint MD001: replace each "### Task X: ..." heading with "## Task X: ..."
(e.g., update the "### Task 1: Shortcuts catalog" heading and the other
occurrences noted at lines 22, 45, 56, 77, and 85) so the headings increment
correctly under the top-level H1.
In `@packages/slides/src/view/editor/shortcuts-catalog.ts`:
- Line 78: The top-line comment "// History" is inconsistent with the shortcuts
below which use category 'Help' for the undo/redo entries; update one to match
the other: either change the comment to "// Help" or rename the category string
for the undo/redo shortcuts to 'History' (look for the undo/redo shortcut
objects or the shortcuts array in shortcuts-catalog.ts, e.g., entries with
identifiers or labels like "undo"/"redo" or methods registering Undo/Redo) so
the category and comment describe the same group.
---
Nitpick comments:
In `@packages/frontend/src/app/slides/slides-shortcuts-help.tsx`:
- Around line 50-52: The rendering loop currently iterates only CATEGORY_ORDER,
dropping any other groups from grouped derived from SHORTCUTS; update the
iteration in slides-shortcuts-help.tsx to include uncategorized groups by
computing the remaining categories (e.g. Array.from(grouped.keys()).filter(k =>
!CATEGORY_ORDER.includes(k))) and concatenating them to CATEGORY_ORDER (or sort
grouped entries using CATEGORY_ORDER as a priority) before mapping; ensure you
still skip empty entries (the existing entries check) and reference
CATEGORY_ORDER, grouped, and SHORTCUTS when making the change.
🪄 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: fa8b710d-1345-4f3c-8d85-d04523c19a6d
📒 Files selected for processing (13)
docs/design/README.mddocs/design/slides/slides-keyboard-shortcuts.mddocs/tasks/active/20260514-slides-keyboard-shortcuts-todo.mdpackages/docs/src/view/text-box-editor.tspackages/frontend/src/app/slides/slides-shortcuts-help.tsxpackages/frontend/src/app/slides/slides-view.tsxpackages/slides/src/index.tspackages/slides/src/view/editor/editor.tspackages/slides/src/view/editor/interactions/keyboard.test.tspackages/slides/src/view/editor/interactions/keyboard.tspackages/slides/src/view/editor/shortcuts-catalog.test.tspackages/slides/src/view/editor/shortcuts-catalog.tspackages/slides/src/view/editor/text-box-editor.ts
|
|
||
| --- | ||
|
|
||
| ### Task 1: Shortcuts catalog |
There was a problem hiding this comment.
Fix heading-level increment to satisfy markdownlint (MD001).
These ### headings are directly under an H1, which violates heading increment rules. Use ## for each task section.
Proposed fix
-### Task 1: Shortcuts catalog
+## Task 1: Shortcuts catalog
...
-### Task 2: Extend `KeyboardContext` + add key rules
+## Task 2: Extend `KeyboardContext` + add key rules
...
-### Task 3: Link callback plumbing
+## Task 3: Link callback plumbing
...
-### Task 4: Frontend wiring
+## Task 4: Frontend wiring
...
-### Task 5: Verify and commit
+## Task 5: Verify and commit
...
-### Task 6: Archive
+## Task 6: ArchiveAlso applies to: 22-22, 45-45, 56-56, 77-77, 85-85
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 11-11: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tasks/active/20260514-slides-keyboard-shortcuts-todo.md` at line 11,
Change the task section headings from level 3 to level 2 to fix markdownlint
MD001: replace each "### Task X: ..." heading with "## Task X: ..." (e.g.,
update the "### Task 1: Shortcuts catalog" heading and the other occurrences
noted at lines 22, 45, 56, 77, and 85) so the headings increment correctly under
the top-level H1.
| { category: 'Present', keys: ['Mod+Enter'], description: 'Start presentation from the current slide' }, | ||
| { category: 'Present', keys: ['Mod+Shift+Enter'], description: 'Start presentation from the first slide' }, | ||
|
|
||
| // History -------------------------------------------------------------- |
There was a problem hiding this comment.
Update comment to match the category.
The comment says "History" but the shortcuts below use category 'Help'. Consider either changing the comment to // Help or renaming the category to 'History' if that better describes Undo/Redo.
📝 Proposed fix
- // History --------------------------------------------------------------
+ // Help (includes history operations) -----------------------------------
{ category: 'Help', keys: ['Mod+Z'], description: 'Undo' },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // History -------------------------------------------------------------- | |
| // Help (includes history operations) ----------------------------------- | |
| { category: 'Help', keys: ['Mod+Z'], description: 'Undo' }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/slides/src/view/editor/shortcuts-catalog.ts` at line 78, The
top-line comment "// History" is inconsistent with the shortcuts below which use
category 'Help' for the undo/redo entries; update one to match the other: either
change the comment to "// Help" or rename the category string for the undo/redo
shortcuts to 'History' (look for the undo/redo shortcut objects or the shortcuts
array in shortcuts-catalog.ts, e.g., entries with identifiers or labels like
"undo"/"redo" or methods registering Undo/Redo) so the category and comment
describe the same group.
Contributor
Verification: verify:selfResult: ✅ PASS in 189.1s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The doc had accumulated PR-scope language and step-by-step task content. A design doc outlives the PR that lands it, so: - Drop the 4-step "Link popover wiring" procedure; replace with a one-paragraph statement of the callback flow. - Drop the Help modal's UI-implementation bullets; keep only the architectural fact that it renders from the catalog. - Remove the Testing section — test layout is a task artifact. - Remove duplicate "Catalog drift" risk and the post-implementation "Categories expanded" note that crept in during review. - Bump target-version to 0.4.1. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This was referenced May 14, 2026
4 tasks
hackerwins
added a commit
that referenced
this pull request
May 18, 2026
These seven slides + PPTX tasks shipped (PRs #238, #240, #241, #255, #259, #260, #261) but their checklist boxes were never ticked during execution, so the archive script left them in active/. Sweep the checkboxes to reflect what actually landed, add lessons files for the two that were missing (keyboard-shortcuts, presentation-mode), and convert the keyboard-shortcuts onLinkRequest popover bullet from a checkbox to a "Deferred (follow-up)" note so the gate stops blocking on work that is intentionally out of scope. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
9 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add the Google Slides parity keyboard shortcuts the slides editor was missing — selection (
Cmd+A,Esc,Tab/Shift+Tab,F2/Enter), slide navigation (Cmd+M,Cmd+Shift+D,Page Up/Page Down), present-mode entry (Cmd+Enter,Cmd+Shift+Enter), and aCmd+/shortcuts-help modal.interactions/keyboard.tsarray;SlidesEditorOptionsgains three optional host callbacks (onStartPresentation,onShowShortcutsHelp,onLinkRequest).shortcuts-catalog.tsis the single source of truth for shortcut metadata; the help modal renders directly from it.Cmd+K(link) is plumbed end-to-end throughdocs/text-box-editor→ slides text-box →SlidesEditor. The host wiring for a real link popover is deferred (requires expandingTextBoxEditorAPIwithinsertLink/getLinkAtCursor).Design:
docs/design/slides/slides-keyboard-shortcuts.md. Task:docs/tasks/active/20260514-slides-keyboard-shortcuts-todo.md.Group/Ungroup and Find/Replace are explicitly out of scope (no group concept in the model; find/replace needs a deck-wide UX).
Test plan
pnpm verify:fast— greenkeyboard.test.ts(Cmd+A, Esc, Tab/Shift+Tab, F2/Enter, Cmd+M, Cmd+Shift+D, Page Up/Down, present callbacks, Cmd+/, interactive-widget gate)shortcuts-catalog.test.ts(invariants +formatCombo)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation