From f79efe25165e81fc8d00c4c3f55190ad147742e5 Mon Sep 17 00:00:00 2001 From: Byron Gamatos Date: Mon, 22 Jun 2026 10:07:36 +0200 Subject: [PATCH] fix(v3): pedal click opens the plugin's screen, not its settings (#556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v3 Pedalboard's settingsTarget() resolved settings-first, so a plugin that ships both a screen and a settings panel (notably the bundled Audio Engine) could only ever reach its settings from the pedalboard — its actual page was unreachable. Flip to screen-first (stompbox metaphor: step on the pedal, see the pedal), falling back to settings when there is no screen. Keep a settings fallback in openPluginSettings() when a declared screen isn't mounted yet (installing/failed) so settings-bearing plugins are never stranded on a toast. Drive the pedal aria-label off the same target so it never promises the wrong surface. Update the unit test contract to screen > settings > none. Fixes #555 Co-authored-by: Claude Opus 4.8 (1M context) --- static/v3/plugins-page.js | 25 +++++++++++++++++++------ tests/js/plugins_page.test.js | 8 ++++++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/static/v3/plugins-page.js b/static/v3/plugins-page.js index 93ec719..af15319 100644 --- a/static/v3/plugins-page.js +++ b/static/v3/plugins-page.js @@ -5,8 +5,8 @@ * renders as a stompbox pedal (thumbnail + name + short description). Pedals are * free-form draggable within their board (positions persist in localStorage), * and decorative patch cables (pedal-cables.js) sag/swing between them. Clicking - * (not dragging) a pedal opens that plugin's SETTINGS page; plugins with no - * settings fall back to their screen. Data comes from the enriched /api/plugins + * (not dragging) a pedal opens that plugin's own SCREEN (its page); plugins with + * no screen fall back to their settings panel. Data comes from the enriched /api/plugins * (now carrying description/category/icon — see plugins/__init__.py::_nav_entry). * The bundled Capability Inspector still owns the live capability graph; we * surface a deep-link to it rather than rebuilding it. @@ -136,11 +136,15 @@ return !!(p.nav || p.has_screen || (p.has_script && document.getElementById('plugin-' + p.id))); } - // Decide what a pedal click should open: its settings panel, else its - // screen, else nothing (toast). Pure — unit-tested. + // Decide what a pedal click should open. A plugin's own screen (its full + // "page") is the primary surface, so clicking the pedal opens it when one + // exists — matching the stompbox metaphor (step on the pedal → see the + // pedal). Plugins with no screen fall back to their settings panel; the + // settings of a screen+settings plugin (e.g. audio_engine) stay reachable + // from the main Settings screen. Pure — unit-tested. function settingsTarget(p) { - if (p && p.has_settings) return { kind: 'settings', id: p.id }; if (openable(p)) return { kind: 'screen', id: p.id }; + if (p && p.has_settings) return { kind: 'settings', id: p.id }; return { kind: 'none', id: p && p.id }; } @@ -198,8 +202,12 @@ var off = p.enabled === false; // Description shows only as a hover tooltip on the pedal, not on the face. var tip = (p.name || p.id) + (desc ? ' — ' + desc : ''); + // The action verb tracks what a click actually opens (screen-first, + // then settings) so the a11y label never promises the wrong surface. + var kind = settingsTarget(p).kind; + var action = kind === 'screen' ? ' — open' : (kind === 'settings' ? ' — open settings' : ''); return '
' + + 'aria-label="' + esc((p.name || p.id) + action) + '" title="' + esc(tip) + '"' + style + '>' + '' + (p.bundled ? 'core' : '') + '' + @@ -272,6 +280,11 @@ // failed plugin has manifest has_screen but no #plugin- div yet. if (window.showScreen && document.getElementById('plugin-' + tgt.id)) { window.showScreen('plugin-' + tgt.id); + } else if (p && p.has_settings) { + // Screen declared but not mounted yet — fall back to the + // settings panel rather than stranding the user on a toast. + if (window.showScreen) window.showScreen('settings'); + openSettingsPanel(p.id); } else { toast('This plugin is still loading — try again in a moment.'); } diff --git a/tests/js/plugins_page.test.js b/tests/js/plugins_page.test.js index 8cd9d3a..1bcb7c7 100644 --- a/tests/js/plugins_page.test.js +++ b/tests/js/plugins_page.test.js @@ -73,11 +73,15 @@ test('thumbUrl: manifest icon routes through the asset endpoint; else default', assert.equal(t.thumbUrl({ id: 'x', icon: '' }), '/static/v3/pedal-default.svg'); }); -test('settingsTarget: settings > screen > none', () => { +test('settingsTarget: screen > settings > none', () => { const { t } = loadPage(); - assert.deepEqual(t.settingsTarget({ id: 'a', has_settings: true, nav: true }), { kind: 'settings', id: 'a' }); + // A screen wins even when the plugin also has settings (e.g. audio_engine): + // the pedal opens the plugin's page, not its settings panel. + assert.deepEqual(t.settingsTarget({ id: 'a', has_settings: true, nav: true }), { kind: 'screen', id: 'a' }); assert.deepEqual(t.settingsTarget({ id: 'b', has_settings: false, nav: true }), { kind: 'screen', id: 'b' }); assert.deepEqual(t.settingsTarget({ id: 'c', has_settings: false, has_screen: true }), { kind: 'screen', id: 'c' }); + // Settings-only plugin (no screen) falls back to its settings panel. + assert.deepEqual(t.settingsTarget({ id: 'e', has_settings: true }), { kind: 'settings', id: 'e' }); assert.deepEqual(t.settingsTarget({ id: 'd' }), { kind: 'none', id: 'd' }); });