Fix slides Delete key, multi-select drag, and shared Slides routing#201
Conversation
Slides ignored Delete/Backspace because no key rule existed for them (only the context menu's Delete item was wired). And a no-shift click on an already-selected element collapsed the selection to a single hit, so a follow-up drag only moved that one element instead of the whole group. Add a Delete/Backspace rule that removes selected elements via store.removeElements, skipping when focus is in INPUT/TEXTAREA/SELECT or contenteditable so character deletion in the inline text-box editor still works. Make selectAt preserve the existing selection when a no-shift click hits one of the already-selected elements; the follow-up startDrag then operates on the full selection. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Share-link resolve returns the document's stored type ("sheet" |
"doc" | "slides"), but SharedDocumentInner only special-cased "doc"
and fell through to the spreadsheet branch otherwise. A shared Slides
document was therefore attached under a sheet-{id} Yorkie docKey and
rendered with SheetView, so the viewer saw an empty spreadsheet
instead of the deck.
Extend ResolvedShareLink.type to include "slides", route the slides
branch to a new SharedSlidesLayout that mounts SlidesView under a
slides-{id} docKey, and keep the existing sheet/doc branches intact.
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 (8)
✨ 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 |
Verification: verify:selfResult: ✅ PASS in 163.1s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Adds @wafflebase/slides as a third surface alongside Sheets and Docs, plus 53-shape library (Phase 1+2), adjustment handles for 9 pilot shapes (P3-A.1), live snap guides, align/distribute toolbar, themed authoring, and layout-change UI. Also ships Sheets cell comments (Phase B), docs peer-avatar caret jump, yorkie-js-sdk 0.7.8 upgrade, and CLI/REST API improvements (docs export imageFetcher, expired session refresh, REST docs split). Minor bump because slides is a new top-level package. Highlights: #184 #185 #186 #187 #188 #189 #190 #191 #192 #197 #198 #201 #202 #203 #204 #205 #206 #207 #209 #210
1 participant
Summary
Three slides bugs reported in one session, fixed together because they share the same code paths:
packages/slides/src/view/editor/interactions/keyboard.tshad no key rule forDeleteorBackspace— the only Delete entry point was the right-click context menu. Added a rule that callsstore.removeElementson the current selection. The rule skips whene.targetis anINPUT/TEXTAREA/SELECTorisContentEditable, so Backspace inside the inline text-box editor still deletes characters.selectAtinpackages/slides/src/view/editor/interactions/select.tsreturned[hit]on a no-shift click regardless of the current selection, so the follow-upstartDrag(editor.ts:716) only moved the clicked element. NowselectAtpreservescurrentwhencurrent.includes(hit); replacement still happens for clicks on a non-selected element. Matches Google Slides / Keynote / Figma behavior.share-link.controller.ts:74already passed the document's stored type ("sheet" | "doc" | "slides") through to the resolver response, butSharedDocumentInneronly special-cased"doc"— slides fell into the spreadsheet branch with asheet-{id}Yorkie docKey. ExtendedResolvedShareLink.typeto include"slides", added aSharedSlidesLayoutthat mountsSlidesViewunder aslides-{id}docKey (matching the standalone route atslides-detail.tsx:220).Known limitations
SlidesView'sreadOnlyprop is documented as a Phase 4b no-op (slides-view.tsx:25-30), so aviewer-role share link will see the "View only" badge but the editor remains interactive. The role is forwarded toSlidesViewso it lights up automatically when 4b lands; called out with a comment inSharedSlidesLayout. This is a separate issue from the routing bug — pre-PR, viewers saw the wrong document entirely; post-PR, they at least see the right one.SlidesViewinshared-document.tsx(so non-slides share links don't pay for the slides bundle) caused vite to splitslides-viewand itsslide-rendererdependency into their own chunks (+2). Reasoned inharness.config.json.Test plan
pnpm verify:self— all 9 lanes pass (sheets/docs/slides build, verify:fast 226 + 1265 + 267 + 174 + 764, frontend build, chunk gate, backend build, cli build, entropy).select.test.ts— click on already-selected element preserves multi-selection; click on non-selected element with multi-selection replaces.keyboard.test.ts— Delete/Backspace remove selection (single + multi, single undo entry); Backspace inside textarea is a no-op; no-selection Delete is a no-op.editor.test.ts— multi-select + click-on-selected + drag moves the whole group with identical per-element delta.pnpm dev: select shape + Delete, multi-select + drag, share a slides doc and open the link.🤖 Generated with Claude Code