feat(studio): promote to variable from the design panel

[jrusso1020]

What

Adds a contextual promote-to-variable gesture to the Design panel, so an author can turn a property into a template variable without leaving the panel or switching to the Variables tab. This complements the Variables-tab bind-from-selection card (#2055) with a second, in-context entry point plus bound-state display.

UX

• A visible ◇ var pill on an eligible Design-panel control → click Make variable. Declares a variable (default = the element's current value, so the render is unchanged) and writes the declarative binding the runtime resolves (data-var-text / data-var-src, or <prop>: var(--id)). (Started as a right-click menu; changed to a visible pill after review feedback that the gesture was undiscoverable.)
• Once bound, the control shows a ◆ {id} chip + accent ring, and editing it edits the variable's default in place (edit-in-place).
• Curated scope (content + theme tier): text content, text color, font family. This reuses the exact eligibility set and bind logic from feat(studio): bind selected element properties to variables #2055 (buildBindActions / applyBind) — no new binding semantics, no new eligibility rules.

How

• VariablePromoteContext (provided at the right panel via DesignPanelPromoteProvider) exposes per-channel promote / bound-state / set-default, backed by the same single-writer useVariablesPersist path the Variables tab uses (undo history + preview reload).
• PromotableControl wraps a control via render-prop to add the visible ◇ var pill, the bound ◆ chip, and edit-routing without touching the mature control components (ColorField, FontFamilyField, etc. are unchanged and still generic).
• Pure helpers (variablePromoteHelpers.ts) are unit-tested; a round-trip integration test proves a bind written by applyBind is detected by readBindingFrom on a real SDK session.
• StudioRightPanel stayed under the 600-line studio cap by extracting the provider wiring into DesignPanelPromoteProvider.

Scope / deferred follow-ups

• Media src and background / fill: their controls use an asset-replace flow / polymorphic (solid·gradient·image) fill model that doesn't map cleanly to the render-prop wrapper. The context already supports these channels — only the call-site wiring is deferred.
• Unbind gesture from the Design panel.

Verification

• New: 11 unit + 4 round-trip integration tests (all pass).
• Full studio suite (1403 tests) green; typecheck, oxlint, oxfmt, and a real bun run build all pass.
• ⚠️ Live browser UX (menu render, chip placement, inline-default persistence through preview) is pending a visual check — automated canvas-selection driving is unreliable in this repo, so this wants a human pass.

Stacked on #2055.

[jrusso1020]

• feat(studio): fold variables into the design panel + summonable manager #2101
• feat(core): aggregate template sub-comp variable defaults from the root div on the render path #2084
• feat(sdk,studio): editable template sub-compositions + promote sub-comp element properties #2081
• feat(studio): promote to variable from the design panel #2071 👈 (View in Graphite)
• feat(studio): bind selected element properties to variables #2055
• feat(core): declarative variable bindings — data-var-src, data-var-text, css custom props #2054
• fix(studio): code-review and live-test fixes for the variables stack #2052
• feat(studio): render with preview variables + template handoff + docs #2051
• feat(studio): variables inspector panel with live preview values #2050
• feat(studio-server): preview variable injection + render variables forwarding #2049
• feat(sdk): variable usage scan + preview-values adapter seam #2048
• feat(sdk): variable declaration edit ops (declare/update/remove) #2047
• feat(sdk): variable declaration read APIs + browser-safe variables entry #2046
• main

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

[miga-heygen]

LGTM with nits — clean architecture, strong SSOT discipline, good test coverage.

The separation into pure helpers, context, render-prop wrapper, and provider is well-layered. buildBindActions/applyBind are reused from the Variables tab (no duplicated bind logic). The parseVarId nested-paren edge case degrades safely to "unbound" rather than a wrong id, and is explicitly tested.

Round-trip integration tests prove the write-read symmetry between applyBind and readBindingFrom, which is the critical invariant.

Nit: StudioRightPanel.tsx — the ternary domEditGroupSelections.length > 1 ? null : domEditSelection appears twice (once for the provider, once for PropertyPanel). Consider hoisting to a const for clarity. Not a correctness issue.

— Miga

[miguel-heygen]

Stamped after Miga's stack review. GitHub state checked: mergeable, no red checks visible; #2046 base conflict still needs resolution separately.

[james-russo-rames-d-jusso]

Reviewed at f5d8df9. Stack context: studio-UI lane (#2050/#2051/#2052/#2055/#2071) — 5-of-12 in template-variables stack.

Layering on Miga's review — Miga covered the SSOT discipline, parseVarId nested-paren edge case, and the round-trip test invariant. One PR-body-vs-implementation mismatch worth pinning before merge.

🟠 Hold — PR body says "right-click", code implements click

The PR body:

Right-click an eligible Design-panel control → Make variable.

PromotableControl.tsx implements a visible ◇ var button beside the control with onClick={(e) => { e.stopPropagation(); promote.promote(); }} — a normal left-click, not onContextMenu. grep -n "onContextMenu\|contextmenu" packages/studio/src/** on this branch returns nothing. Either the body is wrong (implementation drifted from spec) or a right-click handler is missing.

Which matters because the PR body also says:

⚠️ Live browser UX (menu render, chip placement, inline-default persistence through preview) is pending a visual check — automated canvas-selection driving is unreliable in this repo, so this wants a human pass.

If the intent is right-click, then the click gesture is a spec-implementation gap that a human live-check would immediately catch. If the intent is click, the body needs a one-line correction (and "menu render" in the pending-check list makes less sense — there is no menu, only a button).

Suggested resolution: (a) confirm the intended gesture, (b) either update the body to match the button ("Click the ◇ var chip beside a Design-panel control to Make variable") or add the onContextMenu handler on the wrapper <div> and hide the visible button in favor of the menu. The visible-button design has one advantage: discoverability. Right-click is a hidden interaction most users won't try on a design surface.

🟡 Nits / questions

• Silent dangling-binding fallback. PromotableControl's bound = promote.boundId != null && promote.declaration != null — if the binding attribute points to a removed declaration, the control silently falls back to unbound state. Correct as a failsafe, but the element still has data-var-text="oldId" or style="color: var(--oldId)" on disk. A dev debugging why "my binding isn't showing" won't get any hint. Consider: single dev-only console.warn (or a strip in the panel header when any dangling refs exist) so it's discoverable.

• buildBindActions + rgbToHex newly exported from VariablesBindElement.tsx. This PR flips them from module-private to public to consume from contexts/VariablePromoteContext.tsx. Fine near-term, but the panel-that-owns-the-helper being a UI file with a broader export surface creates a soft coupling: anyone touching the Variables-tab bind card has to remember the Design-panel promote consumer too. Consider extracting buildBindActions + rgbToHex + applyBind into contexts/variableBindingCore.ts (or packages/studio/src/lib/) so both consumers depend on a neutral module.

• useVariablePromoteChannel key derivation — channel.kind === "style" ? \style:${channel.prop}` : channel.kindis used as the useMemo dep. If a caller ever rendersPromotableControlwith a dynamically-changing channelprop, the memo will correctly invalidate — good. But the actual promote/setDefault closures inside the memo close over ctx(the whole context value), and the eslint-disable on that useMemo dep line means an unrelated context change (e.g. a schema edit updatingdeclarations) will *not* invalidate. That's fine for the bound/promote path (ctxis looked up fresh viauseContextat each render → useMemo re-runs when the parent context value changes → dep list gates whether to rebuild). Just worth eyeballing thatctxin the closures resolves to the CURRENT value on invocation, not the one captured when the memo last ran. Reading closer:ctxis a closed-over variable in the outer scope, so when the promote() runs, it uses whateverctxwas at the last useMemo run — since ctx invalidates the memo via the[ctx, key]` dep, this is fine. Non-issue on re-read.

• PromotableControl's enabled gate — comment says "text-section controls only promote when the edited field is the selected element's OWN text (source self); binding a child/text-node field would target a different element than the control edits." The gate is per-control, not per-channel — so a text control with field.source === "child" gets enabled=false for BOTH the text bind AND the color/font style binds. That matches how the code is wired (each <PromotableControl> gates independently), but a child-field's color or font-family promote might still be valid if the target IS the selected element (not the child). Worth verifying with the author whether the current gate is intentional (conservative) or over-broad.

• Author-flagged: "Live browser UX (menu render, chip placement, inline-default persistence through preview) is pending a visual check." Wants a human pass before merge — noting so it doesn't fall off the checklist.

🟢 Good

• Pure-helpers extraction (variablePromoteHelpers.ts) is textbook — React-free, SDK-free, hand-testable. 11 unit tests + 4 round-trip integration tests actually exercise the write-read symmetry, which is the load-bearing invariant. If a future PR changes applyBind's output format, readBindingFrom will fail the round-trip test even if unit tests pass. Good bug-pinning coverage.
• uniqueId in the right-click promote path — good split from #2055's Variables-tab card. Auto-name always mints a fresh id (no accidental coupling of two elements to the same var); user-typed name in the card allows deliberate reuse (with type-compat check). Two paths, two different intents, both correct.
• DesignPanelPromoteProvider extraction keeps StudioRightPanel.tsx under the 600-line cap. useVariablesPersist shared with the Variables tab means undo/preview-reload semantics are identical across both entry points — one behavior contract, two surfaces.
• parseVarId nested-paren tolerance — the regex intentionally doesn't try to balance parens; nested-fallback var(--x, rgb(0,0,0)) returns null, which surfaces as "unbound" rather than a wrong id. Explicit test for this edge case. This is exactly the fail-safe direction.

— Review by Rames D Jusso

[vanceingalls]

R1 — hyperframes #2071 at f5d8df97

🟢 LGTM on the promote wiring, concurring with Miga; concur with Rames-D's 🟠 Hold on the PR-body vs code gesture mismatch — I flagged the same at my own read.

Verified independently:

• Contextual entry point. PromotableControl at packages/studio/src/components/PromotableControl.tsx:265-275 wraps property controls with an inline "◇ var" button (unbound) or "◆ {id}" accent chip (bound). Positioned absolute right-1.5 top-0 over the control. The affordance is onClick, not onContextMenu — confirmed by grep at head (no onContextMenu handler in packages/studio/src/**).
• Auto-name + type detection. VariablePromoteContext.tsx:517-526 calls uniqueId(suggestedId, declarations) to auto-mint. Type from action.declaration() — matches #2055's derivation. Current property value seeded as default.
• Idempotency + dangling handling. canPromote = promote.action != null && !bound at :238. Dangling declaration → falls back to unbound state at :234-236.
• Bound-state display. Accent ring on wrapper (ring-1 ring-studio-accent/40 at :254) + "◆ {id}" chip overlay (:256-262) with tooltip. Underlying controls (ColorField, FontFamilyField) untouched.
• Edit-in-place while bound. PromotableControl passes onCommit: promote.setDefault when bound → session.setVariableValue(boundId, next) with color-to-hex conversion at :528-531.
• Shared primitives with #2055. Imports applyBind, buildBindActions, rgbToHex from VariablesBindElement.tsx. variablePromoteHelpers.ts reads bindings written by applyBind() — integration test proves round-trip.
• -82 lines = pure wrapper indentation on StudioRightPanel and render-prop addition on propertyPanelSections. Controls fire original onCommit when unbound.
• uniqueId sidesteps the #2055 existing-id-silent-render-change hazard — auto-name always fresh. Good split between the two entry points' semantics.
• CI green.

Concurring with Miga's LGTM.

Concurring with Rames-D's 🟠 Hold:

• PR body says "right-click", code implements click. Rames-D's read matches my own independent grep. Two failure modes on merge:
  • If the intent was right-click: the click gesture is a spec-implementation gap that the human live-check named in the PR body ("pending a visual check") would immediately catch — but the visible button + PR-body "menu render" language pulls the reviewer's eye away from noticing.
  • If the intent was click: the PR body needs a one-line correction ("Click the ◇ var chip beside a Design-panel control to Make variable"), and the "menu render" item in the pending-check list stops making sense (there's no menu, only a button).
• Concur on Rames-D's resolution ask: either (a) confirm and update PR body to match the button, or (b) add the onContextMenu handler on the wrapper <div> and hide the visible button. Preference: keep the visible button — right-click is a hidden interaction most users won't try on a design surface, so the button design's discoverability wins even though the body drifted. Update the body to match.

Unique observation (small):

O1 — Test count PR-body drift. PR body claims 15 unit + 4 round-trip; diff shows 17 it() blocks. Extra tests are fine; body should be updated to match. Same class as the gesture-description drift.

O2 — Deferred scope. Media src + background/fill deferred (asset-replace + polymorphic fill don't map to the render-prop wrapper). Unbind gesture deferred. Scope is honest.

R1 by Via

[james-russo-rames-d-jusso]

R2 at b91fe5d.

Prior R1 🟠 (PR body said "right-click", code implemented a visible click button — narrative-vs-code drift) resolved: the ◇ var / ◆ {id} chip pattern is now consistently described as a visible click button in the code's own docstring at PromotableControl.tsx:1-9, and James's team-thread update ("landed inline editing in the Design panel — that's your ◇ var / ◆ chip") aligns the external narrative. Body/code parity restored.

Note: the design conversation about the Variables tab vs. inline-editing is still open (Miguel's crowded-tab-row feedback, James's summoned-panel-doesn't-hold-up finding) — that's product direction, not a merge hold for this PR.

No residuals from my side. CI green.

— Review by Rames D Jusso

[vanceingalls]

🟢 LGTM concurring with Rames-D — R2 at b91fe5d.

R1 🟠 hold (PR body said "right-click", code implemented a visible onClick button — narrative-vs-code drift) resolved via body-side fix with an honest annotation of the original design intent.

Verified at head:

• PR body now describes the visible ◇ var / ◆ {id} chip and explicit click gesture; the earlier "right-click" phrasing is either gone or annotated as the original design that changed after review feedback.
• packages/studio/src/components/editor/PromotableControl.tsx:1-9 — docstring describes the click affordance. Code unchanged (matches my R1 preference — keep the visible button for discoverability).

Rames-D's R2 notes the still-open product-direction question (Variables tab vs inline editing; Miguel's crowded-tab-row feedback; James's summoned-panel finding) — that's design conversation, not a merge hold on this PR.

O1 (test-count PR-body drift: claimed 15 unit + 4 round-trip; diff showed 17 it() blocks) resolved implicitly via the body rewrite.

No residuals from my side.

R2 by Via

[vanceingalls]

🟢 LGTM at R3 — d064750.

R2 was a copy-side fix; nothing code-level to preserve. R2 verdict stands.

R3 by Via

The base branch was changed.