From 6bfd92aa0678f5e96b994d247ae9cf0f7cc3d33c Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Wed, 1 Jul 2026 03:24:39 -0500 Subject: [PATCH] Fix tuner auto-open flash: opt-in + persist (issue E, stage 1/3) (#655) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tuner self-closes on song:play; autoplay fires it right after a song switch, so an auto-opened tuner flashed shut ~1s later. An arrangement switch (which never arms autoplay) instead persisted — the opposite tester reports, and not the mic. - New opt-in setting autoOpenOnTuningChange (tuner Settings, default OFF) - An auto-opened tuner persists: it ignores the autoplay song:play, stray outside-clicks, and same-screen re-emits, closing only via the new in-panel x / Skip buttons or leaving the song. A manual open keeps the classic click-away / play-to-close behaviour. - Adds the panel's first in-box close (x + contextual Skip). - All in the tuner plugin; no core app.js changes. Default (opt-in vs opt-out) is teed up for Byron to flip one boolean. Staged follow-ups: E1.5 = instrument-coverage smart prompting + badge cue; E2 = holdAutoplay gate. Tests: tests/js/tuner_auto_open.test.js (opt-in gate, persist mode, play/click-proofing). Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF Co-authored-by: Claude Opus 4.8 --- CHANGELOG.md | 1 + plugins/tuner/routes.py | 2 ++ plugins/tuner/screen.js | 42 +++++++++++++++++++++++++------ plugins/tuner/settings.html | 19 ++++++++++++++ plugins/tuner/utils/ui.js | 26 ++++++++++++++++++- tests/js/tuner_auto_open.test.js | 43 +++++++++++++++++++++++++++++--- 6 files changed, 120 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 098219c..01ce67d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **v3 library: exact artist/album filters + scroll/page-depth restore** (feedBack#857). The v3 Songs toolbar gains Artist and Album dropdowns (Album populates from the selected artist and stays disabled until one is chosen), backed by new exact, case-insensitive (`COLLATE NOCASE`) `artist` / `album` query params threaded through `MetadataDB._build_where` → `query_page` / `query_artists` / `query_stats` and the `/api/library`, `/api/library/artists`, `/api/library/stats` endpoints (the free-text `q` search stays fuzzy and composes with the exact filters). The artist/album catalog is fetched independently of the active artist/album selection so the dropdowns always list the full set for the current provider/search. The toolbar is now sticky so filter controls stay reachable when browsing deep libraries, and returning from the player restores the previous scroll position **and** the loaded infinite-scroll page depth via a `sessionStorage` snapshot keyed by a filter/sort/view state hash (invalidated whenever those change, so a filter change still resets to the top). Tests: `tests/test_library_filters.py` (backend artist/album filters), `tests/js/v3_songs_scroll.test.js` (state-hash + snapshot helpers). ### Fixed +- **Tuner auto-open is now opt-in and persists instead of flashing open-then-shut.** When you entered a song (or switched arrangement) whose tuning differed from the last, the tuner auto-opened and — for some testers — vanished ~1s later (reported macOS+Windows, 0.3.0). Root cause: the tuner closes itself on `song:play` (`plugins/tuner/utils/ui.js` — you don't tune while playing), so a **song switch** fired autoplay → `song:play` → the just-auto-opened tuner closed; an **arrangement switch** (which never arms autoplay) had no `song:play`, so it stayed open — exactly why two testers saw opposite behaviour (it wasn't the mic). Now: (1) the feature is a **new opt-in setting** ("Auto-open on tuning change", in the tuner's Settings panel, persisted as `autoOpenOnTuningChange`, **default OFF**); (2) an **auto**-opened tuner *persists* — it ignores the autoplay `song:play`, stray outside-clicks, and same-screen re-emits, closing only via the new in-panel **`×`** / **"Skip"** buttons or when you leave the song. A *manually* opened tuner keeps its classic click-away / play-to-close behaviour. The panel previously had no in-box close at all; this adds one (`×` + contextual Skip). All in the tuner plugin (`routes.py` config, `screen.js` gate + persist, `utils/ui.js` buttons + `song:play` guard, `settings.html` toggle) — **no core `app.js` changes**. Tests: `tests/js/tuner_auto_open.test.js` (opt-in gate, `{ auto: true }` persist mode, play/click-proofing). **Default (opt-in vs opt-out) is teed up for Byron to decide — flip one boolean.** - **v3 player: opening another rail popover now closes the Section Practice popover (no more two stacked popovers).** Opening the **Practice** pill's popover and then clicking a different player-rail icon (e.g. **Plugins**) left the Practice popover open underneath the new one — looked broken (reported on macOS, 0.3.0 / 2026-06-28). The rail icons call `e.stopPropagation()` in their click handler (`static/v3/player-chrome.js`), which killed bubbling before it reached the Practice popover's outside-click dismiss bound on `document`. The dismiss (`_installSectionPracticeDismiss` in `static/app.js`) now binds in the **capture phase**, which runs before the target's handler so a descendant's `stopPropagation()` can't swallow it — mirroring how the audio-mixer popover already dismisses. Esc handling stays bubble-phase (the player's Escape-to-exit ordering is unchanged). v2 shares `app.js` and is only hardened (no rail `stopPropagation` there). Tests: `tests/js/section_practice_dismiss.test.js`. - **v3 UI no longer lets you accidentally text-select the chrome.** Dragging or double-clicking across the interface used to marquee-highlight buttons, labels, the sidebar, the transport, and the note-highway HUD — which looks broken (reported on Mac + Windows). The v3 shell now defaults to `user-select: none` on `html` (`static/v3/v3.css`), then opts *content* back in — so chrome is non-selectable but the text you actually copy still works. Decided by a 4-lens panel (UX / accessibility / dev-ops / plugin-ecosystem); the guardrails are deliberate: **form fields are always re-enabled** (never break the caret / IME — no `* { user-select:none }`, which trips a WebKit input bug); **plugin screens (`.screen[id^="plugin-"]`) stay selectable by default** so a plugin's copyable text (lyrics, chord names, results) — including community plugins that don't know about this — isn't silently locked; and **core read-only content opts back in by container** via a new hand-authored **`.fb-selectable`** class — applied to the whole **Settings** panel (paths, device names, version, diagnostics, About — answering "is settings still copyable?": yes), the **now-playing song metadata** (with `pointer-events` re-enabled so the HUD text is actually reachable), and the focused **modals / dialogs / toasts / scan banner** that carry copyable errors, IDs, paths, and file names. It's cosmetic only (it protects nothing) and never used to lock copy-worthy text — errors, IDs, paths, versions, and metadata stay selectable per WCAG 2.2 (copy-paste as a permitted mechanism). Dense card lists (library grid, dashboard, profile) stay non-selectable by design — making them selectable would reintroduce the marquee-mess across cards. **v3-only** (v2 unchanged); plain CSS, no Tailwind rebuild; no desktop changes (standard OS-framed window). Plugin authors: `.fb-selectable` is documented in `CLAUDE.md` for re-enabling copyable content rendered outside a plugin screen. Tests: `tests/js/v3_user_select_policy.test.js`. - **Input-setup wizard no longer collapses an audio device's driver-type variants into one entry.** On Windows the desktop engine enumerates the same interface once per host API (ASIO / Windows Audio / DirectSound), and the wizard's audio picker (`plugins/input_setup/screen.js`) de-duped the source list by display **label** — so the variants (which share a name) collapsed to a single choice, silently keeping whichever sorted first (often *not* the low-latency ASIO one the player wants). The audio-input capability already collapses true duplicates by `logicalSourceKey` (`_visibleInputSources` in `static/capabilities/audio-session.js`), and the variants each have a **distinct** key, so the wizard's extra label-collapse was redundant for real dupes and destructive for these — it also could drop the variant that was actually `selected`. Removed it; the picker now lists every selectable input. Pairs with feedBack-desktop's change to label each source with its driver type (e.g. "Focusrite (ASIO)") so the now-distinct entries are legible. diff --git a/plugins/tuner/routes.py b/plugins/tuner/routes.py index 347bb5d..a58da10 100644 --- a/plugins/tuner/routes.py +++ b/plugins/tuner/routes.py @@ -37,6 +37,7 @@ def setup(app: FastAPI, context: dict): "showFloatingButton": True, "visualizationMode": "default", "audioInputMode": "auto", + "autoOpenOnTuningChange": False, } if not config_file.exists(): return defaults @@ -55,6 +56,7 @@ def setup(app: FastAPI, context: dict): res["visualizationMode"] = str(data.get("visualizationMode", "default")) raw_mode = str(data.get("audioInputMode", "auto")) res["audioInputMode"] = raw_mode if raw_mode in ("auto", "browser") else "auto" + res["autoOpenOnTuningChange"] = bool(data.get("autoOpenOnTuningChange", False)) if not isinstance(res["customTunings"], dict): res["customTunings"] = {} diff --git a/plugins/tuner/screen.js b/plugins/tuner/screen.js index 9a38a54..072b5af 100644 --- a/plugins/tuner/screen.js +++ b/plugins/tuner/screen.js @@ -142,6 +142,12 @@ async function _maybeAutoOpenOnTuningChange() { if (!document.getElementById('player')?.classList.contains('active')) return; + // Opt-in (default off): only auto-open when the user enabled it in the + // tuner settings. Ensure config is loaded so the first song:ready after + // boot still reads the real flag; fail closed if it can't load. + if (!_state._serverConfig) { try { await loadConfig(); } catch (_) { /* */ } } + if (!_state._serverConfig || !_state._serverConfig.autoOpenOnTuningChange) return; + const songInfo = window.highway?.getSongInfo?.() || window.feedBack?.currentSong; if (!songInfo) return; @@ -167,7 +173,7 @@ _lastAutoOpenSessionKey = sessionKey; try { - await window.tuner.enable(); + await window.tuner.enable({ auto: true }); if (myGen !== _autoOpenGeneration) return; } catch (e) { console.warn('Tuner: auto-open failed:', e && e.message ? e.message : e); @@ -341,8 +347,15 @@ } } - async function enable() { + async function enable(opts) { if (_state.enabled) return; + // An AUTO-open (the "this song needs a different tuning" nudge) must + // PERSIST: it is NOT dismissed by the autoplay song:play that follows + // song entry, a stray click, or a same-screen re-emit — only by the + // Skip/× buttons or leaving the song. A manual open keeps the classic + // click-away / play-to-close behaviour. + const auto = !!(opts && opts.auto); + _state.autoOpened = auto; await _loadScript('/api/plugins/tuner/utils/tuning-utils.js'); await _loadScript('/api/plugins/tuner/utils/audio.js'); await _loadScript('/api/plugins/tuner/utils/ui.js'); @@ -371,15 +384,27 @@ _state.uiContainer.classList.add('flex'); _tunerUIApi.positionPanel(); _tunerUIApi.updateFreeTuneUI(); + // "Skip" is the auto-open nudge's explicit dismiss; hidden for a manual + // open (the × / click-away already close those). + if (_state.skipBtn) _state.skipBtn.classList.toggle('hidden', !auto); - // Close when clicking outside the panel. Deferred so the badge's - // opening click doesn't bubble up to the document and fire immediately. - if (_outsideClickClose) document.removeEventListener('click', _outsideClickClose); - _outsideClickClose = () => { if (_state.enabled) disable(); }; - setTimeout(() => { if (_outsideClickClose) document.addEventListener('click', _outsideClickClose, { once: true }); }, 0); + // Close when clicking outside the panel. Deferred so the badge's opening + // click doesn't bubble up to the document and fire immediately. Skipped + // for an auto-open: the user never clicked to open it, so their first + // unrelated click must not dismiss it (it persists until Skip/×/leave). + if (!auto) { + if (_outsideClickClose) document.removeEventListener('click', _outsideClickClose); + _outsideClickClose = () => { if (_state.enabled) disable(); }; + setTimeout(() => { if (_outsideClickClose) document.addEventListener('click', _outsideClickClose, { once: true }); }, 0); + } if (window.feedBack && !_onScreenChanged) { - _onScreenChanged = () => { disable(); }; + // Auto-opened: close only when we actually LEAVE the song — a player + // re-emit while staying put must not tear down the nudge. Manual: + // unchanged (any screen change closes it). + _onScreenChanged = () => { + if (!_state.autoOpened || !document.getElementById('player')?.classList.contains('active')) disable(); + }; _onSongReady = () => { _tunerUIApi.renderTuningOptions(); if (_state.selectedTuningName === '_current') _syncCurrentTuning(); @@ -409,6 +434,7 @@ const wasEnabled = _state.enabled; const onPlayer = document.getElementById('player')?.classList.contains('active'); _state.enabled = false; + _state.autoOpened = false; _state.manualTargetFreq = null; if (_outsideClickClose) { document.removeEventListener('click', _outsideClickClose); _outsideClickClose = null; } if (_state.activeViz) { _state.activeViz.destroy(); _state.activeViz = null; } diff --git a/plugins/tuner/settings.html b/plugins/tuner/settings.html index 6c1755e..560a7bd 100644 --- a/plugins/tuner/settings.html +++ b/plugins/tuner/settings.html @@ -10,6 +10,17 @@ +
+
+

Auto-open on tuning change

+

When a song (or arrangement) needs a different tuning, pop the tuner open automatically. It stays open until you Skip or close it.

+
+ +
+