fix(v3): pedal click opens the plugin's screen, not its settings (#556)

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) <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos 2026-06-22 10:07:36 +02:00 committed by GitHub
parent 63eb7a4ffc
commit f79efe2516
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 25 additions and 8 deletions

View File

@ -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 '<div class="v3-pedal' + (off ? ' v3-pedal-off' : '') + '" data-id="' + esc(p.id) + '" tabindex="0" role="button" ' +
'aria-label="' + esc(p.name || p.id) + ' — open settings" title="' + esc(tip) + '"' + style + '>' +
'aria-label="' + esc((p.name || p.id) + action) + '" title="' + esc(tip) + '"' + style + '>' +
'<span class="v3-pedal-glow" aria-hidden="true"></span>' +
(p.bundled ? '<span class="v3-pedal-bundled" title="Ships with Slopsmith core">core</span>' : '') +
'<span class="v3-pedal-offbadge" aria-hidden="true">off</span>' +
@ -272,6 +280,11 @@
// failed plugin has manifest has_screen but no #plugin-<id> 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.');
}

View File

@ -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' });
});