From babed0c8b95adf3c94534f12bceebdd42b6b3a98 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:36:31 +0000 Subject: [PATCH 1/3] feat(viz-popover): grey out a control until the toggle it depends on is on A viz control descriptor can now set dependsOn: ''. The Viz popover disables and dims that row, with a "Requires ..." tooltip, until the toggle is on, and follows later clicks and saved state. 3D Highway's "Locked zoom" does nothing unless "Lock camera at frets 1-12" is on, so it looked broken in a panel. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH --- CLAUDE.md | 2 +- plugin.json | 2 +- screen.js | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 756f4ae..7d9209f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -207,7 +207,7 @@ Each panel is always in exactly one of these modes. Flags are mutually exclusive When a panel is in viz mode, splitscreen shows a `vizSettingsBtn` ("Viz ⚙") that opens `vizPopover` — a small popover with controls that override that viz plugin's settings **for this panel only**. The controls are generated from a descriptor, so adding a new per-panel option doesn't require touching the popover code. -- **Descriptor lookup** — `getPanelControlsFor(pluginId)` first reads `settings` from the matching provider in `window.feedBack.vizDomain.snapshot().providers`. Any visualization can declare these in `capabilities.visualization.settings` and implement `applySetting(key, value)` on each renderer instance. Descriptors use `{ key, label, type:'toggle'|'range'|'select', default, min?, max?, step?, options? }`, with `options` as `[{id,label}]`. The old factory `panelControls` and built-in highway_3d descriptor remain as compatibility fallbacks. An empty declared array hides the button. +- **Descriptor lookup** — `getPanelControlsFor(pluginId)` first reads `settings` from the matching provider in `window.feedBack.vizDomain.snapshot().providers`. Any visualization can declare these in `capabilities.visualization.settings` and implement `applySetting(key, value)` on each renderer instance. Descriptors use `{ key, label, type:'toggle'|'range'|'select', default, min?, max?, step?, options? }`, with `options` as `[{id,label}]`. A descriptor may also set `dependsOn: ''`; the popover then greys out and disables that control (with a "Requires …" tooltip) until the named toggle in the same list is on. This is honoured for the legacy factory `panelControls` path; core's manifest validation may strip unknown keys from declared `capabilities.visualization.settings`, so don't rely on it there. A missing or non-toggle target is ignored and the control stays usable. The old factory `panelControls` and built-in highway_3d descriptor remain as compatibility fallbacks. An empty declared array hides the button. - **Provider-refresh reconciliation** — core loads plugin scripts *before* `loadLibraryProviders()` publishes providers, so a panel can initialize against an empty snapshot: descriptor lookup finds nothing, so the value is never restored and the button never appears. `_reconcileVizProviders` runs the init-time sequence (restore saved values, then reveal + build the popover) for every live viz panel whose plugin has since declared controls, driven by `window.feedBack.capabilities.subscribe('visualization:providers-refreshed', …)` (the unsub is dropped — the subscription lives as long as the IIFE). Core normalises an empty declared array to absent, so a provider can't use `settings: []` to suppress the legacy fallback. - **Storage** — capability settings are scoped to renderer instances. Splitscreen persists them under `splitscreenVizSetting:::` and reapplies them when it creates a replacement renderer. The renderer's optional `getSetting(key)` supplies the value the popover *shows* when no override is saved, but `_restoreVizSettings` only re-applies values this panel actually saved (`_vizSavedSetting`). Applying `getSetting()`'s value back would pin an override equal to the plugin's current global, and the panel would stop following later global Settings changes. Legacy highway_3d controls continue using `h3d_bg_panel_` and its global setter event to preserve existing preferences. - **Panel identity (`vizId`)** — the middle segment is a random id minted lazily on the panel's first declared-control read/write (`_vizId`), **not** the panel's array position: pop-out compacts the survivors and a popup builds its own slot 0, while localStorage is shared between the windows, so an index-keyed value follows the slot and can be read or overwritten by an unrelated panel. A per-window counter doesn't help either (main's first panel and a popup's first panel would both draw from their own `1`). The id is carried through panel prefs (`panelToPrefs` → `initPanel`) and follower config (`_captureFollowerConfig` → pop-out URL / `docked` payload / relay `config`), so it survives layout rebuilds, pop-out/dock and a follower's own re-split; follower sub-panels created beyond the incoming cfgs get their own. Numeric-slot keys from the pre-release scheme are deliberately **not** migrated — that mapping is exactly the ambiguity being removed. diff --git a/plugin.json b/plugin.json index 658f835..db74095 100644 --- a/plugin.json +++ b/plugin.json @@ -1,7 +1,7 @@ { "id": "splitscreen", "name": "Split Screen", - "version": "1.15.0", + "version": "1.16.0", "private": false, "standards": ["capability-pipelines.v1", "plugin-runtime-idempotent.v1"], "capabilities": { diff --git a/screen.js b/screen.js index c205a9d..7bf6bf6 100644 --- a/screen.js +++ b/screen.js @@ -2081,6 +2081,9 @@ try { title.textContent = (vizPlugins.find(p => p.id === pluginId)?.name || pluginId) + ' — this panel'; title.style.cssText = 'font-size:10px;color:#6b7280;margin-bottom:6px;white-space:nowrap;'; pop.appendChild(title); + // key -> { row, input } so a control that declares `dependsOn` can be + // greyed out while the toggle it depends on is off. + const built = Object.create(null); for (const ctl of controls) { const row = document.createElement('label'); row.style.cssText = 'display:flex;align-items:center;gap:8px;margin:4px 0;font-size:11px;color:#cbd5e1;white-space:nowrap;cursor:pointer;'; @@ -2095,6 +2098,7 @@ try { cb.onchange = () => _vizPanelSet(pluginId, panels.indexOf(panel), ctl, cb.checked); row.appendChild(name); row.appendChild(cb); + built[ctl.key] = { row, input: cb }; } else if (ctl.type === 'range') { const { lo, hi, st } = _ctlRange(ctl); const sl = document.createElement('input'); @@ -2113,6 +2117,7 @@ try { row.appendChild(name); row.appendChild(sl); row.appendChild(val); + built[ctl.key] = { row, input: sl }; } else if (ctl.type === 'select') { const sel = document.createElement('select'); sel.style.cssText = 'background:#1a1a2e;border:1px solid #333;border-radius:4px;padding:2px 4px;font-size:10px;color:#ccc;outline:none;'; @@ -2125,11 +2130,40 @@ try { sel.onchange = () => _vizPanelSet(pluginId, panels.indexOf(panel), ctl, sel.value); row.appendChild(name); row.appendChild(sel); + built[ctl.key] = { row, input: sel }; } else { continue; } pop.appendChild(row); } + _wireVizControlDeps(controls, built); + } + + /** + * Grey out a control whose descriptor sets `dependsOn: ''` + * until that toggle is on — e.g. 3D Highway's "Locked zoom" does nothing + * unless "Lock camera at frets 1-12" is enabled. Only toggles can gate; + * an unknown or non-toggle `dependsOn` is ignored, so the control stays + * usable. The gating checkbox is read live, so this follows both the + * saved value at build time and later clicks. + */ + function _wireVizControlDeps(controls, built) { + for (const ctl of controls) { + const dep = typeof ctl.dependsOn === 'string' ? ctl.dependsOn : null; + const me = built[ctl.key]; + const gate = dep && built[dep]; + const depCtl = dep && controls.find(c => c.key === dep); + if (!me || !gate || !depCtl || depCtl.type !== 'toggle') continue; + const apply = () => { + const on = !!gate.input.checked; + me.input.disabled = !on; + me.row.style.opacity = on ? '' : '0.4'; + me.row.style.cursor = on ? 'pointer' : 'not-allowed'; + me.row.title = on ? '' : 'Requires "' + depCtl.label + '"'; + }; + gate.input.addEventListener('change', apply); + apply(); + } } /** From a4fafb5a57978a26e97ce12668704c449333470a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:42:59 +0000 Subject: [PATCH 2/3] fix(viz-popover): make dependsOn greying cycle-safe and transitive, add tests A control that named itself, or sat on a dependsOn cycle, was disabled by its own gate and could never be re-enabled. A control behind a greyed-out gate also stayed live because only the gate's checked state was read. Treat self-references and cycle members as ungated, and compute usability through the whole gate chain. Cover the greying, saved state, ignored targets, self-reference, cycle and chain cases in the suite; the DOM stub now has disabled/title and a recording addEventListener variant. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH --- screen.js | 57 ++++++++++++++++------ tests/screen.test.js | 109 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 13 deletions(-) diff --git a/screen.js b/screen.js index 7bf6bf6..14a856f 100644 --- a/screen.js +++ b/screen.js @@ -2144,26 +2144,57 @@ try { * until that toggle is on — e.g. 3D Highway's "Locked zoom" does nothing * unless "Lock camera at frets 1-12" is enabled. Only toggles can gate; * an unknown or non-toggle `dependsOn` is ignored, so the control stays - * usable. The gating checkbox is read live, so this follows both the - * saved value at build time and later clicks. + * usable. + * + * A bad descriptor must never brick a control, so a control that names + * itself, or sits on a dependency cycle (A -> B -> A), is treated as + * ungated. Gating is transitive: a control behind a gate that is itself + * greyed out is greyed out too, so there is never an editable control + * behind an unreachable gate. Gates are read live, so this follows both + * the saved value at build time and later clicks. */ function _wireVizControlDeps(controls, built) { - for (const ctl of controls) { + const byKey = Object.create(null); + for (const c of controls) byKey[c.key] = c; + // The control `ctl` is gated by, or null when it has no valid gate. + const gateOf = (ctl) => { const dep = typeof ctl.dependsOn === 'string' ? ctl.dependsOn : null; - const me = built[ctl.key]; - const gate = dep && built[dep]; - const depCtl = dep && controls.find(c => c.key === dep); - if (!me || !gate || !depCtl || depCtl.type !== 'toggle') continue; - const apply = () => { - const on = !!gate.input.checked; + const depCtl = dep && dep !== ctl.key ? byKey[dep] : null; + return depCtl && depCtl.type === 'toggle' && built[dep] && built[ctl.key] ? depCtl : null; + }; + // Walking the gate chain from `key` comes back to `key`. + const inCycle = (key) => { + const seen = new Set([key]); + for (let g = gateOf(byKey[key]); g; g = gateOf(g)) { + if (g.key === key) return true; + if (seen.has(g.key)) return false; // a cycle further on, not through `key` + seen.add(g.key); + } + return false; + }; + const gated = controls.filter(c => gateOf(c) && !inCycle(c.key)); + if (!gated.length) return; + // Whether `ctl` is usable: no live gate, or its gate is on AND usable. + // Chains end because every cycle member is excluded from `gated`. + const usable = (ctl) => { + if (!gated.includes(ctl)) return true; + const gate = gateOf(ctl); + return !!built[gate.key].input.checked && usable(gate); + }; + const refresh = () => { + for (const ctl of gated) { + const on = usable(ctl); + const me = built[ctl.key]; me.input.disabled = !on; me.row.style.opacity = on ? '' : '0.4'; me.row.style.cursor = on ? 'pointer' : 'not-allowed'; - me.row.title = on ? '' : 'Requires "' + depCtl.label + '"'; - }; - gate.input.addEventListener('change', apply); - apply(); + me.row.title = on ? '' : 'Requires "' + gateOf(ctl).label + '"'; + } + }; + for (const gateKey of new Set(gated.map(c => gateOf(c).key))) { + built[gateKey].input.addEventListener('change', refresh); } + refresh(); } /** diff --git a/tests/screen.test.js b/tests/screen.test.js index f8cf3ad..0cc7494 100644 --- a/tests/screen.test.js +++ b/tests/screen.test.js @@ -1741,6 +1741,8 @@ function makeVizElementStub(tag) { textContent: '', value: '', checked: false, + disabled: false, + title: '', min: '', max: '', step: '', @@ -2216,6 +2218,113 @@ test('buildVizPopover creates a checkbox for toggle controls and fires onchange' assert.equal(global.localStorage.getItem('h3d_bg_panel0_cameraLockLow'), 'false'); }); +// ── buildVizPopover: dependsOn greying ─────────────────────────────────────── + +// makeVizElementStub discards addEventListener, so a `change` listener wired by +// buildVizPopover would never run. This variant records listeners and can fire +// them, which is what the dependsOn wiring needs. +function makeListeningVizElement(tag) { + const el = makeVizElementStub(tag); + const listeners = {}; + el.addEventListener = (type, fn) => { (listeners[type] = listeners[type] || []).push(fn); }; + el.fire = (type) => { (listeners[type] || []).forEach((fn) => fn()); }; + return el; +} + +// Build the popover for `controls` and return { key: { row, input } }. +// `saved` pre-sets per-panel values so a toggle can start ticked. +function buildDepsPopover(controls, saved = {}) { + const mod = freshVizPlugin(); + mod._setArrangementsForTest([{ name: 'Lead' }]); + window.feedBackViz_highway_3d = function () {}; + window.feedBackViz_highway_3d.panelControls = controls; + for (const [key, v] of Object.entries(saved)) global.localStorage.setItem('h3d_bg_panel0_' + key, String(v)); + const pop = makeVizElementStub('div'); + const panel = { vizPopover: pop, vizMode: 'highway_3d' }; + mod._setPanelsForTest([panel]); + const rows = []; + global.document.createElement = (tag) => { + const el = makeListeningVizElement(tag); + if (tag === 'label') rows.push(el); + return el; + }; + mod.buildVizPopover(panel, 'highway_3d'); + const out = {}; + controls.forEach((ctl, i) => { + const row = rows[i]; + out[ctl.key] = { row, input: row.children.find((c) => c.tagName === 'INPUT' || c.tagName === 'SELECT') }; + }); + return out; +} + +const DEP_TOGGLE = { key: 'lock', label: 'Lock', type: 'toggle', default: false }; +const DEP_RANGE = { key: 'zoom', label: 'Zoom', type: 'range', default: 0.5, dependsOn: 'lock' }; + +test('a control with dependsOn is disabled and dimmed until its toggle is on, and follows later clicks', () => { + const ui = buildDepsPopover([DEP_TOGGLE, DEP_RANGE]); + assert.equal(ui.zoom.input.disabled, true, 'starts disabled while the toggle is off'); + assert.equal(ui.zoom.row.style.opacity, '0.4'); + assert.match(ui.zoom.row.title, /Requires "Lock"/); + assert.equal(ui.lock.input.disabled, false, 'the gating toggle itself stays usable'); + + ui.lock.input.checked = true; + ui.lock.input.fire('change'); + assert.equal(ui.zoom.input.disabled, false); + assert.equal(ui.zoom.row.style.opacity, ''); + assert.equal(ui.zoom.row.title, ''); + + ui.lock.input.checked = false; + ui.lock.input.fire('change'); + assert.equal(ui.zoom.input.disabled, true, 'greys out again when the toggle is cleared'); +}); + +test('a dependsOn control starts enabled when its toggle was saved on', () => { + const ui = buildDepsPopover([DEP_TOGGLE, DEP_RANGE], { lock: true }); + assert.equal(ui.zoom.input.disabled, false); + assert.equal(ui.zoom.row.style.opacity, ''); +}); + +test('a missing or non-toggle dependsOn target is ignored and the control stays usable', () => { + const ui = buildDepsPopover([ + { key: 'a', label: 'A', type: 'range', default: 0.5, dependsOn: 'nope' }, + { key: 'r', label: 'R', type: 'range', default: 0.5 }, + { key: 'b', label: 'B', type: 'range', default: 0.5, dependsOn: 'r' }, + ]); + assert.equal(ui.a.input.disabled, false, 'unknown target'); + assert.equal(ui.b.input.disabled, false, 'range is not a valid gate'); +}); + +test('a control that depends on itself is not locked', () => { + const ui = buildDepsPopover([{ key: 'self', label: 'Self', type: 'toggle', default: false, dependsOn: 'self' }]); + assert.equal(ui.self.input.disabled, false, 'otherwise it could never be ticked again'); + assert.ok(!ui.self.row.style.opacity, 'and not dimmed'); +}); + +test('a dependsOn cycle leaves every member usable', () => { + const ui = buildDepsPopover([ + { key: 'a', label: 'A', type: 'toggle', default: false, dependsOn: 'b' }, + { key: 'b', label: 'B', type: 'toggle', default: false, dependsOn: 'a' }, + ]); + assert.equal(ui.a.input.disabled, false); + assert.equal(ui.b.input.disabled, false); +}); + +test('gating is transitive: a control behind a greyed-out gate is greyed out too', () => { + const ui = buildDepsPopover([ + { key: 'c', label: 'C', type: 'toggle', default: false }, + { key: 'b', label: 'B', type: 'toggle', default: false, dependsOn: 'c' }, + { key: 'a', label: 'A', type: 'range', default: 0.5, dependsOn: 'b' }, + ], { b: true }); + // B is saved on, but its own gate C is off, so A must not be live. + assert.equal(ui.b.input.disabled, true); + assert.equal(ui.a.input.disabled, true, 'checked-but-unreachable gate must not enable its dependent'); + + ui.c.input.checked = true; + ui.c.input.fire('change'); + assert.equal(ui.b.input.disabled, false); + assert.equal(ui.a.input.disabled, false, 'follows once the root gate is on'); +}); + test('buildVizPopover re-fires the plugin setter when its onchange handler runs', () => { const mod = freshVizPlugin(); mod._setArrangementsForTest([{ name: 'Lead' }]); From 9c79f4f3fcfbd4693dcceaf5a5efae1b4c595071 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:43:57 +0000 Subject: [PATCH 3/3] docs(viz-popover): document that dependsOn is dropped on the declared-settings path Add the missing @param tags on _wireVizControlDeps and state the caveat precisely: core whitelists declared setting descriptors in both layers, so dependsOn is silently dropped there and would stop working the day a plugin migrates from panelControls to declared settings. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH --- CLAUDE.md | 2 +- screen.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7d9209f..002e95c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -207,7 +207,7 @@ Each panel is always in exactly one of these modes. Flags are mutually exclusive When a panel is in viz mode, splitscreen shows a `vizSettingsBtn` ("Viz ⚙") that opens `vizPopover` — a small popover with controls that override that viz plugin's settings **for this panel only**. The controls are generated from a descriptor, so adding a new per-panel option doesn't require touching the popover code. -- **Descriptor lookup** — `getPanelControlsFor(pluginId)` first reads `settings` from the matching provider in `window.feedBack.vizDomain.snapshot().providers`. Any visualization can declare these in `capabilities.visualization.settings` and implement `applySetting(key, value)` on each renderer instance. Descriptors use `{ key, label, type:'toggle'|'range'|'select', default, min?, max?, step?, options? }`, with `options` as `[{id,label}]`. A descriptor may also set `dependsOn: ''`; the popover then greys out and disables that control (with a "Requires …" tooltip) until the named toggle in the same list is on. This is honoured for the legacy factory `panelControls` path; core's manifest validation may strip unknown keys from declared `capabilities.visualization.settings`, so don't rely on it there. A missing or non-toggle target is ignored and the control stays usable. The old factory `panelControls` and built-in highway_3d descriptor remain as compatibility fallbacks. An empty declared array hides the button. +- **Descriptor lookup** — `getPanelControlsFor(pluginId)` first reads `settings` from the matching provider in `window.feedBack.vizDomain.snapshot().providers`. Any visualization can declare these in `capabilities.visualization.settings` and implement `applySetting(key, value)` on each renderer instance. Descriptors use `{ key, label, type:'toggle'|'range'|'select', default, min?, max?, step?, options? }`, with `options` as `[{id,label}]`. A descriptor may also set `dependsOn: ''`; the popover then greys out and disables that control (with a "Requires …" tooltip) until the named toggle in the same list is on. This is honoured only on the legacy factory `panelControls` path. Core whitelists declared `capabilities.visualization.settings` descriptors (`key`, `type`, `label`, `default`, `min`/`max`/`step`, `options`) in both `static/capabilities.js` and `plugins/__init__.py`, so `dependsOn` is dropped there silently, with no warning. `getPanelControlsFor` prefers declared settings and only falls back to `panelControls` for `highway_3d`, so the day a plugin starts declaring its settings its `dependsOn` stops being read and the greying disappears without any signal. Core must carry the field before a declaring plugin can rely on it. Self-references and dependency cycles are treated as ungated, and a control behind a greyed-out gate is greyed out too. A missing or non-toggle target is ignored and the control stays usable. The old factory `panelControls` and built-in highway_3d descriptor remain as compatibility fallbacks. An empty declared array hides the button. - **Provider-refresh reconciliation** — core loads plugin scripts *before* `loadLibraryProviders()` publishes providers, so a panel can initialize against an empty snapshot: descriptor lookup finds nothing, so the value is never restored and the button never appears. `_reconcileVizProviders` runs the init-time sequence (restore saved values, then reveal + build the popover) for every live viz panel whose plugin has since declared controls, driven by `window.feedBack.capabilities.subscribe('visualization:providers-refreshed', …)` (the unsub is dropped — the subscription lives as long as the IIFE). Core normalises an empty declared array to absent, so a provider can't use `settings: []` to suppress the legacy fallback. - **Storage** — capability settings are scoped to renderer instances. Splitscreen persists them under `splitscreenVizSetting:::` and reapplies them when it creates a replacement renderer. The renderer's optional `getSetting(key)` supplies the value the popover *shows* when no override is saved, but `_restoreVizSettings` only re-applies values this panel actually saved (`_vizSavedSetting`). Applying `getSetting()`'s value back would pin an override equal to the plugin's current global, and the panel would stop following later global Settings changes. Legacy highway_3d controls continue using `h3d_bg_panel_` and its global setter event to preserve existing preferences. - **Panel identity (`vizId`)** — the middle segment is a random id minted lazily on the panel's first declared-control read/write (`_vizId`), **not** the panel's array position: pop-out compacts the survivors and a popup builds its own slot 0, while localStorage is shared between the windows, so an index-keyed value follows the slot and can be read or overwritten by an unrelated panel. A per-window counter doesn't help either (main's first panel and a popup's first panel would both draw from their own `1`). The id is carried through panel prefs (`panelToPrefs` → `initPanel`) and follower config (`_captureFollowerConfig` → pop-out URL / `docked` payload / relay `config`), so it survives layout rebuilds, pop-out/dock and a follower's own re-split; follower sub-panels created beyond the incoming cfgs get their own. Numeric-slot keys from the pre-release scheme are deliberately **not** migrated — that mapping is exactly the ambiguity being removed. diff --git a/screen.js b/screen.js index 14a856f..ed6736f 100644 --- a/screen.js +++ b/screen.js @@ -2152,6 +2152,8 @@ try { * greyed out is greyed out too, so there is never an editable control * behind an unreachable gate. Gates are read live, so this follows both * the saved value at build time and later clicks. + * @param {*} controls the descriptor list the popover was built from + * @param {*} built map of control key -> { row, input } for the built rows */ function _wireVizControlDeps(controls, built) { const byKey = Object.create(null);