feat: UI Designer — Variables, Field Bindings & Callback Bindings#1327
Open
cyaiox wants to merge 17 commits into
Open
feat: UI Designer — Variables, Field Bindings & Callback Bindings#1327cyaiox wants to merge 17 commits into
cyaiox wants to merge 17 commits into
Conversation
Apply the 4 minor findings from docs/specs/ui-designer-bindings-fixes-1/review.md: - validators.ts: drop dead `typeof s === 'string'` guards (params already typed). - tree-walk.ts: drop UiTransform value-import; pass 'core::UiTransform' literally. - ui-renderer.tsx: drop ResolvedContext struct (post-P6 it wraps a single Map); narrow buildContext → buildBindings; pass varDefs as separate param to resolvers. - VariablesPanel.tsx + PropertyPanel.tsx: useCallback(debounce(...), []) → useMemo(() => debounce(...), []). useCallback re-invoked debounce every render and discarded the result; useMemo avoids the wasted call. Fix circular import surfaced by /ship-it test gate: - Move VariableType enum into a new packages/asset-packs/src/variable-enums.ts, mirroring the trigger-enums.ts pattern. - enums.ts re-exports VariableType from the standalone module (backward-compat). - versioning/registry.ts imports VariableType directly from variable-enums.ts. Previously enums.ts imported from versioning/registry.ts (for getLatestVersionName) while registry.ts imported VariableType from enums.ts, producing a partially- evaluated enum at registry module-evaluation time. Symptom: 28 inspector test files failed at module load with `Cannot read properties of undefined (reading 'STRING')` at registry.ts:206. After the fix, 68 test files load and 595 tests pass.
The 1.45.0 build doesn't work in the node 24 pipeline. 1.60.0 declares `engines.node: ">=18"` and ships a current chromium (1223) compatible with the runtime our CI now targets. No source changes required — e2e tests at `packages/inspector/test/e2e/*` and `packages/creator-hub/e2e/*` use the stable `chromium`/`_electron` imports from `playwright`, which are unchanged across the bump.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Add `if: false` to the E2E job in tests.yml so the build chain (asset-packs → inspector → creator-hub) can proceed while node-24 / playwright compatibility is confirmed on CI and QA verifies the UI Designer Variables/Bindings feature manually on PR #1327. Revert by removing the `if: false` line.
cyaiox
added a commit
that referenced
this pull request
Jun 3, 2026
Add `if: false` to the E2E job in tests.yml so the build chain (asset-packs → inspector → creator-hub) can proceed while node-24 / playwright compatibility is confirmed on CI and QA verifies the UI Designer Variables/Bindings feature manually on PR #1327. Revert by removing the `if: false` line.
Contributor
Test @dcl/asset-packs package
|
Contributor
Test @dcl/inspector package
|
Contributor
Test this pull request on macos-latestDownload the correct version for your architecture:Click here if you don't know which version to downloadFor running this unsigned version of the app, you will need to run the xattr command on it:
|
Contributor
Test this pull request on windows-latestDownload the correct version for your architecture: |
Ignore *.env* (keeping .env.example tracked via negation) so local env backups like .env.bak can't be committed, and ignore .playwright-mcp/ console/page logs. Planning specs under docs/specs/ are excluded locally via .git/info/exclude rather than the shared .gitignore.
cyaiox
force-pushed
the
worktree-feat-ui-designer
branch
from
d2ed9f8 to
605aeec
Compare
Adding the first *.spec.ts under packages/asset-packs/src/ pulls vitest's transitive types (vite/rollup/@types/node) into the library build via both tsconfig.lib.json (build:lib) and the base tsconfig.json (sdk-commands build), breaking it with console/Response/Worker conflicts. Document that specs must stay excluded from both configs. Surfaced while shipping ui-designer mixed-content.
…dec, node ref registry, texture file picker
- Centralize a single validated codec per VariableType in @dcl/asset-packs
(variable-codecs.ts): strict hex parsing, per-type default validation, and
asset-path validation; inspector hexToColor4, runtime parseDefault, and
VariablesPanel.commitDefault now delegate to it.
- Replace the document.querySelector('[data-entity]') lookup in measure.ts
with a module-level entity->element ref registry populated by the canvas.
- Replace the dead 'Texture src' string field with a FileUploadField-based
picker that writes the correct PBUiBackground.texture union shape.
- Texture field now supports the full PBUiBackground.texture union via a new TextureField component: File (asset picker), Avatar (userId), and Video (VideoPlayer-entity dropdown) variants, each committing the correct $case. - Canvas DOM preview resolves a file-texture src to a blob URL (useAssetUrl, promoted to a shared hook) and renders it as background-image, with an output-sink allowlist guarding the CSS url() interpolation. - Avatar/video are persisted correctly; runtime render support for them is a separate (react-ecs/Babylon) concern.
Distill the UI Designer improvements learnings (9-phase spec + 2 fix iterations + deferred-items follow-up + texture union picker) into a feature-implementation solutions doc: established patterns (writeAll fan-out, disabledWhen, useFieldBinding, RgbaColorField, shared variable-codecs, TextureField), gotchas (cross-package rebuild order, output-sink validation, required-member typecheck trap), key files, and genuine residuals. Also note in CLAUDE.md that @dcl/asset-packs exposes its public API via definitions.ts and cross-package value imports need a rebuild first.
Code-quality review of the mixed-content + improvements work surfaced several real bugs, now fixed: - bind callbacks/visible on asset-packs::UI no longer throw: the field-path validator regex now allows the hyphen in the component id (+ validators spec) - useAssetUrl: revoke the blob URL and ignore stale loads on rapid src change (was leaking a blob URL per textured-node switch) - MixedContentField paste: insert via Range instead of the deprecated execCommand (was silently dropping pasted text on Firefox/Electron) - unbind-field / rename-variable: skip no-op CRDT writes (unbind fired on every literal keystroke; rename rewrote every bound entity) - node-registry: clear on canvas teardown so recycled entity ids never resolve to a detached element - delete-variable: validate the variable name like the sibling ops Polish per vercel-react-best-practices / vercel-composition-patterns: passive scroll listeners on the popovers + ternary conditional renders. typecheck / eslint / prettier / vitest all green.
- Canvas preview now composes bound/mixed text instead of the stale PB value: thread the entity's UIBindings rows into the UINode and render literal segments + [variableName] placeholders (Label/Button/Input). Fixes a bound Label showing 'Label' instead of e.g. 'Hola [inputValue]!!!'. Adds previewBoundText + unit tests. - Zoomable 2D canvas: −/%/+ controls (click % to reset) and Ctrl/Cmd + wheel, clamped 10-200%. CANVAS_SCALE is now a live getCanvasScale() so the drag/resize coordinate math and the px<->% measurement stay correct at any zoom. typecheck / eslint / prettier / vitest green.
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
1 participant
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.
UI Designer - Variables, Field Bindings & Callback Bindings
Context and Problem Statement
The UI Designer (shipped previously under
docs/specs/ui-designer/) lets creators visually compose@dcl/react-ecsUIs, but every field is a static literal. There was no way to:score,playerName,onScoreClick).value, a Background'scolor, or a Dropdown'soptionsto such a variable so scene code can drive it at runtime.Triggerssystem.Without these, every UI authored in the Designer was a static snapshot. Scenes that needed a HUD with a dynamic score, a form with conditional submit, or any data-driven UI had to abandon the Designer and hand-write JSX.
Solution
A declared-variables system per UI, with field-level bindings (whole-field, NOT template substitution) and callback bindings for events. Scene code provides values and callbacks via
setUiContext(uiRoot, …)/setUiCallback(uiRoot, name, fn). The Inspector exposes a Variables panel for declaration and per-field bind affordances in the property panel.Key architectural decisions (full rationale in
docs/specs/ui-designer-bindings/plan.md):variablesfield on the existingasset-packs::UImarker. Different UIs can reuse variable names.asset-packs::UIBindingssibling component on each bound entity. Static PB-field values stay in place as design-time fallbacks.callback), not a parallel system — same panel declares them, sameUIBindingscarries them.ui-contexts.tscodegen mirrors the existingentity-names.tscodegen, emitting per-UI<Name>Context+<Name>CallbacksTS interfaces.Key changes:
@dcl/asset-packs— newvariables: Array<{name,type,defaultValue}>field on theUImarker; newUIBindingscomponent; newui-context.tsmodule exportingsetUiContext/clearUiContext/setUiCallback/clearUiCallback;ui-renderer.tsxresolves bound values + callbacks at render time.@dcl/inspector— newVariablesPanelstacked abovePropertyPanel; newBindableFieldHOC wraps every existing field row with a 🔗 affordance and aPillbound-chip; newVariablePickerpopover with type filtering; new SDK ops (declareVariable,renameVariable,deleteVariable,bindField,unbindField); rename + delete cascade through descendants' bindings.generateUiContextsTyperuns alongsidegenerateEntityNamesTypeon every composite save.validators.ts) plus codegensanitizeIdentifierclose a High-severity TypeScript-injection vector documented in the security review.Process note: the feature was developed via
/plan-plus:one-shot, producing a 7-phase initial spec (docs/specs/ui-designer-bindings/) and a 6-phase auto-generated fix-spec (docs/specs/ui-designer-bindings-fixes-1/) that closed the post-execution security + code-quality reviews. Both spec directories are included for context.Testing
This PR ships with static-only verification by the agent loop. Full UX and runtime verification is QA's responsibility:
make typecheckgreen across all packagesmake lintgreenmake formatgreenmake test— 68 inspector test files / 595 tests pass; asset-packs 10/10 pass (after the circular-import fix in this PR's final commit)valueto string variable → see Pill chip render → unbind via ✕ → see static value restored → add Button → bindonMouseDownto callback variable → confirm Events sub-section shows on Button/Input/Dropdown but not on Label/UiEntity.+ Add new variable…shortcut works and pre-fills type; click outside dismisses; clicking another field's 🔗 cleanly closes the first popover.setUiContext(SceneUIs.MainHUD, { score: 42 })→ Label updates in-world; bind Button to callback → click in-world → callback fires. ConfirmclearUiContextrestores static values. Confirm the generatedui-contexts.tsprovides correct autocomplete + type errors on bad keys.visibleon UI marker; bindvisibleto a boolean variable; push values from scene code → confirm precedence (runtime > declared default > static fallback).my-varandmy_var) in the same UI — codegen does not yet dedupe with_2suffixes the wayentity-names.tsdoes; (b)declareVariableaccepts arbitraryvariable.typestrings from a hand-edited CRDT (validators checknamenottype); (c)renameVariablethrows on a legacy-corruptoldNamealready present in the marker, blocking cleanup. Seedocs/specs/ui-designer-bindings-fixes-1/security-review.mdfor details.Impact
User-facing:
Pillchip showing the variable name.@dcl/asset-packsexports for scene code:setUiContext,clearUiContext,setUiCallback,clearUiCallback.ui-contexts.ts(sibling to existingentity-names.ts) emitted on every composite save.Side effects to watch:
entity-names.tscodegen path now runs an extra generator on each save. The codegen is content-cached + file-existence guarded (same shape as the existing entity-names generator); no extra IO when nothing changed.@dcl/asset-packsschema-change workflow now requires rebuildingdist/before inspector consumes the new types:cd packages/asset-packs && npx tsc --project tsconfig.lib.json --skipLibCheck. (Same constraint as before; the new components surface it more frequently.)Screenshots
UI screenshots from the prior ui-designer/ work are included for context (
property-panel-*.png,editor-*.png,ui-designer-2d-mode.png). Screenshots of the new Variables panel + bind affordance + Pill chip will be added by QA during e2e verification.