feat(studio): asset click policy and canvas nudge gate#2200
Open
ukimsanov wants to merge 1 commit into
Open
Conversation
This was referenced Jul 11, 2026
Collaborator
Author
This was referenced Jul 11, 2026
ukimsanov
force-pushed
the
studio-dnd/pr08-timeline-zones
branch
from
f26cdeb to
95e9ca4
Compare
ukimsanov
force-pushed
the
studio-dnd/pr09-asset-click-nudge-policy
branch
2 times, most recently
from
af9f377 to
fc4382f
Compare
ukimsanov
force-pushed
the
studio-dnd/pr08-timeline-zones
branch
from
95e9ca4 to
d0aa2e0
Compare
miga-heygen
reviewed
Jul 11, 2026
miga-heygen
left a comment
miga-heygen
left a comment
Contributor
There was a problem hiding this comment.
Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga
vanceingalls
left a comment
vanceingalls
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: LGTM (green) — small, sharp, correctly bounded
Two pure helpers plus a module-level singleton claim gate. The scope is deliberately narrow; each unit is exercised at its edges. No unit-consistency traps (both files are pixel-only or dimensionless).
The pointed queries
- Click-vs-drag threshold (fair tie-break).
packages/studio/src/utils/assetClickBehavior.ts:73–:75—isPointerClick(dx, dy) = Math.abs(dx) < 4 && Math.abs(dy) < 4. Strict<at the boundary; exactly 4px is a drag, not a click. Deterministic and symmetric across axes. Test at :94–:98 pins the boundary in both directions (positive AND negative displacement); :100–:103 confirms the sign symmetry. findClipForAsset— matcher normalisation. :43–:61 handles the foursrcshapes documented byderiveUsedPaths(absolute URL,/api/projects/.../preview/...,./-prefixed, bare relative), plus query-string stripping and percent-decode. Every shape has a dedicated test at :26–:60. Theearliest startdeterministic pick at :62–:67 avoids returning an arbitrary duplicate. Robust.- Nudge gate — nesting semantics.
packages/studio/src/utils/canvasNudgeGate.ts:10–:26— module-levelclaims: numbercounter;acquireCanvasNudgeKeys()returns an idempotent release closure that decrements at most once. Test at :16–:24 verifies double-release is a no-op (guarded by thereleasedflag at :16). Correct for nested overlays. - Wiring with
#2203(canvas nudge math). No direct import at these PR SHAs —canvasNudgeGate.tsdoesn't referencedomEditNudge.tsand vice versa. That's expected: this PR ships the gate,#2203ships the delta math, and a downstream PR (per.fallowrc.jsoncmarkers,studio-dnd/pr22) wires both intoDomEditOverlay. As long as the eventual consumer acquires the gate BEFORE binding its arrow-key listener and releases it on unmount, the ownership arbitration between DomEditOverlay andusePlaybackKeyboard(documented at :5–:8) will hold. No coupling to inspect here.
Nits (non-blocking)
- The module-level
claimscounter incanvasNudgeGate.tspersists across tests within a Vitest worker. Currently both tests clean up to zero, but there is nobeforeEach(() => { /* reset */ })— if a future test throws mid-way and leaks a claim, the next test's initialexpect(claimed()).toBe(false)would fail without an obvious signal. AbeforeEachguard, or exposing a__resetForTests()helper, would future-proof this. Cosmetic — the current tests happen to be well-behaved.
R1 by Via
ukimsanov
force-pushed
the
studio-dnd/pr08-timeline-zones
branch
from
d0aa2e0 to
b3f0ed0
Compare
ukimsanov
force-pushed
the
studio-dnd/pr09-asset-click-nudge-policy
branch
2 times, most recently
from
8e77fb1 to
ba28195
Compare
ukimsanov
force-pushed
the
studio-dnd/pr08-timeline-zones
branch
from
b3f0ed0 to
41f7e1d
Compare
ukimsanov
force-pushed
the
studio-dnd/pr09-asset-click-nudge-policy
branch
from
ba28195 to
ab2e3db
Compare
What: two small pure modules with tests — assetClickBehavior (click vs double-click policy for sidebar assets) and canvasNudgeGate (debounce gate for arrow-key canvas nudges). Why: policy dependencies of the upcoming asset card and nudge hook, reviewable as plain decision tables. How: new files only. Test plan: bunx vitest run on both test files; tsc --noEmit; fallow audit clean.
ukimsanov
force-pushed
the
studio-dnd/pr09-asset-click-nudge-policy
branch
from
ab2e3db to
b88fb09
Compare
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
3 participants
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
two small pure modules with tests — assetClickBehavior (click vs double-click policy for sidebar assets) and canvasNudgeGate (debounce gate for arrow-key canvas nudges).
Why
policy dependencies of the upcoming asset card and nudge hook, reviewable as plain decision tables.
How
new files only.
Test plan
bunx vitest run on both test files; tsc --noEmit; fallow audit clean.
Stack position 9/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.