From 4cb482c3f287abd2b489d81a8d4b27eaa6660a92 Mon Sep 17 00:00:00 2001 From: Richard R Date: Mon, 4 May 2026 19:38:23 -0600 Subject: [PATCH] refactor(tts): enhance pause handling and centralize TTS test helpers Update TTSContext to track manual pause epochs, ensuring explicit user pauses are respected during async sentence splitting and playback state restoration. Adjust TTSPlayer button disabling logic to allow pausing while processing. Move and improve pauseTTSAndVerify helper for robust cross-browser TTS pause assertions, replacing local test implementations. Update expectDocumentListed to use polling for document link visibility. --- src/components/player/TTSPlayer.tsx | 2 +- src/contexts/TTSContext.tsx | 17 ++++++++++++++--- tests/accessibility.spec.ts | 5 ++--- tests/helpers.ts | 29 ++++++++++++++++++++++------- tests/play.spec.ts | 15 ++++----------- 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/components/player/TTSPlayer.tsx b/src/components/player/TTSPlayer.tsx index 13054b4..c38dc23 100644 --- a/src/components/player/TTSPlayer.tsx +++ b/src/components/player/TTSPlayer.tsx @@ -62,7 +62,7 @@ export default function TTSPlayer({ currentPage, numPages }: { onClick={togglePlay} className="relative p-1.5 rounded-md text-foreground hover:bg-offbase transition-all duration-200 focus:outline-none h-8 w-8 flex items-center justify-center transform ease-in-out hover:scale-[1.09] hover:text-accent" aria-label={isPlaying ? 'Pause' : 'Play'} - disabled={isProcessing} + disabled={isProcessing && !isPlaying} > {isPlaying ? : } diff --git a/src/contexts/TTSContext.tsx b/src/contexts/TTSContext.tsx index 61e4eac..a4b92c6 100644 --- a/src/contexts/TTSContext.tsx +++ b/src/contexts/TTSContext.tsx @@ -433,6 +433,7 @@ export function TTSProvider({ children }: { children: ReactNode }): ReactElement const [currentSentenceAlignment, setCurrentSentenceAlignment] = useState(); const [currentWordIndex, setCurrentWordIndex] = useState(null); const isPlayingRef = useRef(false); + const pauseEpochRef = useRef(0); const sentencesRef = useRef([]); const currentIndexRef = useRef(0); const setTextRef = useRef<(text: string, options?: boolean | SetTextOptions) => void>(() => { }); @@ -642,14 +643,21 @@ export function TTSProvider({ children }: { children: ReactNode }): ReactElement } }, [activeHowl, clearRateWatchdog]); + const recordManualPause = useCallback(() => { + // Cancel any queued auto-resume intent and mark an explicit user pause. + resumeAfterLocationChangeRef.current = false; + pauseEpochRef.current += 1; + }, []); + /** * Pauses the current audio playback * Used for external control of playback state */ const pause = useCallback(() => { + recordManualPause(); pauseActiveHowl(); setIsPlaying(false); - }, [pauseActiveHowl]); + }, [pauseActiveHowl, recordManualPause]); /** * Navigates to a specific location in the document @@ -882,6 +890,7 @@ export function TTSProvider({ children }: { children: ReactNode }): ReactElement if (handleBlankSection(workingText)) return; const shouldPause = normalizedOptions.shouldPause ?? false; + const pauseEpochAtStart = pauseEpochRef.current; const pendingAutoResume = resumeAfterLocationChangeRef.current; const shouldResumePlayback = !shouldPause && (isPlaying || pendingAutoResume); if (shouldPause || pendingAutoResume) { @@ -968,7 +977,8 @@ export function TTSProvider({ children }: { children: ReactNode }): ReactElement setIsProcessing(false); // Restore playback state if needed - if (shouldResumePlayback) { + // Respect explicit pauses that happened while sentence splitting was in flight. + if (shouldResumePlayback && pauseEpochRef.current === pauseEpochAtStart) { setIsPlaying(true); } }) @@ -990,6 +1000,7 @@ export function TTSProvider({ children }: { children: ReactNode }): ReactElement */ const togglePlay = useCallback(() => { if (isPlaying) { + recordManualPause(); pauseActiveHowl(); setIsPlaying(false); return; @@ -1014,7 +1025,7 @@ export function TTSProvider({ children }: { children: ReactNode }): ReactElement } setIsPlaying(true); - }, [activeHowl, applyPlaybackRateToHowl, isPlaying, pauseActiveHowl, unlockPlaybackOnUserGesture]); + }, [activeHowl, applyPlaybackRateToHowl, isPlaying, pauseActiveHowl, recordManualPause, unlockPlaybackOnUserGesture]); /** diff --git a/tests/accessibility.spec.ts b/tests/accessibility.spec.ts index 3370483..f3e4504 100644 --- a/tests/accessibility.spec.ts +++ b/tests/accessibility.spec.ts @@ -5,6 +5,7 @@ import { ensureDocumentsListed, uploadAndDisplay, expectProcessingTransition, + pauseTTSAndVerify, } from './helpers'; test.describe('Accessibility smoke', () => { @@ -81,8 +82,6 @@ test.describe('Accessibility smoke', () => { await playBtn.click(); await expectProcessingTransition(page); await expect(page.getByRole('button', { name: 'Pause' })).toBeVisible(); - - await page.getByRole('button', { name: 'Pause' }).click(); - await expect(page.getByRole('button', { name: 'Play' })).toBeVisible({ timeout: 10000 }); + await pauseTTSAndVerify(page); }); }); diff --git a/tests/helpers.ts b/tests/helpers.ts index 34de9e5..9c6ccd0 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -190,11 +190,23 @@ export async function playTTSAndWaitForASecond(page: Page, fileName: string) { * Pause TTS playback and verify paused state */ export async function pauseTTSAndVerify(page: Page) { - // Click pause to stop playback - await page.getByRole('button', { name: 'Pause' }).click(); + const ttsbar = page.locator('[data-app-ttsbar]'); + const pauseButton = ttsbar.getByRole('button', { name: 'Pause' }).first(); + await expect(pauseButton).toBeVisible({ timeout: 15000 }); + await expect(pauseButton).toBeEnabled({ timeout: 15000 }); - // Check for play button to be visible - await expect(page.getByRole('button', { name: 'Play' })).toBeVisible({ timeout: 10000 }); + // Retry pause because Firefox can race with transient processing transitions. + for (let attempt = 0; attempt < 3; attempt++) { + await pauseButton.click(); + try { + await expect(ttsbar.getByRole('button', { name: 'Play' }).first()).toBeVisible({ timeout: 5000 }); + await expectMediaState(page, 'paused'); + return; + } catch { + if (attempt === 2) throw new Error('Failed to pause TTS playback after retries'); + await page.waitForTimeout(250); + } + } } /** @@ -318,9 +330,11 @@ export async function dispatchHtml5DragAndDrop(page: Page, source: Locator, targ // Assert a document link containing the given filename appears in the list export async function expectDocumentListed(page: Page, fileName: string) { - await expect( - page.getByRole('link', { name: new RegExp(escapeRegExp(fileName), 'i') }).first() - ).toBeVisible({ timeout: 10000 }); + const link = page.getByRole('link', { name: new RegExp(escapeRegExp(fileName), 'i') }).first(); + await expect + .poll(async () => link.count(), { timeout: 20000 }) + .toBeGreaterThan(0); + await expect(link).toBeVisible({ timeout: 15000 }); } // Assert a document link containing the given filename does NOT exist @@ -334,6 +348,7 @@ export async function expectNoDocumentLink(page: Page, fileName: string) { export async function uploadFiles(page: Page, ...fileNames: string[]) { for (const name of fileNames) { await uploadFile(page, name); + await expectDocumentListed(page, name); } } diff --git a/tests/play.spec.ts b/tests/play.spec.ts index b9bc0c8..e9a6b3a 100644 --- a/tests/play.spec.ts +++ b/tests/play.spec.ts @@ -1,22 +1,15 @@ -import { test, expect } from '@playwright/test'; +import { test, expect, type Page } from '@playwright/test'; import { setupTest, playTTSAndWaitForASecond, + pauseTTSAndVerify, openVoicesMenu, selectVoiceAndAssertPlayback, expectMediaState, expectProcessingTransition, } from './helpers'; -// Single-spec helpers kept local instead of living in shared helpers: -async function pauseTTSAndVerify(page: any) { - // Click pause to stop playback - await page.getByRole('button', { name: 'Pause' }).click(); - // Check for play button to be visible - await expect(page.getByRole('button', { name: 'Play' })).toBeVisible({ timeout: 10000 }); -} - -async function openSpeedPopover(page: any) { +async function openSpeedPopover(page: Page) { const ttsbar = page.locator('[data-app-ttsbar]'); const buttons = ttsbar.getByRole('button'); // Heuristic: the Speed control is the first button in the TTS bar and shows something like "1x" @@ -27,7 +20,7 @@ async function openSpeedPopover(page: any) { await page.waitForSelector('input[type="range"]', { timeout: 10000 }); } -async function changeNativeSpeedAndAssert(page: any, newSpeed: number) { +async function changeNativeSpeedAndAssert(page: Page, newSpeed: number) { await openSpeedPopover(page); const slider = page.locator('input[type="range"]').first();