From a0cc10ffca959e71d8814ffd58f2e6ec567caef1 Mon Sep 17 00:00:00 2001 From: Mistral Vibe Date: Wed, 8 Apr 2026 21:31:08 +0200 Subject: [PATCH] fix(audio): re-attach waveform canvas on re-navigation to same song MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When navigating away from SongPage and back to the same song, the container div is a new DOM element but the URL is unchanged. The previous early-return (currentUrl === url) would skip initialization entirely, leaving WaveSurfer pointing at the detached old container — nothing rendered. Fix: track currentContainer alongside currentUrl. When URL matches but container has changed, call wavesurfer.setOptions({ container }) which moves the existing canvas into the new container without reloading audio or interrupting playback. WaveSurfer v7 renderer.setOptions() supports this: it calls newParent.appendChild(this.container) to relocate the canvas div. Three paths in initialize(): 1. Same URL + same container → no-op 2. Same URL + new container → setOptions re-attach (no reload) 3. Different URL → full teardown and reload Co-Authored-By: Claude Sonnet 4.6 --- web/src/services/audioService.ts | 17 ++++++++++++++--- web/tests/audioService.test.ts | 30 ++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/web/src/services/audioService.ts b/web/src/services/audioService.ts index 638c7e8..26cb26e 100755 --- a/web/src/services/audioService.ts +++ b/web/src/services/audioService.ts @@ -5,6 +5,7 @@ class AudioService { private static instance: AudioService; private wavesurfer: WaveSurfer | null = null; private currentUrl: string | null = null; + private currentContainer: HTMLElement | null = null; private isReady = false; private lastTimeUpdate = 0; // Persistent audio element attached to document.body so playback survives @@ -44,10 +45,18 @@ class AudioService { if (!container) throw new Error('Container element is required'); if (!url) throw new Error('Valid audio URL is required'); - // Reuse the existing instance when the URL hasn't changed - if (this.currentUrl === url && this.wavesurfer) return; + // Same URL and same container — nothing to do + if (this.currentUrl === url && this.wavesurfer && this.currentContainer === container) return; - // Tear down the previous instance and clear stale store state + // Same URL, different container: navigated away and back to the same song. + // Move the waveform canvas to the new container without reloading audio. + if (this.currentUrl === url && this.wavesurfer) { + this.wavesurfer.setOptions({ container }); + this.currentContainer = container; + return; + } + + // Different URL — tear down the previous instance and clear stale store state if (this.wavesurfer) { this.destroyWaveSurfer(); usePlayerStore.getState().batchUpdate({ isPlaying: false, currentTime: 0, duration: 0 }); @@ -72,6 +81,7 @@ class AudioService { this.wavesurfer = ws; this.currentUrl = url; + this.currentContainer = container; this.setupEventHandlers(ws); await new Promise((resolve, reject) => { @@ -162,6 +172,7 @@ class AudioService { this.mediaElement = null; this.wavesurfer = null; this.currentUrl = null; + this.currentContainer = null; this.isReady = false; } } diff --git a/web/tests/audioService.test.ts b/web/tests/audioService.test.ts index 510ebb9..b34a394 100644 --- a/web/tests/audioService.test.ts +++ b/web/tests/audioService.test.ts @@ -23,6 +23,7 @@ function createMockWaveSurfer(duration = 120) { isPlaying: vi.fn(() => false), unAll: vi.fn(), destroy: vi.fn(), + setOptions: vi.fn(), // Helper to fire events from tests _emit: (event: EventName, ...args: any[]) => handlers[event]?.(...args), }; @@ -114,7 +115,24 @@ describe('AudioService', () => { await expect(initPromise).rejects.toThrow('network error'); }); - it('reuses the existing instance for the same URL', async () => { + it('does nothing when called with same URL and same container', async () => { + const WaveSurfer = (await import('wavesurfer.js')).default; + const mockWs = createMockWaveSurfer(); + vi.mocked(WaveSurfer.create).mockReturnValue(mockWs as any); + + const url = 'http://example.com/same.mp3'; + const container = makeContainer(); + const p1 = service.initialize(container, url); + mockWs._emit('ready'); + await p1; + + const createCount = vi.mocked(WaveSurfer.create).mock.calls.length; + await service.initialize(container, url); // exact same reference + expect(vi.mocked(WaveSurfer.create).mock.calls.length).toBe(createCount); + expect(mockWs.setOptions).not.toHaveBeenCalled(); + }); + + it('re-attaches waveform to new container when same URL but container changed (re-navigation)', async () => { const WaveSurfer = (await import('wavesurfer.js')).default; const mockWs = createMockWaveSurfer(); vi.mocked(WaveSurfer.create).mockReturnValue(mockWs as any); @@ -124,9 +142,13 @@ describe('AudioService', () => { mockWs._emit('ready'); await p1; - const createCallCount = vi.mocked(WaveSurfer.create).mock.calls.length; - await service.initialize(makeContainer(), url); // same URL - expect(vi.mocked(WaveSurfer.create).mock.calls.length).toBe(createCallCount); // no new instance + const newContainer = makeContainer(); // different DOM element, same URL + const createCount = vi.mocked(WaveSurfer.create).mock.calls.length; + await service.initialize(newContainer, url); + + // No new WaveSurfer instance — just a canvas re-attach + expect(vi.mocked(WaveSurfer.create).mock.calls.length).toBe(createCount); + expect(mockWs.setOptions).toHaveBeenCalledWith({ container: newContainer }); }); it('resets store state when URL changes', async () => {