Files
feedBack/tests/browser/exit-confirm.spec.ts
fef870047b fix(player): reliable Escape "Back" + resumable, optionally-confirmed song exit (#619)
* fix(player): make Escape a reliable Back; resumable + optionally-confirmed song exit

Escape didn't always leave a song: clicking a transport control (play/FF/RW/
restart) left that <button> focused, and _shortcutDispatchBlocked() bails the
shortcut dispatcher for any focused INPUT/SELECT/TEXTAREA/BUTTON — so the
player-scope Escape=Back shortcut never fired until the user clicked empty
canvas to blur the control. Space already had a player-screen carve-out (#593);
Escape did not. That asymmetry was the bug.

Phase 1 — focus fix: generalize the Space carve-out in _shortcutDispatchBlocked
to Escape, on the player AND settings screens (both register Escape=Back;
settings had the identical latent bug). The earlier guards still win: text
inputs are exempted first, the Section Practice popover already claims Escape,
and a true modal (role=dialog aria-modal=true / .feedBack-modal) still traps it.
Plugins' player-scope Escape shortcuts are fixed identically.

Phase 2 — resume: leaving the player snapshots {song, arrangement, position,
speed} to localStorage; a non-blocking "Resume practice" pill offers it back on
the next non-player screen / next launch. playSong() gains a {resume} option
that restores speed + seeks to the saved position on song:ready instead of the
normal autostart. Conservative (ignores <3s / near-end), cleared on natural
song-end and once consumed, expires after 24h.

Phase 3 — opt-in "Ask before leaving a song" (Gameplay tab, default OFF). A
true-modal confirm with monotonic Escape (the second Escape leaves) and
Space/Enter = Leave. The player Escape shortcut and the v3 close button route
through window.requestExitSong(); auto-exit on song-end and a results screen's
own Close stay unguarded.

Design rationale: a multi-seat design charrette (engagement, learning-design,
operability, codebase-reality) — leaving a song should be reliable and
recoverable, not gated; the confirm is opt-in only.

Tests: tests/browser/{keyboard-shortcuts,resume-session,exit-confirm}.spec.ts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF

* test(browser): suppress first-run onboarding in keyboard/resume/exit specs

The first-run onboarding overlay (#v3-onboarding) is a modal that intercepts
pointer/keyboard events; on a fresh profile it covers the player and breaks any
test that presses Escape or clicks. Stub GET /api/profile to an onboarded
profile in each beforeEach so the app behaves like a returning user (the state
these tests assume).

Also tighten the Section Practice Escape test to assert the guarantee the fix
actually provides — Escape does not exit the song while the popover is open (the
line-447 guard wins over the carve-out) — rather than asserting the popover's
own close handler fires, which isn't wired for a synthetic bar.

Verified locally against a worktree server (Chromium): all 16 new specs pass
(5 Escape + 6 resume + 5 exit-confirm) plus the existing #593 Space tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF

* fix(player): exit-confirm — Escape cancels back to song, pause on open/resume on stay

Refinements from tester feedback on the exit-confirm (default stays OFF):

- Escape on the open prompt now = Stay (dismiss + return to the song), matching
  every other modal and the generic _confirmDialog (Esc=cancel). A second
  Escape therefore returns to the song instead of leaving it. Leaving stays the
  explicit, default-focused "Leave" button, so Space/Enter/click = "just get me
  out" (the OP's "Space always hits leave").
- Opening the prompt PAUSES the song (via the canonical togglePlay path, HTML5
  + _juceMode) so it isn't running/being scored behind the modal; Stay resumes
  exactly what we paused. Guards: cancel any count-in on open; resume only if we
  paused (wasPlaying), only if still the same live song on the player
  (_audioSeekGen unchanged), and never auto-resume a song the user had paused.
- Trap Tab inside the dialog; backdrop click was already Stay.

Specs: exit-confirm.spec.ts updated — the monotonic "second Escape leaves" test
becomes "second Escape stays", plus a backdrop-click-stays test. The audio
pause/resume itself is verified manually on web + desktop (the mock song has no
backing track); these specs lock the navigation + keyboard semantics.

NOTE: the pause/resume adds a new pause→resume cycle on the desktop JUCE
transport (known play/pause-desync path) — smoke-test on the desktop build
before merge.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF

* fix(player): accurate exit-confirm copy + keep resume snapshot on failed load

Two review follow-ups on the Escape/resume/confirm work:

- Settings copy said "a second Escape (or Space/Enter) still leaves",
  but Escape dismisses the confirm (Stay) like every other modal — only
  Space/Enter/Leave exit. Corrected the Gameplay-tab description so it
  matches the implementation (and the committed exit-confirm specs).

- resumeLastSession() cleared the snapshot BEFORE awaiting playSong(), so
  a transient load/connect failure permanently lost the Resume pill with
  no retry. Clear only after the load resolves; on failure keep the
  snapshot (and drop the pending in-memory resume) so the pill re-offers
  it on the next non-player screen.

All 16 Escape/resume/exit-confirm Playwright specs still pass.

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>
2026-06-28 12:32:33 +02:00

125 lines
5.9 KiB
TypeScript

import { test, expect } from '@playwright/test';
// Opt-in "Ask before leaving a song" confirm. Default OFF → Escape/✕ leave
// instantly. When ON, a true-modal confirm appears and PAUSES the song; Escape
// (like every other modal) DISMISSES it → Stay, so a second Escape returns to
// the song rather than leaving, and Space/Enter activate the default-focused
// "Leave". (The mock song has no backing audio, so the pause-on-open /
// resume-on-Stay is verified manually on web + desktop; these specs lock the
// navigation + keyboard semantics.)
const CONFIRM_KEY = 'confirmExitSong';
async function installMockSong(page) {
await page.evaluate(() => {
const messages = [
{ type: 'song_info', title: 'Mock Song', artist: 'Mock Artist', arrangement: 'Lead', arrangement_index: 0, duration: 90, tuning: [0, 0, 0, 0, 0, 0], stringCount: 6, arrangements: [{ index: 0, name: 'Lead', notes: 1 }] },
{ type: 'ready' },
];
class MockWebSocket {
static CONNECTING = 0; static OPEN = 1; static CLOSING = 2; static CLOSED = 3;
readyState = MockWebSocket.CONNECTING;
onopen = null; onmessage = null; onerror = null; onclose = null; url;
constructor(url) {
this.url = url;
setTimeout(() => {
this.readyState = MockWebSocket.OPEN;
if (this.onopen) this.onopen(new Event('open'));
for (const m of messages) if (this.onmessage) this.onmessage({ data: JSON.stringify(m) });
}, 0);
}
send() {}
close() { this.readyState = MockWebSocket.CLOSED; if (this.onclose) this.onclose(new CloseEvent('close')); }
}
// @ts-ignore
window.WebSocket = MockWebSocket;
});
}
async function openPlayerWithMockSong(page) {
await installMockSong(page);
await page.evaluate(async () => { /* @ts-ignore */ await window.playSong('mock-song.sloppak'); });
await page.waitForSelector('#player.active', { timeout: 5000 });
await expect(page.locator('#hud-title')).toHaveText('Mock Song', { timeout: 5000 });
}
test.describe('Exit-confirm toggle', () => {
test.beforeEach(async ({ page }) => {
// Suppress the first-run onboarding overlay (a modal that intercepts
// pointer/keyboard events) so Escape reaches the player, not the overlay.
await page.route('**/api/profile', async (route) => {
if (route.request().method() === 'GET') {
await route.fulfill({ json: { display_name: 'Test', player_hash: 'test', onboarded: true } });
} else { await route.continue(); }
});
await page.goto('/');
await page.waitForSelector('.screen.active', { timeout: 10000 });
await page.evaluate((k) => localStorage.removeItem(k), CONFIRM_KEY);
});
test('default OFF: Escape exits the song immediately, no confirm', async ({ page }) => {
await openPlayerWithMockSong(page);
await page.keyboard.press('Escape');
await expect(page.locator('#fb-exit-confirm')).toHaveCount(0);
await expect(page.locator('#player.active')).toHaveCount(0);
});
test('ON: Escape opens the confirm and the song stays', async ({ page }) => {
await page.evaluate(() => { /* @ts-ignore */ window.setConfirmExitSong(true); });
await openPlayerWithMockSong(page);
await page.keyboard.press('Escape');
await expect(page.locator('#fb-exit-confirm')).toBeVisible();
await expect(page.locator('#player.active')).toHaveCount(1);
// "Leave" is focused so Space/Enter leaves immediately.
await expect(page.locator('#fb-exit-confirm button', { hasText: 'Leave' })).toBeFocused();
});
test('ON: a second Escape dismisses the prompt and stays in the song', async ({ page }) => {
await page.evaluate(() => { /* @ts-ignore */ window.setConfirmExitSong(true); });
await openPlayerWithMockSong(page);
await page.keyboard.press('Escape');
await expect(page.locator('#fb-exit-confirm')).toBeVisible();
// Escape = dismiss (Stay), matching every other modal — NOT leave.
await page.keyboard.press('Escape');
await expect(page.locator('#fb-exit-confirm')).toHaveCount(0);
await expect(page.locator('#player.active')).toHaveCount(1);
});
test('ON: clicking the backdrop dismisses the prompt and stays', async ({ page }) => {
await page.evaluate(() => { /* @ts-ignore */ window.setConfirmExitSong(true); });
await openPlayerWithMockSong(page);
await page.keyboard.press('Escape');
await expect(page.locator('#fb-exit-confirm')).toBeVisible();
// mousedown on the overlay backdrop (top-left, away from the centered card)
// is Stay — never an accidental leave.
await page.locator('#fb-exit-confirm').click({ position: { x: 5, y: 5 } });
await expect(page.locator('#fb-exit-confirm')).toHaveCount(0);
await expect(page.locator('#player.active')).toHaveCount(1);
});
test('ON: "Stay" keeps you in the song; "Leave" exits', async ({ page }) => {
await page.evaluate(() => { /* @ts-ignore */ window.setConfirmExitSong(true); });
await openPlayerWithMockSong(page);
await page.keyboard.press('Escape');
await page.locator('#fb-exit-confirm button', { hasText: 'Stay' }).click();
await expect(page.locator('#fb-exit-confirm')).toHaveCount(0);
await expect(page.locator('#player.active')).toHaveCount(1);
await page.keyboard.press('Escape');
await page.locator('#fb-exit-confirm button', { hasText: 'Leave' }).click();
await expect(page.locator('#fb-exit-confirm')).toHaveCount(0);
await expect(page.locator('#player.active')).toHaveCount(0);
});
test('ON: Enter on the default-focused "Leave" leaves', async ({ page }) => {
await page.evaluate(() => { /* @ts-ignore */ window.setConfirmExitSong(true); });
await openPlayerWithMockSong(page);
await page.keyboard.press('Escape');
await expect(page.locator('#fb-exit-confirm')).toBeVisible();
await page.keyboard.press('Enter');
await expect(page.locator('#fb-exit-confirm')).toHaveCount(0);
await expect(page.locator('#player.active')).toHaveCount(0);
});
});