feat(studio): always-on crop with reposition handle, drop crop mode

[miguel-heygen]

Stacked on #2089 (sub-composition scoping fix).

What

Reworks Studio crop from a separate "crop mode" into an always-on part of the element selection, matching the crop UX users expect.

• No crop mode. Removes the crop-mode toggle (canvas toolbar + property-panel buttons), the cropMode/cropAvailable player-store state, and the double-click-to-crop gesture.
• Selection is crop-capable. Selecting a croppable element shows edge handles positioned just outside each side. Once cropped, the full content shows with the cropped-away area dimmed, plus a center reposition handle to pan the crop window.
• Clear gesture split. Dragging the element body moves it; dragging an edge handle crops that side (with a rule-of-thirds guide while dragging); dragging the center handle pans the crop. Corners stay free for the existing resize handle.
• Handles use the primary accent color.

The underlying clip-path: inset(...) model is unchanged; this is a UI/interaction change only.

Why

The old flow required entering a mode, and the handles overlapped the element body so a normal move-drag could accidentally crop. The always-on model removes the mode and keeps crop targets off the body so moving never crops by accident.

Tests

domEditOverlayCrop unit tests cover edge-drag, corner-independent edge crop, and the reposition (pan) math. Studio typecheck clean.

Verify

On a project with a croppable element: select it, drag the body (moves, never crops), drag an edge handle (crops), then drag the center circle (repositions the crop). Requires #2089 for compositions with sub-compositions to render correctly.

[miguel-heygen]

• feat(studio): always-on crop with reposition handle, drop crop mode #2090 👈 (View in Graphite)
• fix(core): scope sub-composition html/body styles so they don't clobber the host document #2089
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[james-russo-rames-d-jusso]

Reviewed at b7c01e0.

Nice net-removal — the always-on model + gesture split (drag body → move, drag edge → crop, drag center → pan) is a real simplification and the deleted state fields (cropMode/cropAvailable) have no straggling references anywhere in the repo. A few things I found while tracing what should die with the mode:

Blockers

• ShortcutsPanel.tsx still documents the old crop UX — packages/studio/src/player/components/ShortcutsPanel.tsx:63-70 renders a user-visible "Crop" hints section (via PlayerControls.tsx:507) that lists three affordances this PR removes / changes:

  • DblClick — Crop selected element (the double-click-on-box gesture was deleted from DomEditOverlay.tsx).
  • Drag frame — Move crop window (the frame is now pointer-events-none; pan is via the new center circle handle, not by dragging the frame).
  • Esc — Exit crop (or click outside) (no mode to exit; the Escape listener was removed from useCropMode.ts).

  Only Drag edge — Adjust crop side still matches reality. Since this panel is the in-app shortcut help, users looking at it will be told to do things that no longer work. Update to reflect the new gesture set (edge = crop, center handle = reposition, no dblclick/Esc needed).

Concerns

• readElementCropInsets hoisted out of its guard — DomEditCropHandles.tsx:96 reads getComputedStyle(...) on every render, but the result is only consumed inside the if (state.element !== selection.element) branch on line 97. Previously (per the diff) the read was inside the conditional. During a drag, setState fires on every pointermove, which re-runs this hook and re-invokes getComputedStyle unnecessarily each frame. Cheap fix: move the const liveInsets = readElementCropInsets(...) inside the if branch it feeds.

• Comment vs. code drift on the same block — the sync block at DomEditCropHandles.tsx:94-108 is commented as "Re-sync when the selection element changes, or when the committed crop changes underneath us (undo/redo, property panel) and no drag is active". But the code only re-syncs on element identity change — there's no separate branch that detects underlying committed-crop drift for a fixed element. In practice undo/redo may re-key the element and cover this, but the comment currently overpromises. Either add the drift check or trim the comment.

• Filename hooks/useCropMode.ts no longer exports useCropMode/useCropModeProps — only useCropOverlay remains. Since the "mode" concept is fully retired by this PR, the filename now misleads a grep useCropMode searcher into finding a file that has nothing to do with a mode. Consider renaming to useCropOverlay.ts (import site is just DomEditOverlay.tsx:24).

• Fire-and-forget commit chain lacks a .catch — DomEditCropHandles.tsx:192-196:

  void Promise.resolve(onStyleCommit?.(...)).then(() => {
    if (liftedRef.current) el.style.setProperty("clip-path", "none");
  });
  

  If onStyleCommit rejects, the re-lift never runs (so the committed crop stays visible instead of "full content + dim") and the rejection is unhandled. Same shape as before this PR, so not a regression, but with crop now front-and-center on every selection it's more exposed. A .catch that at least re-lifts on failure would keep the presentation invariant.

