Slides smart guides (equal-spacing / distance / size)#322
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 19 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR implements smart snapping guides for the slides editor, adding equal-spacing, equal-distance (during drag), and equal-size (during resize) snapping with visual arrow and dashed-box overlays. The feature includes core detection algorithms, comprehensive tests, overlay rendering, and full editor integration. ChangesSlides Smart Guides Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
Spec and bite-sized implementation plan for PowerPoint-style smart guides during element drag and resize. Detection module pure and unit-testable; overlay rendering reuses the existing HTML/CSS guide layer; drag and resize handlers gain a single composed call after the existing snapDelta / resizeFrameWorld passes. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
First step toward PowerPoint-style equal-spacing / equal-distance / equal-size guides during shape drag and resize. This commit lands just the SmartGuide type and an identity-return smartGuides() so the later tasks can layer detection on top one pattern at a time. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When the dragged element sits between two others on the same row or column, snap the middle gap to balance with the outer one. Same 8 px band the existing snapDelta uses; per-axis smallest-adjust tie-break keeps the choice deterministic against multiple qualifying trios. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Extend the equal-spacing pattern so the dragged element snaps when it sits on either END of a pair, not only between them — matches PowerPoint behaviour. Same 8 px threshold and smallest-adjust tie-break as the middle-trio case. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When the dragged element sits to the side of a neighbour and the resulting gap matches a gap already formed by two other elements, snap the dragged element to that distance. Reuses the existing tryX/tryY selectors — smallest |adjust| wins regardless of kind, so a precise distance match isn't unseated by a coarser spacing one in coincidental setups. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Pure resize-side smart-guide entry point: snaps the bbox's width or height to a peer's when within 8 px, with handle-aware origin compensation so the anchored corner / edge stays put. Collects every peer that shares the matched dimension so the overlay can highlight the full group. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds two HTML/CSS overlay primitives next to makeGuide — paired double-headed arrows for equal-spacing / equal-distance, and 1 px dashed boxes for equal-size matched peers. Dispatched from renderOverlay by guide.kind so the existing SnapGuide rendering stays unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Composes smartGuides() onto the snap-corrected (dx, dy), then merges the resulting guides with the existing edge / centre / user-guide set. Shift's axis re-lock continues to have the last word on the delta. Reuses the otherFrames already collected for snapDelta — no extra candidate work. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Runs matchSize after resizeFrameWorld inside startResize. Same 8 px band; handle-aware origin compensation keeps the anchored edge fixed. Shift (preserve-aspect) intentionally bypasses equal-size matching so the two modes do not fight. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Final review found that matchSize's corner-handle origin compensation (nw compensates both axes, ne y-only, sw x-only) was implemented but not directly tested. Add three tests so the branch logic is locked in against accidental regression. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Plan and design doc updated to reflect what shipped: smallest |adjust| wins across equal-spacing and equal-distance candidates, no priority between kinds. The original priority rule produced test contradictions where a precise distance match lost to a coarser spacing match in coincidental setups. Also adds the paired lessons file capturing the workspace-dist rebuild trap, subagent scaffolding pitfall, and follow-up items (N>30 culling, integration tests, visual diffs) deliberately cut from v1. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The resize onUp handler skipped repaintOverlay, so equal-size dashed outlines lingered after the mouse released. Mirror the move-drag onUp pattern. Also documents the matchSize peer-collection semantics (replacement-on-better-target is intentional, not a dropped-peers bug). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds a small white pill with the rounded pixel distance at the midpoint of every equal-spacing and equal-distance arrow. Matches Google Slides; PowerPoint omits them. Equal-size dashed outlines stay unlabeled — the highlighted peer IS the answer to "same as what." Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Use the slides editor's rotation-angle tooltip styling (dark translucent pill with white text) for the equal-spacing / equal-distance distance labels instead of the earlier white-on-red pill — keeps a single visual language for overlay annotations. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/slides/src/view/editor/editor.ts (1)
3317-3329:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSkip the resize commit when the frame never changed.
This path always writes
updateElementFrame(...), so a mousedown/mouseup on a resize handle with no actual resize still creates a no-op mutation and can pollute undo history.Possible fix
const onUp = () => { document.removeEventListener('pointermove', onMove); document.removeEventListener('pointerup', onUp); + if ( + live.worldFrame.x === startWorldFrame.x && + live.worldFrame.y === startWorldFrame.y && + live.worldFrame.w === startWorldFrame.w && + live.worldFrame.h === startWorldFrame.h && + live.worldFrame.rotation === startWorldFrame.rotation + ) { + this.renderer.markDirty(); + this.render(); + this.repaintOverlay(); + return; + } // Convert world frame back to scope-local before committing. const localFrame = fromWorldFrame(live.worldFrame, scope, startSlide); this.options.store.batch(() => { this.options.store.updateElementFrame(startSlide.id, elementId, localFrame); });🤖 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/editor.ts` around lines 3317 - 3329, The onUp handler currently always calls this.options.store.updateElementFrame(...) causing no-op mutations; compute the element's original frame (from startSlide or the saved start frame for elementId) and compare it to the computed localFrame (fromWorldFrame(live.worldFrame, scope, startSlide)); only call this.options.store.updateElementFrame(startSlide.id, elementId, localFrame) inside the batch if the frames differ (use an appropriate deep/frame equality check), otherwise skip the store update to avoid polluting undo history while keeping renderer.markDirty(), render(), and repaintOverlay() behavior unchanged.
🤖 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/design/slides/slides-smart-guides.md`:
- Around line 281-287: Update the risk-table row to reflect the current
tie-break rule by replacing the text that reads "edge > equal-spacing >
equal-distance and smallest `|adjust|` tie-break" with a single clear rule that
the smallest |adjust| wins (and remove the contradictory priority phrasing);
escape the literal pipes so the Markdown table parses (e.g., write the tie-break
as smallest \|adjust\| wins or use HTML entity &`#124`;adjust&`#124`;), and ensure
the row text mentions the strict priority order only if it matches the Priority
section above.
In `@packages/slides/src/view/editor/editor.ts`:
- Around line 2465-2473: The assembled guides array uses pre-lock results
(snapped.guides and smart.guides) but the final movement may have one axis
zeroed by lockAxis; update the assembly so after computing final = ev.shiftKey ?
lockAxis(smart.dx, smart.dy) : smart you filter the combined guides to remove
any guide that reports/depends on movement along an axis that was zeroed (e.g.
drop guides whose axis === 'x' when final.dx === 0, and whose axis === 'y' when
final.dy === 0), then use that filtered list for the overlay instead of the raw
[...snapped.guides, ...smart.guides]; keep references to smartGuides, lockAxis,
snapped.guides, smart.guides, final, dx, dy to locate the change.
In `@packages/slides/src/view/editor/smart-guides.ts`:
- Around line 77-80: The Cand type currently contains a fully materialized guide
that bakes in d.centerXPx/d.centerYPx too early, so when X and Y adjustments are
resolved separately the rendered guides use stale centers; update the candidate
construction to store only the guide template/metadata (not the computed
geometry) and the adjust value, and then rebuild the guide geometry from the
final snapped bounding box (final d.centerXPx and d.centerYPx) when you choose
the winning candidate or render guides; change places that create Cand instances
(references to Cand, the guide property, and code paths that build SmartGuide
instances) to stop precomputing center-based fields and instead compute those
fields from the final centers before rendering or returning the selected guide.
- Around line 95-119: The equal-spacing/distance candidate code currently only
checks overlaps between each non-dragged frame and the dragged frame (using
overlapsRow/overlapsCol) which permits staircase false positives; add an
additional check that the two candidate frames overlap each other on the
perpendicular axis (i.e., call overlapsRow(a, b) in the X-axis branch and
overlapsCol(a, b) in the Y-axis branch) before creating the guide (the tryX/tryY
calls that build guides with kind 'equal-spacing' or 'equal-distance'); apply
this same extra overlap check in the other analogous loops that use
overlapsRow(d, ...) and overlapsCol(d, ...) and add a regression test with a
staggered/stair-step layout to ensure no guides are produced for non-overlapping
pairs.
---
Outside diff comments:
In `@packages/slides/src/view/editor/editor.ts`:
- Around line 3317-3329: The onUp handler currently always calls
this.options.store.updateElementFrame(...) causing no-op mutations; compute the
element's original frame (from startSlide or the saved start frame for
elementId) and compare it to the computed localFrame
(fromWorldFrame(live.worldFrame, scope, startSlide)); only call
this.options.store.updateElementFrame(startSlide.id, elementId, localFrame)
inside the batch if the frames differ (use an appropriate deep/frame equality
check), otherwise skip the store update to avoid polluting undo history while
keeping renderer.markDirty(), render(), and repaintOverlay() behavior unchanged.
🪄 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: 00a58db3-480b-4376-8840-caa017a1c9da
📒 Files selected for processing (9)
docs/design/README.mddocs/design/slides/slides-smart-guides.mddocs/tasks/README.mddocs/tasks/active/20260531-slides-smart-guides-lessons.mddocs/tasks/active/20260531-slides-smart-guides-todo.mdpackages/slides/src/view/editor/editor.tspackages/slides/src/view/editor/overlay.tspackages/slides/src/view/editor/smart-guides.tspackages/slides/test/view/editor/smart-guides.test.ts
hackerwins
force-pushed
the
feat/slides-smart-guides
branch
from
May 31, 2026 11:16
59b842b to
beea0a5
Compare
Contributor
Verification: verify:selfResult: ✅ PASS in 259.0s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
hackerwins
added a commit
that referenced
this pull request
May 31, 2026
PRs #257/#301/#303/#304/#305/#306/#307/#309/#315/#316/#317/#321/#322 shipped between v0.4.2 and the v0.4.3 cut, leaving their paired todo files in active with the process checkboxes still open. Flip the remaining boxes for the work that landed, then move the paired todo/lessons into docs/tasks/archive/2026/05/ via pnpm tasks:archive. Active count drops from 17 to 6; archive count grows by 21 files. Patch release covering 29 commits since v0.4.2. Headline theme is Slides editing & UX polish toward Google Slides / PowerPoint parity: smart guides (equal-spacing / distance / size), Shift drag modifiers, text inside shapes, a Format options right panel, tier-1 universal toolbar controls, text autofit (shrink + grow), group-selection visuals, and several smaller text-box / selection fixes. PPTX import gained paragraph-level bullet styles, body anchor (vertical alignment), and stretched blipFill cover-crop. Docs added a pending-inline-style at collapsed caret, a caret that tracks the resolved text color, bullet indent on Tab / Shift+Tab, font / size / line-spacing / clear formatting toolbar controls, an inline-style-attribute clear that actually lands in the CRDT, and a polished toolbar trigger / color swatch pass shared across docs / sheets / slides. Sheets gained .xlsx import. Infrastructure: Docker images now build on native arm64 runners, fixing the release hang seen on v0.4.2. No Prisma migration, no new backend env vars — the devops bump is a routine image-tag change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
w/hsnaps to a peer with handle-aware origin compensation.packages/slides/src/view/editor/smart-guides.ts; overlay primitives inoverlay.ts; wired intoeditor.tsmove-drag andstartResizereusing the existingcollectSnapCandidatespipeline (no extra candidate work).Design: docs/design/slides/slides-smart-guides.md
Plan: docs/tasks/active/20260531-slides-smart-guides-todo.md
Lessons: docs/tasks/active/20260531-slides-smart-guides-lessons.md
Test plan
smart-guides.test.tscover middle/end trio, pair-to-pair distance, all 8 ResizeHandle origin-compensation paths, multi-peer equal-size, and threshold/perpendicular boundariespnpm verify:fastgreen (56 files / 871 tests)pnpm --filter @wafflebase/slides buildcleanpnpm dev:ehandle to within 8 px of another shape'sw→ snap + dashed red outline on the peer; release → outline clearsKnown cut from v1 (tracked in lessons)
editor.test.ts(unit tests cover the algorithm; manual smoke covers wiring)pnpm verify:browser:dockerMobileSlidesViewlight edit (touch ergonomics need separate tuning)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation