mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-26 14:52:13 +00:00
199550e5fb
* fix(v3): dismiss Section Practice popover when another player popover opens The Section Practice popover (Songs > Song > Practice pill) stayed open when the user then clicked a v3 player-rail icon (Plugins, Audio, …), leaving two popovers stacked on top of each other. Reported on 0.3.0 (macOS) and still reproducing in the 2026-06-28 build. Root cause: the popover's outside-click dismiss was bound in the bubbling phase, but the v3 rail's icon buttons call e.stopPropagation() in their click handler (player-chrome.js wireRail), which kills bubbling before the click reaches document. So the dismiss listener never fired for a rail-icon click and the popover was orphaned open. Fix: bind the outside-click dismiss in the capture phase, which runs before the target's handler so stopPropagation() can't swallow it. This mirrors the audio mixer popover (audio-mixer.js), which already dismisses outside-clicks via capture-phase listeners for exactly this reason. Esc handling stays in the bubble phase (no rail handler stops keydown propagation, and capturing it would reorder it ahead of the player's Escape-to-exit handling). Shared app.js code, so v2 is covered too; v2 has no stopPropagation rail, so its outside-click dismiss behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * chore(#638): add CHANGELOG entry + capture-phase regression test Review follow-ups for the Section Practice popover dismiss fix: - CHANGELOG [Unreleased] → Fixed entry (repo workflow requires one). - tests/js/section_practice_dismiss.test.js pins the fix: the outside-click dismiss binds in the CAPTURE phase (so a rail icon's stopPropagation can't swallow it), exactly one capture binding (Escape keydown stays bubble-phase), and the #section-practice-control containment guard (no self-close). A revert to bubble-phase fails the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
44 lines
2.2 KiB
JavaScript
44 lines
2.2 KiB
JavaScript
// Guards the Section Practice popover's outside-click dismiss in static/app.js
|
|
// (_installSectionPracticeDismiss). The v3 player-rail icon buttons call
|
|
// e.stopPropagation() in their click handler (static/v3/player-chrome.js
|
|
// wireRail), so a BUBBLE-phase document dismiss never fires when the user clicks
|
|
// a different rail icon (Plugins, Audio, …) — leaving the Practice popover
|
|
// stranded open under the newly-opened one (feedBack#638). The dismiss must bind
|
|
// in the CAPTURE phase (runs before the target's stopPropagation can swallow it).
|
|
// Esc must stay bubble-phase so it doesn't reorder ahead of the player's
|
|
// Escape-to-exit handling. A revert to bubble-phase should fail here.
|
|
//
|
|
// Source-level only — same strategy as the other tests/js/ files.
|
|
|
|
const { test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
|
|
const src = fs.readFileSync(path.join(__dirname, '..', '..', 'static', 'app.js'), 'utf8');
|
|
const m = src.match(/function _installSectionPracticeDismiss\s*\(\)\s*\{[\s\S]*?\n\}/);
|
|
assert.ok(m, '_installSectionPracticeDismiss() not found in static/app.js');
|
|
const body = m[0];
|
|
|
|
test('the outside-click dismiss binds in the CAPTURE phase', () => {
|
|
assert.match(
|
|
body,
|
|
/addEventListener\(\s*['"]click['"][\s\S]*?,\s*true\s*\)/,
|
|
'the click dismiss must pass the capture flag (`, true`) so a rail icon\'s '
|
|
+ 'stopPropagation() cannot swallow it',
|
|
);
|
|
});
|
|
|
|
test('only the click listener is capture (Escape keydown stays bubble-phase)', () => {
|
|
// Exactly one capture binding in the installer — the click. The keydown
|
|
// (Escape) listener must NOT be capture.
|
|
const captureBinds = body.match(/,\s*true\s*\)/g) || [];
|
|
assert.equal(captureBinds.length, 1, 'expected exactly one capture-phase binding (the click)');
|
|
});
|
|
|
|
test('the dismiss ignores clicks inside the control (no self-close)', () => {
|
|
assert.match(body, /section-practice-control/, 'must scope to #section-practice-control');
|
|
assert.match(body, /ctrl\s*&&\s*ctrl\.contains\(e\.target\)\)\s*return/,
|
|
'a click inside the control (incl. the pill) must not dismiss the popover');
|
|
});
|