Nits

• Edge handle hit targets are 5×26 px (or 26×5). Fine for mouse; below WCAG 2.5.5 (44×44) for touch. Pre-existing pattern in the repo — flagging for awareness, not asking you to change it here. (nit)

Questions

• The crop-handle overlay now renders for any croppable single selection, cropped or not — including tiny elements. For an ~20×20 icon, four 26-px dot-pill handles floating 8 px outside all four edges will be visually larger than the element itself. Was this considered? A hasCrop gate on the edge handles as well (or a min-element-size gate) would make selection of small croppable elements less noisy — but I can also see the "always-on" affordance being a deliberate discoverability signal, so happy to defer.

• Discoverability: with the toolbar/property-panel Crop buttons gone, the only affordance for "you can crop this" is the four floating edge handles. Any thoughts on a first-run hint or tooltip? (Genuine question — happy to trust product judgement here.)

What I didn't verify

• Runtime behavior — did not launch the app.
• Loom demo — not watched (per brief).
• HF-runtime / core-interop side — that lane belongs to Via/Miga per HF review split.
• Whether the redundant getComputedStyle reads actually show up on a profiler; browsers cache computed style aggressively, so the perf ask above is theoretical until measured.

Cross-PR: #2089 is base but purely compiler-scoping (packages/core/**) — no code-level interaction with the crop change; the stacking is a runtime-render dependency, not a shared-surface risk.

— Review by Rames D Jusso

[vanceingalls]

R1 — hyperframes #2090 at b7c01e00

🟡 Blocking on the same finding as Rames: ShortcutsPanel still documents the removed crop UX. Well-considered UX refactor otherwise — the always-on model + gesture split is a real simplification, and the mode teardown is complete across every file that used to carry it.

Blocker (concurring with Rames)

ShortcutsPanel.tsx:63-70 still documents removed gestures. Verified independently at b7c01e00 — the panel's "Crop" section still lists:

• DblClick — Crop selected element — the double-click-on-box gesture was deleted from DomEditOverlay.tsx (the whole lastBoxPointerDownAtRef + isDoubleClick block is gone).
• Drag frame — Move crop window — the frame is now pointer-events-none absolute border border-dashed at DomEditCropHandles.tsx:237-244; the pan gesture is the new 22-px center circle handle, not the frame.
• Esc — Exit crop (or click outside) — no mode to exit; the Escape listener + bumpAfterExit reducer were removed from useCropMode.ts.

Only Drag edge — Adjust crop side still matches reality. This is the in-app shortcut panel users open to learn the affordances; three of four hints will teach them to do things that no longer do anything. Update to reflect the new gesture set (edge = crop, center handle = reposition, no dblclick/Esc). Same severity Rames rated it — the PR's own thesis ("no more crop mode; gestures integrated into selection") is falsified by user-facing help text that still advertises the mode.

Verified independently

• Crop-mode teardown is complete. Grepped the head SHA across all previously-touching files for cropMode, cropAvailable, useCropModeProps — zero residual matches. Store fields + setters, useCropModeProps hook + CropModeProps interface, Escape handler, setCropAvailable publisher, bumpAfterExit reducer, SnapToolbar Crop button, property-panel Scissors button, all four if (cropMode) bail-outs in DomEditOverlay, and the double-click-to-crop timestamp gesture are all removed.
• Mount gate is semantically equivalent to the pre-fix cropAvailable. DomEditOverlay.tsx:493: selection.capabilities.canCrop && groupSelections.length <= 1 inlines the exact predicate that pre-fix useCropOverlay published to the store as cropAvailable. Same shape, round-trip removed. ✅
• Body-vs-handle gesture-region invariant. edgeHandlePlacement() at DomEditCropHandles.tsx:34-58 places each edge handle at rect.<edge> ± EDGE_HANDLE_GAP px plus a translate transform that pushes the handle FULLY past the boundary. The element body div stays at overlayRect. Verified in the math: for a 200×200 cropRect at (0,0), the right-edge handle center is at (208, 100), the resize handle at (overlayRect.right - 1.5, overlayRect.bottom - 1.5) — different points, corners free.
• Pointer capture is set. DomEditCropHandles.tsx:146: event.currentTarget.setPointerCapture(event.pointerId) on startCropGesture. Drags whose pointer moves off the button still route back. finishCropGesture and cancelCropGesture both null gestureRef.current and setDragging(false) — full lifecycle coverage.
• Test file diff is inert. domEditOverlayCrop.test.ts +16/-16 is a pure reorder — resolveCropInsetFromMoveDrag block moved above cropRectFromInsets. Same content, so still exercises edge-drag / clamp / move math.

