feat(svg-editor): Shift + side-edge resize aspect-locks about the opposite-edge center#865
Merged
Merged
Conversation
…osite-edge center Holding Shift while dragging a side-edge handle (N/S/E/W) now resizes uniformly: the perpendicular axis scales by the same factor as the dragged axis, about the opposite side's center, preserving the aspect ratio. This matches every mainstream graphics tool and the long-standing corner behavior. The modifier already flowed end-to-end (dom.ts maps Shift -> aspect_lock: "uniform", with a mid-drag redrive); it dead-ended because compute_factors masked the off-axis on edge handles. The fix is entirely in the headless math core, mirroring the from-center (Alt) feature: - resize-pipeline.ts: ResizePlan gains `aspect_lock`; compute_factors' Shift block gains edge arms (perpendicular follows the driven factor about the opposite-edge center); `apply` passes Shift to compute_factors only for edges. Corners are untouched — they keep carrying the lock as rewritten deltas via the aspect_lock stage, so corner gestures stay byte-identical. (An edge handle's tracked midpoint is invariant under perpendicular center-scaling, so edge-uniform can't be a delta; it must be a factor.) - orchestrator.ts: threads aspect_lock onto the plan. HUD dashed-preview parity (so the overlay squares up under Shift; also fixes a pre-existing latent corner-preview lag): - gesture.ts: applyResize/applyResizeRect gain an `aspect` opt mirroring the core math (corner = max-magnitude; edge = perpendicular follows). Rebuild reuses cmath.rect.scale. - state.ts: resizePreviewShape forwards Shift; mid-drag toggle refreshes. Tests: new resize-aspect-edge.test.ts (core: compute_factors arms, per-primitive apply, multi-member union, orchestrator end-to-end, round-trip revert) and apply-resize-aspect.test.ts (HUD). Docs + manual TC added.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds Shift-based aspect-lock resize for side-edge handles in both the SVG editor resize pipeline and the canvas HUD. ChangesShift aspect-lock for side-edge resize handles
Sequence Diagram(s)sequenceDiagram
participant User as User (Shift+drag edge)
participant SurfaceState
participant applyResize
participant applyResizeRect
participant ResizeOrchestrator
participant resize_pipeline_apply
User->>SurfaceState: pointer move (dx, dy) with Shift held
SurfaceState->>applyResize: initial, direction, dx, dy, { fromCenter: alt, aspect: shift }
applyResize->>applyResizeRect: rect, direction, dx, dy, fromCenter, aspect=true
applyResizeRect->>applyResizeRect: compute driven/perpendicular scale factors
applyResizeRect->>applyResizeRect: select anchor (pinned edge or bbox center)
applyResizeRect-->>applyResize: scaled SelectionShape
applyResize-->>SurfaceState: dashed preview shape with aspect guide
User->>ResizeOrchestrator: drive(dx, dy) with aspect_lock="uniform"
ResizeOrchestrator->>resize_pipeline_apply: ResizePlan { direction, dx, dy, aspect_lock: true }
resize_pipeline_apply->>resize_pipeline_apply: compute shift = aspect_lock && !is_corner
resize_pipeline_apply->>resize_pipeline_apply: compute_factors(shift=true) → sy=sx or sx=sy
resize_pipeline_apply-->>ResizeOrchestrator: updated DOM attributes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
🚥 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef9927f3c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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/grida-canvas-hud/event/gesture.ts`:
- Around line 531-532: The scale factor calculations for sxRaw and syRaw at
lines 531-532 can produce negative values when dragging crosses the opposite
edge, causing the HUD preview to diverge from the core resize behavior which
clamps factors to a positive floor. Apply Math.max clamping to both sxRaw and
syRaw calculations to ensure they remain positive, and apply the same fix to the
corresponding calculations at lines 547-548 to maintain consistency.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2436dde-f66d-4c99-bfee-e91a3cd13057
📒 Files selected for processing (9)
packages/grida-canvas-hud/__tests__/apply-resize-aspect.test.tspackages/grida-canvas-hud/event/gesture.tspackages/grida-canvas-hud/event/state.tspackages/grida-svg-editor/__tests__/resize-aspect-edge.test.tspackages/grida-svg-editor/__tests__/resize-pipeline.test.tspackages/grida-svg-editor/docs/keybindings.mdpackages/grida-svg-editor/src/core/resize-pipeline/orchestrator.tspackages/grida-svg-editor/src/core/resize-pipeline/resize-pipeline.tstest/svg-editor-resize-shift-edge-aspect.md
`resize_text` infers corner-ness from the factors (`sx`/`sy !== 1`), so the new edge aspect-lock — which makes both factors non-1 — fooled it into treating a `<text>` edge drag as a corner drag and scaling x/y/font-size, breaking the established text-edge no-op contract (and diverging from snap / pixel-grid, which already gate on `resize_capability.constraint`). `apply` now computes free factors first and only re-locks for edges when the element's `constraint` is not a no-op — routing the decision through the same authority snap uses, so apply and snap stay consistent. The synthesized group baseline is a free rect, so group members (text included) still scale uniformly, matching corner-drag. Adds a regression test driving `resize_pipeline.apply` (which exercises the gate) on a text edge+Shift gesture. Addresses Codex review on #865.
The HUD aspect preview derived sx/sy from the free box dimensions, which can go negative when the drag crosses the opposite edge — mirroring the dashed box. The core `compute_factors` clamps factors to a positive floor (0.001), so it commits a degenerate-thin box pinned at the anchor instead. Clamp the preview factors the same way so the dashed box tracks committed geometry on crossover instead of flipping. Adds a crossover test. Addresses CodeRabbit review on #865.
While Shift aspect-locks a resize, draw the diagonal of the preview box opposite the dragged handle (the locked ratio the user is holding) — mirroring the main editor's `AspectRatioGuide`. Rendered as a dashed `HUDLine` in the resize chrome's `decoration_lines` band, reusing the `transformPreview` group. Geometry comes from the shared `cmath.ui.diagonalForDirection` (+ `transformLine` to project a rotated/sheared box's diagonal through its matrix), so no new geometry lives in the HUD. Gated on `state.modifiers.shift` + an active resize gesture; the HUD can't see per-element refusals (e.g. text-on-edge no-ops), so the guide tracks the gesture, not the committed write. No svg-editor change — the HUD already tracks Shift and draws its own chrome.
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.
What
Holding Shift while dragging a side-edge handle (N/S/E/W) in
@grida/svg-editornow resizes uniformly, preserving the aspect ratio. The perpendicular axis scales by the same factor as the dragged axis, about the opposite side's center as the anchor — matching every mainstream graphics tool and the long-standing corner behavior. Composes with Alt (from-center) → Shift+Alt is uniform-about-center.Before: Shift on a side edge did nothing. After: drag the E handle with Shift → width and height grow proportionally, left edge pinned, top/bottom move symmetrically.
Why
The modifier already flowed end-to-end —
dom.tsmapsShift → aspect_lock: "uniform"for all directions, with a mid-drag redrive — but it dead-ended becausecompute_factorsmasked the off-axis on edge handles. This unmasks it.How
All feature logic lives in the headless math core, mirroring the Alt resize-from-center feature (#859):
Core (
@grida/svg-editor)resize-pipeline.ts:ResizePlangainsaspect_lock;compute_factors' Shift block gains edge arms (the perpendicular follows the driven factor about the opposite-edge center);applypasses Shift tocompute_factorsonly for edges.aspect_lockstage, so corner gestures stay byte-identical.orchestrator.ts: threadsaspect_lockonto the plan.HUD (
@grida/hud) — dashed-preview parity so the overlay squares up under Shift (also fixes a pre-existing latent corner-preview lag):gesture.ts:applyResize/applyResizeRectgain anaspectopt mirroring the core math (corner = max-magnitude; edge = perpendicular follows). The rect rebuild reusescmath.rect.scale.state.ts:resizePreviewShapeforwards Shift; the mid-drag modifier toggle already refreshes the preview.Tests & docs
resize-aspect-edge.test.ts—compute_factorsedge arms, per-primitiveapply(rect/ellipse/line/polygon/path), Shift+Alt about center, multi-member union, orchestrator end-to-end, byte-exact round-trip revert.apply-resize-aspect.test.ts(HUD) — edge/corner/Shift+Alt preview geometry.resize-pipeline.test.ts.docs/keybindings.mdupdated; manual TCtest/svg-editor-resize-shift-edge-aspect.mdadded.Verification
@grida/svg-editor(1451) +@grida/hudsuites green, incl. 24 new tests.distrebuilt) + repo typecheck + oxlint + oxfmt clean.Reviewer notes
apply); the split is principled, not a bandaid.(sx, sy, origin); HUD operates on aRect) — forced by the one-way layering (HUD must not import the editor core) and pinned by mirrored tests on both sides.Summary by CodeRabbit