From c7ffdfa06b023ef23a57770d52696915637d1136 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Mon, 29 Jun 2026 19:15:31 -0500 Subject: [PATCH 1/2] =?UTF-8?q?Fix=20v3=20Songs=20A=E2=80=93Z=20rail:=20re?= =?UTF-8?q?liable=20taps,=20precise=20drag,=20hittable=20size?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #634. Three rail bugs reported on macOS + Windows (0.3.0, 2026-06-29 — =Scr4tch=, MajorMokoto): - Taps often did nothing ("clicked O, nothing happened"). pointerdown calls setPointerCapture, after which the browser retargets the follow-up click to the rail container, so the click handler's closest('.v3-azrail-letter') resolved null and a plain tap (no pointermove) had no other path. Drive the jump from pointerdown itself; reduce the click handler to keyboard activation only (e.detail === 0, Enter/Space). - A drag landed short of the release ("where you release isn't where you get sent"). Every letter crossed fired jumpToLetter with behavior:'smooth'; stacked smooth-scroll animations over the virtualized grid lagged and settled imprecisely. jumpToLetter now takes a smooth flag and scrolls instantly ('auto') while scrubbing, animating only discrete taps/keyboard jumps, so the grid tracks the finger and the release lands on the let-go letter. - The rail was too small at 1440p and didn't scale. Letters were a fixed .62rem glued at right:2px (~13px-tall target). They now scale with the viewport (clamp(.72rem, 1.4vh, 1.05rem)), sit off the edge with taller/wider equal-width hit targets and a hover/active highlight so the scrub target is visible. Keyboard arrow-nav and present-letter gating are unchanged. Tests: tests/js/v3_az_rail.test.js gains pointerdown-seek, keyboard-only click guard, and instant-vs-smooth assertions (809 JS tests; the 13 pre-existing unrelated failures are unchanged). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF --- CHANGELOG.md | 1 + static/v3/songs.js | 69 +++++++++++++++++++++++++------------ static/v3/v3.css | 22 +++++++----- tests/js/v3_az_rail.test.js | 22 ++++++++++++ 4 files changed, 84 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f10a4a1..d92f2964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The tuner badge now passively flags when a song needs a different tuning — and names the retune.** Building on the coverage check: when you enter a song your current instrument doesn't cover, the topbar tuner badge gets an amber ring and a tooltip that **names the change** — e.g. *"retune B→A"* for a Drop-A song on an F♯ 8-string, or *"the reference pitch"* for an A440-vs-A432 mismatch. It's purely **advisory** (it never auto-opens the panel — tap the badge to tune), recomputed on `song:ready` and cleared when a new song loads or you leave the player. The retune diff comes from the tuner plugin's coverage report (`window._tunerAutoOpen.coverageReport` → `{ covered, retune: [{ from, to }], reference, cantCover }`); the cue is CSS-free (an inline ring + native tooltip — no Tailwind rebuild) and no-ops when the tuner plugin isn't installed. **v3-only.** Touches `static/v3/badges.js` (the cue) + `plugins/tuner/screen.js` (the report). Tests: `tests/js/tuner_auto_open.test.js` (the report names the strings; reference mismatch; the badge wiring). _(The splitscreen-suppress and no-usable-input guards move to the playback-gate stage, where they matter for its no-trap rule.)_ - **Tuner auto-open can now gate playback until you've tuned — the "tune before you play" model — via a new core `holdAutoplay()` hook.** With the opt-in auto-open on, when a song needs a retune the tuner opens and **playback waits** for your choice — **Skip** (you've tuned → play, and record the song's tuning as your instrument's current working tuning), **Back to library** / **Esc** (leave the song; a gated retune is never a one-way trap), or press **Play** (always wins). For an auto-open the in-panel **×** is dropped — Skip / Back to library / Esc are its dismiss surface. Previously the song played with the tuner overlaid; now it holds — which also definitively kills the original flash, since autoplay's `song:play` can't fire while playback is held. Implemented as a small **core hook** `window.feedBack.holdAutoplay()` (mirrors the existing `holdAutoExit()`): a plugin claims it **synchronously on `song:loading`** (so it beats the `song:ready` autostart), and `release()` — or a **12-second fail-open backstop** — runs the deferred start. **Generation-guarded** (a new song invalidates a stale hold) and **fail-open** (a wedged or crashed plugin can never permanently strand a song); **manual Play always wins** (it doesn't flow through the autostart path). The tuner claims the gate only when the feature is on, and **releases it the instant** it decides not to open (song already covered / tuning unchanged) or when you Skip. Touches core `static/app.js` (the hook + an autostart refactor) and the tuner plugin (`plugins/tuner/screen.js` — the claim/release; `plugins/tuner/utils/ui.js` — the Skip / Back-to-library buttons, × dropped on auto-open); the hook is generic and shell-agnostic (a test asserts `app.js` still doesn't reference the tuner's internals). Tests: `tests/js/tuner_auto_open.test.js` (claim on `song:loading`, release on dismiss, feature-off no-claim, the core hook + fail-open backstop, the Skip / Back-to-library / Esc escape-hatch) + a `speed_reset.test.js` stub. ⚠️ **Needs a manual smoke-test before shipping** — this is a core playback change; verify on desktop that the tuner mic doesn't contend with note_detect's scoring input (ASIO/exclusive mode), per the design charrette. - **v3 Songs List View: favoriting a song now turns the heart red immediately (no re-search needed).** In the tree / "List View" (Songs → List → expand an artist), clicking the heart flipped the glyph ♡→♥ but it stayed dim grey until you re-searched the library — reported on macOS + Windows, open since 0.3.0 / 2026-06-25. One shared `wireCards()` `[data-fav]` handler (`static/v3/songs.js`) serves both the grid card and the List-View row, but the two render with different idle colours — grid `text-white`, List View `text-fb-textDim` — and the handler only ever removed the grid's `text-white`. So in List View the row kept `text-fb-textDim` alongside the freshly-added `text-fb-accent`, and the dim class won by CSS source order (glyph changed, colour didn't). Each heart now declares its idle colour via a `data-fav-idle` attribute and the handler swaps exactly that class, so only one colour class is ever present; the handler also writes the new state back onto the in-memory song model so a re-render / virtualized-grid recycle agrees instead of reverting. Tests: `tests/js/v3_favorites_toggle.test.js`. +- **v3 Songs A–Z rail: taps now land reliably, a drag releases exactly on the let-go letter, and the rail is large enough to hit on hi-res displays.** Follow-up to the rail's debut (#634); three bugs reported on macOS + Windows (0.3.0, 2026-06-29): a tap often did nothing ("clicked O, nothing happened"), a drag "got you kind of there but where you release isn't where you get sent," and the rail was "way too small" at 1440p and didn't scale with resolution. Root causes & fixes, all in `static/v3/songs.js` + `static/v3/v3.css` (`bindRailOnce`/`jumpToLetter`/`.v3-azrail`): (1) **taps** — `pointerdown` calls `setPointerCapture`, after which the browser **retargets the follow-up `click` to the rail container**, so the click handler's `closest('.v3-azrail-letter')` resolved `null` and a plain tap (no `pointermove`) had no other path → no-op. The jump is now driven from `pointerdown` itself (seek on press); the `click` handler is reduced to **keyboard activation only** (`e.detail === 0`, Enter/Space). (2) **drag precision** — every letter crossed fired `jumpToLetter` with `behavior:'smooth'`; stacked smooth-scroll animations over the virtualized grid lagged and settled short of the release. `jumpToLetter(letter, smooth)` now scrolls **instantly while scrubbing** (`'auto'`) and only animates discrete taps/keyboard jumps, so the grid tracks the finger and the release lands on the let-go letter. (3) **size** — the letters were a fixed `.62rem` glued at `right:2px` (~13px-tall target on the screen edge); they now scale with the viewport (`clamp(.72rem, 1.4vh, 1.05rem)`), sit off the edge with taller/wider equal-width hit targets and a hover/active highlight so the scrub target is visible. Keyboard arrow-nav + the present-letter gating are unchanged. Reported by =Scr4tch= and MajorMokoto. Tests: `tests/js/v3_az_rail.test.js` (pointerdown-seek, keyboard-only click guard, instant-vs-smooth scroll). - **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/static/v3/songs.js b/static/v3/songs.js index e80f929d..5288ae75 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -1867,7 +1867,11 @@ } let _jumpToken = 0; - async function jumpToLetter(letter) { + // `smooth` animates the scroll (a discrete tap / keyboard jump). A drag-scrub + // passes `false` so each step snaps INSTANTLY: stacked smooth animations over + // the windowed grid lag and settle imprecisely, which is why a drag used to + // land somewhere other than the let-go letter. + async function jumpToLetter(letter, smooth = true) { const grid = _gridEl(), sizer = _sizerEl(), main = document.getElementById('v3-main'); if (!grid || !sizer || !main || state.view !== 'grid' || !letter) return; _setRailActive(letter); @@ -1890,7 +1894,7 @@ const toolbar = document.getElementById('v3-songs-toolbar'); const pad = (toolbar ? toolbar.offsetHeight : 0) + 12; // clear the sticky toolbar const top = Math.max(0, sizerTop + targetRow * rowH - pad); - main.scrollTo({ top, behavior: 'smooth' }); + main.scrollTo({ top, behavior: smooth ? 'smooth' : 'auto' }); requestWindowRender(); } @@ -1898,38 +1902,59 @@ const rail = railEl(); if (!rail || rail._bound) return; rail._bound = true; - let dragging = false, moved = false, lastDrag = null; + let dragging = false, lastDrag = null, railBtns = []; + // Resolve the letter button under a viewport-Y from the per-drag cached + // list (avoids a querySelectorAll per pointermove), clamping past the + // top/bottom so a scrub off the ends still seeks the first/last letter. const letterAtY = (y) => { - const els = rail.querySelectorAll('.v3-azrail-letter'); - if (!els.length) return null; - for (const el of els) { const r = el.getBoundingClientRect(); if (y >= r.top && y <= r.bottom) return el; } - return y < els[0].getBoundingClientRect().top ? els[0] : els[els.length - 1]; // clamp past ends + if (!railBtns.length) return null; + for (const el of railBtns) { const r = el.getBoundingClientRect(); if (y >= r.top && y <= r.bottom) return el; } + return y < railBtns[0].getBoundingClientRect().top ? railBtns[0] : railBtns[railBtns.length - 1]; + }; + // Seek to the letter under `y`. `smooth` on the initial press (a tap); + // instant during the scrub so the grid tracks the finger and RELEASE + // lands exactly on the let-go letter. + const seekToY = (y, smooth) => { + const el = letterAtY(y); + if (!el || el.disabled) return; + const L = el.getAttribute('data-letter'); + _showBubble(L); + if (L !== lastDrag) { lastDrag = L; jumpToLetter(L, smooth); } }; - rail.addEventListener('click', (e) => { - const btn = e.target.closest('.v3-azrail-letter'); - if (!btn || btn.disabled) return; - if (moved) { moved = false; return; } // a drag already handled it - jumpToLetter(btn.getAttribute('data-letter')); - }); rail.addEventListener('pointerdown', (e) => { const btn = e.target.closest('.v3-azrail-letter'); if (!btn) return; - dragging = true; moved = false; lastDrag = null; + railBtns = [...rail.querySelectorAll('.v3-azrail-letter')]; + dragging = true; lastDrag = null; + // Capture so a vertical scrub keeps seeking even if the pointer drifts + // off the thin rail horizontally. try { rail.setPointerCapture(e.pointerId); } catch (_) { /* */ } - _showBubble(btn.getAttribute('data-letter')); + // Drive the jump from here, NOT from the click event: pointer capture + // retargets the follow-up click to the rail (never a letter), so a + // captured tap's click can't resolve a letter and used to no-op + // ("clicked O, nothing happened"). preventDefault() suppresses the + // text-selection / focus-scroll default; we re-focus below for kbd. + e.preventDefault(); + try { btn.focus({ preventScroll: true }); } catch (_) { /* */ } + seekToY(e.clientY, true); // jump on press → a tap lands immediately }); rail.addEventListener('pointermove', (e) => { if (!dragging) return; - const el = letterAtY(e.clientY); - if (!el || el.disabled) return; - moved = true; - const L = el.getAttribute('data-letter'); - _showBubble(L); - if (L !== lastDrag) { lastDrag = L; jumpToLetter(L); } // only on change + seekToY(e.clientY, false); // instant tracking during the scrub }); - const end = () => { dragging = false; _hideBubble(); }; + const end = () => { dragging = false; railBtns = []; _hideBubble(); }; rail.addEventListener('pointerup', end); rail.addEventListener('pointercancel', end); + // Keyboard activation only. A pointer-driven click (detail >= 1) is + // retargeted to the rail by pointer capture and can't resolve a letter, + // so the pointer path owns taps; act here only on Enter/Space, whose + // synthesized click has detail === 0. + rail.addEventListener('click', (e) => { + if (e.detail !== 0) return; + const btn = e.target.closest('.v3-azrail-letter'); + if (!btn || btn.disabled) return; + jumpToLetter(btn.getAttribute('data-letter')); + }); rail.addEventListener('keydown', (e) => { if (e.key !== 'ArrowUp' && e.key !== 'ArrowDown') return; const btns = [...rail.querySelectorAll('.v3-azrail-letter:not([disabled])')]; diff --git a/static/v3/v3.css b/static/v3/v3.css index 1e76a5c1..d834cb24 100644 --- a/static/v3/v3.css +++ b/static/v3/v3.css @@ -1205,14 +1205,14 @@ html.fb-immersive #v3-main > .screen.active { centered. Shown only for the grid view + alphabetical (artist/title) sorts. */ .v3-azrail { position: fixed; - right: 2px; + right: 4px; /* off the very edge so letters aren't clipped */ top: 50%; transform: translateY(-50%); z-index: 25; display: flex; flex-direction: column; - align-items: center; - max-height: 84vh; + align-items: stretch; /* equal-width buttons → one wide, even hit column */ + max-height: 92vh; padding: 4px 1px; user-select: none; -webkit-user-select: none; @@ -1224,17 +1224,23 @@ html.fb-immersive #v3-main > .screen.active { background: none; border: 0; color: #94a3b8; /* fb-textDim */ - font-size: .62rem; + /* Scale with viewport height so the 27-letter rail grows on tall / hi-res + displays (a fixed size looked tiny at 1440p) while still fitting 27 rows + within max-height on short screens. */ + font-size: clamp(.72rem, 1.4vh, 1.05rem); font-weight: 700; - line-height: 1.05; - padding: 1px 4px; + line-height: 1.04; + /* Vertical padding fattens the tap target (was ~13px tall → easy to miss). */ + padding: clamp(2px, .55vh, 6px) 9px; margin: 0; cursor: pointer; border-radius: 4px; + text-align: center; } .v3-azrail-letter:hover:not([disabled]), .v3-azrail-letter.is-active { color: #0ea5e9; /* fb-primary */ + background: rgba(14, 165, 233, .15); /* visible target under the scrub */ } .v3-azrail-letter:focus-visible { outline: 2px solid #38bdf8; /* fb-primaryHi */ @@ -1247,7 +1253,7 @@ html.fb-immersive #v3-main > .screen.active { /* Drag indicator bubble (Android fast-scroll pattern). */ .v3-azbubble { position: fixed; - right: 2.6rem; + right: 2.9rem; /* clear the (now wider) rail */ top: 50%; transform: translateY(-50%); z-index: 26; @@ -1269,7 +1275,7 @@ html.fb-immersive #v3-main > .screen.active { /* Coarse-pointer / short viewports: the 27-letter rail can crowd a phone edge. Tighten it; a collapse-to-anchors pass is a follow-up. */ @media (max-height: 640px) { - .v3-azrail-letter { font-size: .55rem; padding: 0 4px; } + .v3-azrail-letter { font-size: clamp(.5rem, 1.3vh, .62rem); padding: 0 7px; } } /* — Practice-aware library home: repertoire meter + "Keep practicing" shelf — */ diff --git a/tests/js/v3_az_rail.test.js b/tests/js/v3_az_rail.test.js index 524c9190..dde612d1 100644 --- a/tests/js/v3_az_rail.test.js +++ b/tests/js/v3_az_rail.test.js @@ -92,3 +92,25 @@ test('the rail supports pointer drag-scrub + keyboard arrows', () => { assert.match(src, /ArrowUp'[\s\S]*?ArrowDown'|ArrowDown'[\s\S]*?ArrowUp'/, 'arrow keys must move between present letters'); }); + +test('pointer taps are driven by pointerdown, not the retarget-prone click', () => { + // A tap must seek on pointerdown (pointer capture retargets the follow-up + // click to the rail, so resolving a letter from click is unreliable — taps + // would no-op, "clicked O, nothing happened"). + assert.match(src, /addEventListener\('pointerdown'[\s\S]*?seekToY\(/, + 'pointerdown must seek immediately so a tap lands without a move'); + // The click handler must ignore pointer-driven clicks (detail >= 1) and only + // handle keyboard Enter/Space activation (synthesized click has detail === 0). + assert.match(src, /addEventListener\('click'[\s\S]*?e\.detail\s*!==\s*0/, + 'the rail click handler must guard on e.detail === 0 (keyboard only)'); +}); + +test('drag scrubs seek instantly while taps/keys seek smoothly', () => { + assert.match(src, /async function\s+jumpToLetter\s*\(\s*letter\s*,\s*smooth/, + 'jumpToLetter must take a smooth flag'); + assert.match(src, /behavior:\s*smooth\s*\?\s*'smooth'\s*:\s*'auto'/, + 'jumpToLetter must scroll instantly during a drag, smoothly on a tap'); + // pointermove scrubs with smooth=false so RELEASE lands on the let-go letter. + assert.match(src, /addEventListener\('pointermove'[\s\S]*?seekToY\([^;]*?,\s*false\)/, + 'pointermove must seek with smooth=false (precise drag tracking)'); +}); From 056f9d57f37f5376fea719cdc7ab846be18f1af5 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Thu, 2 Jul 2026 13:15:01 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(v3):=20ignore=20non-primary=20buttons?= =?UTF-8?q?=20on=20A=E2=80=93Z=20rail=20pointerdown=20(PR=20#653=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right- or middle-clicking the A–Z rail (or a secondary multi-touch pointer) no longer triggers a seek; only the primary tap/drag scrubs. Co-Authored-By: Claude Opus 4.8 (1M context) --- static/v3/songs.js | 1 + 1 file changed, 1 insertion(+) diff --git a/static/v3/songs.js b/static/v3/songs.js index 5288ae75..54fff4eb 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -1922,6 +1922,7 @@ if (L !== lastDrag) { lastDrag = L; jumpToLetter(L, smooth); } }; rail.addEventListener('pointerdown', (e) => { + if (e.button !== 0 || e.isPrimary === false) return; // primary tap only; ignore right/middle-click + secondary touches const btn = e.target.closest('.v3-azrail-letter'); if (!btn) return; railBtns = [...rail.querySelectorAll('.v3-azrail-letter')];