Concurring with Rames

• readElementCropInsets hoist re-runs getComputedStyle per pointermove. Rames named the fix (put the read back inside the if (state.element !== selection.element) guard). Agree — and this is genuinely a follow-up worth taking because a sub-frame getComputedStyle in the drag path can invalidate style caches and provoke reflow.
• Comment-vs-code drift on the same-element re-sync block. Same finding I'd have flagged; Rames covered it cleanly.
• hooks/useCropMode.ts filename no longer matches contents. Fair rename to useCropOverlay.ts; single-caller (DomEditOverlay.tsx:24), no other consequences.
• Fire-and-forget onStyleCommit chain lacks a .catch. Pre-existing shape, but as Rames noted, crop is now front-and-center on every croppable selection so the "unhandled rejection + no re-lift" failure mode is more exposed. A .catch that at minimum re-lifts would preserve the presentation invariant.

Unique observations

O1 — Two crop outlines may visually stack. useCropOverlay still returns cropOutlineInsetPx when the element has committed insets, and DomEditOverlay.tsx:464-473 renders a solid border border-studio-accent/80 rounded outline INSIDE the box at those insets. DomEditCropHandles.tsx:236-254 also renders a dashed border border-dashed border-studio-accent outline at cropRect. Both derive from the same source (element's committed clip-path ↔ DomEditCropHandles' local state after mount), so when not dragging they should coincide — you'd see solid+dashed at the same edge. During a drag the solid one doesn't move (element clip is lifted to none) while the dashed one moves with local state — potentially a nice "before/after" affordance, but potentially visually noisy. Worth confirming from the Loom that the coinciding case doesn't look busy. If it does: gate the solid outline on !dragging OR remove it now that the dashed one covers the same job. Not a bug, just a redundancy question the mode-removal newly exposed.

O2 — CI signal at b7c01e00 is thin. Only 15 checks report against the PR head (Preflights, preview-regression, Detect changes, WIP, player-perf, Preview parity). Fallow, Test, Typecheck, Lint, Build, Render on windows, SDK, Format don't run on a stacked PR whose base is worktree-fix-subcomp-body-style-leak. Green on #2089 (base) plus Preview-parity here is the strongest pre-merge signal available; the full suite re-runs after #2089 lands and #2090 rebases on main. Your PR body's "Studio typecheck clean" carries the local verification. Flagging so the missing-Test doesn't get misread as a red flag — but worth eyeing the full-suite result once the rebase lands, since the test file did move blocks around.

Stack coordination

Base is worktree-fix-subcomp-body-style-leak = #2089's head. PR body correctly names the dependency ("Requires #2089 for compositions with sub-compositions to render correctly."). Merge order: #2089 → rebase #2090 on main → #2090.

R1 by Via — blocking on ShortcutsPanel drift (concur with Rames); everything else is observations.

[miguel-heygen]

Thanks — great catches. Latest push addresses the blocker + all concerns:

• Blocker: ShortcutsPanel.tsx — replaced the stale Crop hints (DblClick / Drag frame / Esc) with the real gesture set: Drag edge = crop a side, Drag center = reposition the crop.
• readElementCropInsets hoist — moved the read inside the if (state.element !== selection.element) guard so a drag's per-frame setState no longer re-runs getComputedStyle.
• Comment/code drift — trimmed the sync-block comment to match reality (re-syncs on element identity change only).
• Rename — hooks/useCropMode.ts -> hooks/useCropOverlay.ts (only useCropOverlay remains); import site updated.
• Fire-and-forget commit — the re-lift now runs on both fulfilment and rejection (.then(reLift, reLift)), so a failed commit still restores the full-content + dim presentation and the rejection is handled.

Deferring with rationale (all flagged as defer-ok):

• Touch target size (5x26) — pre-existing repo pattern; not changing here.
• Handles on tiny elements — can't gate edge handles on hasCrop (you'd have no way to start a crop). A min-element-size gate is reasonable as a follow-up; leaving the always-on affordance for now as the discoverability signal.
• First-run hint / discoverability — product call; happy to add a tooltip in a follow-up if wanted.