Skip to content

feat(studio): arrow-key nudge step math (unwired)#2203

Closed
ukimsanov wants to merge 1 commit into
studio-dnd/pr11-canvas-zorder-menufrom
studio-dnd/pr12-canvas-nudge-math
Closed

feat(studio): arrow-key nudge step math (unwired)#2203
ukimsanov wants to merge 1 commit into
studio-dnd/pr11-canvas-zorder-menufrom
studio-dnd/pr12-canvas-nudge-math

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

What

domEditNudge — pure nudge-step math (direction × modifier → delta, commit debounce constant) with tests. Shipped unwired.

Why

small standalone concept consumed by the canvas nudge hook in the chrome-components PR.

How

new files; one TEMP(studio-dnd) ignoreExports entry until the consumer lands (removed by the app-shell swap PR).

Test plan

bunx vitest run domEditNudge.test.ts; tsc --noEmit; fallow audit clean.


Stack position 12/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary TEMP(studio-dnd) fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.

ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from a5ce90b to f6dd7eb Compare July 11, 2026 00:55
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 9d0fa02 to 78c55d1 Compare July 11, 2026 00:55
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from f6dd7eb to f45f759 Compare July 11, 2026 01:15

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: LGTM (green) — smallest of the batch, correct at every boundary

Two pure helpers: resolveCanvasNudgeDelta maps arrow keys (plain / Shift) to a { dx, dy } in composition pixels; canCanvasNudgeTargets gates the whole nudge on target capabilities. Nothing to trip over.

The pointed queries

  • Step math is unambiguous. packages/studio/src/components/editor/domEditNudge.ts:11–:12CANVAS_NUDGE_STEP_PX = 1, CANVAS_NUDGE_SHIFT_STEP_PX = 10. Constants are integers in composition-space pixels; the returned { dx, dy } is likewise in pixels. Test at :24–:52 pins all four cardinal directions × {plain, Shift}.
  • Chord passthrough. :28 rejects metaKey || ctrlKey || altKey → returns null so browser/app shortcuts route past the nudge. Tested at :54–:58 for all three modifiers. Correct.
  • Non-arrow rejection. :29 gates on CANVAS_NUDGE_KEYS.has(event.key); non-arrows fall through to null. Tested at :60–:63.
  • Target-capability gate. canCanvasNudgeTargets at :42–:44 requires at least one target AND every target's canApplyManualOffset === true. Test at :66–:79 covers [] (false), [movable] (true), [movable, movable] (true), [locked] (false), [movable, locked] (false). All four boundary cases.
  • Debounce constant. :14 (CANVAS_NUDGE_COMMIT_DEBOUNCE_MS = 400) is exported but not yet consumed at this SHA — exempted in .fallowrc.jsonc at :93–:97 with the TEMP marker pointing at studio-dnd/pr22. Honest.
  • No accumulation logic here. Delta accumulation for holding an arrow key (the "burst commit" pattern named in the docstring at :13) is NOT in this file — it belongs to the wiring PR. This module is a pure per-event mapper. Right scope.
  • Wiring with #2200 (canvas nudge gate). No import at this SHA — verified via grep against DomEditOverlay.tsx head. As with #2200, the pair is expected to be wired together by studio-dnd/pr22. This PR's math is independently testable and it is tested.

R1 by Via

@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from f45f759 to a15be5a Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from ad7cd13 to 97977b2 Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from a15be5a to f20cc98 Compare July 11, 2026 08:18
@ukimsanov ukimsanov marked this pull request as ready for review July 11, 2026 09:06
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from f20cc98 to f3e5bb2 Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 143e573 to a087215 Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from f3e5bb2 to b72f457 Compare July 11, 2026 12:51
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch 2 times, most recently from 802df15 to 4d17f3c Compare July 11, 2026 21:08
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch 2 times, most recently from 50e9bb0 to d103af6 Compare July 11, 2026 22:38

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at current head d103af6: diff remains unwired pure step math, prior tests and contracts intact; required checks green. Approving.

@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 138ede6 to 0d1397b Compare July 12, 2026 01:03
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from d103af6 to ee960b6 Compare July 12, 2026 01:03
What: domEditNudge — pure nudge-step math (direction × modifier → delta,
commit debounce constant) with tests. Shipped unwired.

Why: small standalone concept consumed by the canvas nudge hook in the
chrome-components PR.

How: new files; one TEMP(studio-dnd) ignoreExports entry until the consumer
lands (removed by the app-shell swap PR).

Test plan: bunx vitest run domEditNudge.test.ts; tsc --noEmit; fallow audit
clean.
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 0d1397b to 8229961 Compare July 12, 2026 01:22
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from ee960b6 to 951fda3 Compare July 12, 2026 01:22

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head and verified the addressed feedback and required behavior. Approval is for this exact head; Graphite mergeability may still be pending.

@miguel-heygen miguel-heygen deleted the branch studio-dnd/pr11-canvas-zorder-menu July 12, 2026 02:56
@miguel-heygen miguel-heygen deleted the studio-dnd/pr12-canvas-nudge-math branch July 12, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants