Repository navigation
feat(studio): hosts can ask which action holds a key combination - #6979
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit · head 78f6037d
The code looks right: studioKeyClash uses the same comboClashKey and sharedByDesign rules as findClash, so it agrees with setStudioKeys. The only blocker is the PR body.
- blocker — required check
Studio and player capturesfails: the PR touchespackages/studio, and the body has no## Before/## After. The change is 19 counted lines and has no.tsx/.css, so the check accepts a## No visible changeheading instead. Turning the body's "No visual change: no UI code changed" line into that heading should clear it. Because the merge loop only waits for approval, I'm not approving while a required check is red. - nit —
studioShortcuts.ts:125:parseCombo(keys[other])!throws aTypeErrorinsidecomboClashKeywhen a host passes akeysmap with a value that doesn't parse, such as a stored"".findClashnever sees this becauseresolve()validates first. Skipping or treating unparseable entries as "no clash" would make the exported helper safe for raw host maps.
Edit accuracy: accurate 271 (base branch 271), smooth 121 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 open from before · 78f6037d → 6a5717c9
✅ The unparseable-entry nit is fixed, and the new test case covers it.
- blocker — required check
Studio and player capturesis still red: the## No visible changeheading is there now, but the new guard line takes the diff to 20 counted lines underpackages/studio. The check only accepts that heading under 20 ("the limit is under 20"). To clear it, cut one counted line, for example by folding the null check and the comparison into a single expression, or add## Before/## Aftercaptures.
6a5717c to
21647c3
Compare
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 1 resolved · 6a5717c9 → 21647c3d
✅ The required check Studio and player captures passes now. The rebase left the two earlier commits unchanged (git range-diff). The rewrite of studioKeyClash still returns null for text that doesn't parse and skips map entries that don't parse, so behavior is the same. Required CI was still running at review time.
What
Hosts that remap Studio's shortcuts can now ask which Studio action already holds a key combination, and parse, format and match combinations the same way Studio does. Exported from the package entry:
studioKeyClash(action, text, keys?),parseCombo,formatCombo,matchComboand theCombotype.Why
A host's Settings screen has to refuse or offer a swap when a person picks a key another action holds. Without this the host copies Studio's clash rules, including the pairs that share a key on purpose, and the two drift.
How
studioKeyClashreturns the other action on the same modifiers and key, ornull. It skips pairs Studio shares by design (for example stop and add keyframe on K), so it agrees with the checksetStudioKeysalready runs. Nothing else changes.Test plan
studioCombo.test.ts: a combo held by another action is named, Shift changes the answer, a shared pair is allowed, a combo freed by the passed map is not a clash.What I measured
On a Linux box:
studioCombo.test.tsandstudioShortcuts.test.tsxpass (18 tests);oxfmt --check .,oxlint,fallow audit --base origin/mainandtsc --noEmitare clean.What I did NOT exercise
No host uses it in this PR.
No visible change
No UI code changed; the helper only answers a question.