Polish slides chrome: thumbnails, picker, notes, toolbar add-slide#198
Merged
Conversation
The left-side thumbnail panel was drag-resizable but each thumbnail was pinned to 192x108px. Narrow panel → horizontal overflow; wide panel → wasted whitespace. The thumbnail canvas also hardcoded dpr: 1 while the main slide canvas already multiplied by window.devicePixelRatio, so on Retina the thumbnail bitmap rendered at half resolution and was stretched, looking blurry. Compute thumbnail size from the panel's measured width (clamp 80-320px), derive the inner canvas to be EXACTLY 16:9 so the slide-renderer's Math.min(scaleX, scaleY) letterbox does not leave the slide background short of the right/bottom border, and re-render on a ResizeObserver tick when the user drags the column resizer. Backing store is sized at innerW/H * window.devicePixelRatio. Switch the item ring from outline to a constant-thickness border inside a border-box: outline pixels sit at the negative edge of the item and were getting clipped by the panel's overflow box on left/top, making the slide look like its left + top edges were missing. Color-only state change avoids any inner-canvas size jump when the current slide changes. Move the in-panel "+ Add slide" split button onto the formatting toolbar (left of Shapes, with a separator), matching where users expect it from Google Slides; export showLayoutPicker so the toolbar can reuse the existing layout grid for the chevron. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Three connected papercuts in the slides UI surfaces: The layout picker's grid column track was set to PREVIEW_W (160px) but each cell wraps the canvas in 4px padding — the canvas got pushed past the cell's right outline by exactly that padding and the slide background visibly spilled outside the border. Expand the column to PREVIEW_W + 2*CELL_PAD and pin box-sizing: border-box on the cell so future padding tweaks behave. renderLayoutPreview hardcoded dpr: 1, mirroring the same Retina half-resolution bug the thumbnail canvas already had. Multiply the backing store by window.devicePixelRatio and add dpr to the cache key so a window dragged between Retina and a non-Retina monitor gets a freshly rendered preview instead of the previous display's bitmap stretched up. The popover, the speaker-notes textarea, and the thumbnail item borders all hardcoded #2a2a2a / #444 / #ddd / #3a7. In Wafflebase's light mode that reads as a dark island floating in a light surface and the borders disappear into surrounding chrome. Switch to the same shadcn tokens the rest of the frontend uses (--popover, --popover-foreground, --background, --foreground, --border, --primary) with the previous hex values kept as fallbacks for jsdom and theme-less hosts (Storybook, isolated demos). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The "+ Add slide" UI moved from the thumbnail panel to the toolbar in 3d964be, but mountThumbnailPanel's class JSDoc still described the in-panel "+" button — it would mislead readers grepping for how new slides get added. Update the doc to point at the toolbar. renderLayoutPreview's cache-key tuple in the JSDoc lagged behind 8dc28d1's actual key, which now also includes dpr; correct the tuple so the GC-eligibility argument still reads consistently. Trim the toolbar's split-button JSX comment to drop the "before this refactor" history clause — git blame already covers that and trailing-history phrases rot when the next refactor lands. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
📝 WalkthroughWalkthroughThis PR implements responsive slide thumbnails with device pixel ratio (DPR) support. Thumbnails now scale to container width (16:9 aspect ratio with min/max bounds) and render crisp on high-DPI displays by scaling canvas backing stores. The "Add Slide" button moves from the thumbnail panel to the formatting toolbar as a split-button. The panel re-renders on width changes via ResizeObserver. Theme-aware CSS variables replace hardcoded colors throughout layout-picker and notes-panel. ChangesResponsive Thumbnails & DPR Crispness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 docstrings
🧪 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 |
Contributor
Verification: verify:selfResult: ✅ PASS in 165.3s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/frontend/src/app/slides/slides-formatting-toolbar.tsx`:
- Around line 200-213: The chevron handler opens the popover via
showLayoutPicker but never toggles it closed and leaks listeners on unmount;
change showLayoutPicker to return a close function (or accept a cancel signal)
and update onOpenLayoutPicker to store that close handle (e.g., closeRef) so
clicking the chevron can toggle by calling closeRef.current and clearing it, and
add a useEffect cleanup that calls closeRef.current on unmount; also handle the
capture-phase mousedown race by tracking wasOpenAtMouseDownRef in the chevron's
onMouseDown and early-return from onClick when that flag indicates the popover
was torn down by the outside-click handler so onClick does not immediately
re-open (referencing onOpenLayoutPicker, layoutChevronRef, showLayoutPicker, and
store).
🪄 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: c826406f-08b1-4ca9-bdbd-76c634d3a468
📒 Files selected for processing (8)
docs/tasks/active/20260509-slides-thumb-responsive-todo.mdpackages/frontend/src/app/slides/slides-formatting-toolbar.tsxpackages/slides/src/index.tspackages/slides/src/view/canvas/layout-preview.tspackages/slides/src/view/editor/layout-picker.tspackages/slides/src/view/editor/notes-panel.tspackages/slides/src/view/editor/thumbnail-panel.test.tspackages/slides/src/view/editor/thumbnail-panel.ts
The split-button "+ Slide" added to the formatting toolbar in 3d964be changed the slides-toolbar.* fixtures, and the new border-instead-of-outline + themed fallback colors on the thumbnail items shifted the surrounding harness fixtures by a few pixels in both light and dark variants. CI flagged exactly these 8 mismatches; regenerate them in the same Docker container CI uses so the byte-level baseline matches what verify-browser sees. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The toolbar chevron opened the layout picker but had two pre-existing papercuts inherited from showLayoutPicker's fire-and-forget shape: clicking the chevron a second time never closed the popover (the capture-phase outside-click handler tore it down, then React's onClick immediately re-opened it), and unmounting the toolbar while the picker was open leaked the popover element + its document keydown/mousedown listeners. Make showLayoutPicker return its internal close function and accept an optional `trigger` element. The trigger is exempted from the outside-click close path, so the chevron's own onClick can toggle via the returned handle without fighting the mousedown race. Guard close with a closed flag so the returned handle is idempotent — a useEffect cleanup that races a real Escape/outside-click no longer fires onClose twice. In the toolbar, store the close handle in a ref, toggle on a second chevron click, clear the ref via onClose, and add a useEffect cleanup so the popover dies with the toolbar. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
hackerwins
changed the title
hackerwins
added a commit
that referenced
this pull request
May 11, 2026
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
5 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
Math.min(scaleX, scaleY)letterbox doesn't leave the slide background short of the right/bottom border. Re-render on aResizeObservertick when the user drags the column resizer.dpr: 1while the main slide canvas already multiplied bywindow.devicePixelRatio. Pass the real dpr through and size the backing store atinner × dpr. The layout-preview cache key now includes dpr so a window dragged between Retina and a non-Retina monitor gets a freshly rendered preview.outlineto a constant-thicknessborderinside a border-box. Outline pixels sit at the negative edge of the item and were getting clipped by the panel's overflow box on left/top, making the slide look like its left + top edges were missing. Color-only state change, so the inner canvas size doesn't jump on selection.2 × CELL_PADso the preview canvas no longer overflows the cell's outline.#2a2a2a/#444/#ddd/#3a7to shadcn tokens (--popover,--popover-foreground,--background,--foreground,--border,--primary) with the previous hex values kept as fallbacks for jsdom and theme-less hosts.Test plan
pnpm verify:fastgreen (47 docs files / 764 tests + 35 slides files / 264 tests + lint + arch + cli + backend + sheets)pnpm --filter @wafflebase/slides build) so frontend tests resolve the newshowLayoutPickerexport from distpnpm dev):+ Slideadds a blank slide; chevron opens the layout grid; pick a layout → slide appears🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests