From 386c5106252061cade00546bf4d8680d1e99a71a Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Mon, 13 Jul 2026 14:58:51 +0100 Subject: [PATCH] Simplify playlist waveform metadata updates --- package-lock.json | 8 +- packages/block-library/package.json | 2 +- packages/block-library/src/playlist/view.js | 48 ++++----- .../src/utils/test/waveform-player.js | 100 +++++++++++------- .../src/utils/waveform-player.js | 37 +++---- 5 files changed, 101 insertions(+), 94 deletions(-) diff --git a/package-lock.json b/package-lock.json index ad3c7aff759630..8673b831a7341c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43869,7 +43869,7 @@ "version": "10.1.0", "license": "GPL-2.0-or-later", "dependencies": { - "@arraypress/waveform-player": "^1.20.0", + "@arraypress/waveform-player": "^1.21.0", "@wordpress/a11y": "file:../a11y", "@wordpress/api-fetch": "file:../api-fetch", "@wordpress/autop": "file:../autop", @@ -43944,9 +43944,9 @@ } }, "packages/block-library/node_modules/@arraypress/waveform-player": { - "version": "1.20.0", - "resolved": "https://registry.npmjs.org/@arraypress/waveform-player/-/waveform-player-1.20.0.tgz", - "integrity": "sha512-XVUdhbWh26+U2GbslRsCiCAqIMGRbp0v1QoePjFqz16elnWmycQHxQajWLhh8DE/VT07JAXZ8hiu6tZ3YhFzRQ==", + "version": "1.21.0", + "resolved": "https://registry.npmjs.org/@arraypress/waveform-player/-/waveform-player-1.21.0.tgz", + "integrity": "sha512-ERRlqZR7qCl7qzj6H8KpIV3y7rUDh/0W4p5vz0/iIoxxeK7SXBENCrN2Q3h5XlawKKQnw6cVpcjAk1IY6gLbYA==", "license": "MIT", "engines": { "node": ">=14.0.0" diff --git a/packages/block-library/package.json b/packages/block-library/package.json index 5215b0a52afd79..4220b08c2d2e65 100644 --- a/packages/block-library/package.json +++ b/packages/block-library/package.json @@ -92,7 +92,7 @@ "build-module/*/init.mjs" ], "dependencies": { - "@arraypress/waveform-player": "^1.20.0", + "@arraypress/waveform-player": "^1.21.0", "@wordpress/a11y": "file:../a11y", "@wordpress/api-fetch": "file:../api-fetch", "@wordpress/autop": "file:../autop", diff --git a/packages/block-library/src/playlist/view.js b/packages/block-library/src/playlist/view.js index e125fac806b540..ffee72c40e5d68 100644 --- a/packages/block-library/src/playlist/view.js +++ b/packages/block-library/src/playlist/view.js @@ -106,35 +106,25 @@ function initPlayer( ref, track, shouldAutoPlay, context ) { // If a player already exists, load the new track without recreating. if ( existing?.instance ) { - const shouldRecreatePlayer = - !! existing.instance.artworkEl !== !! track.image; - - if ( shouldRecreatePlayer ) { - existing.destroy?.(); - playerState.delete( ref ); - } else { - playlistPlayerState.set( context.playlistId, existing ); - existing.instance - .loadTrack( track.url, track.title, track.artist, { - artwork: track.image, - } ) - .then( () => { - existing.url = track.url; - if ( existing.instance.artworkEl ) { - existing.instance.artworkEl.alt = track.imageAlt || ''; - } - // loadTrack() preserves the previous explicit seekLabel option. - updateSeekControlLabel( - existing.instance, - track.title || ref.dataset.labelSeek - ); - if ( shouldAutoPlay ) { - existing.instance.play()?.catch( logPlayError ); - } - } ) - .catch( logPlayError ); - return; - } + playlistPlayerState.set( context.playlistId, existing ); + existing.instance + .loadTrack( track.url, track.title, track.artist, { + artwork: track.image, + artworkAlt: track.imageAlt, + } ) + .then( () => { + existing.url = track.url; + // loadTrack() preserves the previous explicit seekLabel option. + updateSeekControlLabel( + existing.instance, + track.title || ref.dataset.labelSeek + ); + if ( shouldAutoPlay ) { + existing.instance.play()?.catch( logPlayError ); + } + } ) + .catch( logPlayError ); + return; } // Read translated labels from server-rendered data attributes. diff --git a/packages/block-library/src/utils/test/waveform-player.js b/packages/block-library/src/utils/test/waveform-player.js index 42c468efccc668..631362d8de6f88 100644 --- a/packages/block-library/src/utils/test/waveform-player.js +++ b/packages/block-library/src/utils/test/waveform-player.js @@ -26,14 +26,12 @@ jest.mock( '../waveform-utils', () => ( { function createFakePlayer( options, element ) { const titleEl = document.createElement( 'span' ); titleEl.textContent = options.title ?? ''; - // The artist and artwork elements only exist when the track had an - // artist/image when the player was created, mirroring the library markup. - let artistEl = null; + let artistEl; if ( options.artist ) { artistEl = document.createElement( 'span' ); artistEl.textContent = options.artist; } - let artworkEl = null; + let artworkEl; if ( options.image ) { artworkEl = document.createElement( 'img' ); artworkEl.src = options.image; @@ -48,23 +46,49 @@ function createFakePlayer( options, element ) { element.append( artworkEl ); } + const instance = { + titleEl, + artistEl: artistEl || null, + artworkEl: artworkEl || null, + pause: jest.fn(), + syncArtist: jest.fn( ( artist ) => { + if ( ! artist ) { + instance.artistEl?.remove(); + instance.artistEl = null; + return; + } + if ( ! instance.artistEl ) { + instance.artistEl = document.createElement( 'span' ); + element.append( instance.artistEl ); + } + instance.artistEl.textContent = artist; + instance.artistEl.style.display = ''; + } ), + syncArtwork: jest.fn( ( image, imageAlt = '' ) => { + if ( ! image ) { + instance.artworkEl?.remove(); + instance.artworkEl = null; + return; + } + if ( ! instance.artworkEl ) { + instance.artworkEl = document.createElement( 'img' ); + element.append( instance.artworkEl ); + } + instance.artworkEl.src = image; + instance.artworkEl.alt = imageAlt || ''; + } ), + loadTrack: jest.fn( async ( src, title, artist, trackOptions ) => { + titleEl.textContent = title; + instance.syncArtist( artist ); + instance.syncArtwork( + trackOptions.artwork, + trackOptions.artworkAlt + ); + } ), + }; + return { - instance: { - titleEl, - artistEl, - artworkEl, - pause: jest.fn(), - loadTrack: jest.fn( async ( src, title, artist, trackOptions ) => { - titleEl.textContent = title; - if ( artistEl ) { - artistEl.textContent = artist; - artistEl.style.display = artist ? '' : 'none'; - } - if ( artworkEl && trackOptions.artwork ) { - artworkEl.src = trackOptions.artwork; - } - } ), - }, + instance, destroy: jest.fn(), }; } @@ -168,9 +192,18 @@ describe( 'WaveformPlayer', () => { expect( player.destroy ).not.toHaveBeenCalled(); expect( initWaveformPlayer ).toHaveBeenCalledTimes( 1 ); + expect( player.instance.loadTrack ).toHaveBeenCalledWith( + 'https://example.com/other.mp3', + 'Original Title', + 'Original Artist', + { + artwork: 'https://example.com/cover.jpg', + artworkAlt: 'A bright abstract album cover', + } + ); } ); - it( 'recreates the player to show an image added to a track that had none', () => { + it( 'updates the player in place to show an image added to a track that had none', () => { const { rerender } = render( ); @@ -194,16 +227,15 @@ describe( 'WaveformPlayer', () => { jest.advanceTimersByTime( 100 ); } ); - expect( firstPlayer.destroy ).toHaveBeenCalledTimes( 1 ); - expect( initWaveformPlayer ).toHaveBeenCalledTimes( 2 ); - const secondPlayer = initWaveformPlayer.mock.results[ 1 ].value; - expect( secondPlayer.instance.artworkEl ).toHaveAttribute( + expect( firstPlayer.destroy ).not.toHaveBeenCalled(); + expect( initWaveformPlayer ).toHaveBeenCalledTimes( 1 ); + expect( firstPlayer.instance.artworkEl ).toHaveAttribute( 'src', 'https://example.com/added.jpg' ); } ); - it( 'recreates the player when the image is removed', () => { + it( 'updates the player in place when the image is removed', () => { const { rerender } = render( ); act( () => { @@ -218,10 +250,9 @@ describe( 'WaveformPlayer', () => { jest.advanceTimersByTime( 100 ); } ); - expect( player.destroy ).toHaveBeenCalledTimes( 1 ); - expect( initWaveformPlayer ).toHaveBeenCalledTimes( 2 ); - const secondPlayer = initWaveformPlayer.mock.results[ 1 ].value; - expect( secondPlayer.instance.artworkEl ).toBeNull(); + expect( player.destroy ).not.toHaveBeenCalled(); + expect( initWaveformPlayer ).toHaveBeenCalledTimes( 1 ); + expect( player.instance.artworkEl ).toBeNull(); } ); it( 'updates the player in place to show an artist added to a track that had none', () => { @@ -234,9 +265,7 @@ describe( 'WaveformPlayer', () => { } ); const firstPlayer = initWaveformPlayer.mock.results[ 0 ].value; - // The editor seeds a hidden artist element so artist edits can - // update in place. - expect( firstPlayer.instance.artistEl ).toHaveTextContent( '' ); + expect( firstPlayer.instance.artistEl ).toBeNull(); rerender( ); @@ -263,9 +292,6 @@ describe( 'WaveformPlayer', () => { expect( player.destroy ).not.toHaveBeenCalled(); expect( initWaveformPlayer ).toHaveBeenCalledTimes( 1 ); - expect( player.instance.artistEl ).toHaveTextContent( '' ); - expect( player.instance.artistEl ).toHaveStyle( { - display: 'none', - } ); + expect( player.instance.artistEl ).toBeNull(); } ); } ); diff --git a/packages/block-library/src/utils/waveform-player.js b/packages/block-library/src/utils/waveform-player.js index fca44ee6aeb1e4..8a5d9034b280af 100644 --- a/packages/block-library/src/utils/waveform-player.js +++ b/packages/block-library/src/utils/waveform-player.js @@ -10,20 +10,11 @@ import { __, _x } from '@wordpress/i18n'; */ import { initWaveformPlayer, updateSeekControlLabel } from './waveform-utils'; -const EMPTY_ARTIST_PLACEHOLDER = '\u00a0'; - /** - * Update the DOM of a WaveformPlayer element to reflect current props. - * - * The @arraypress/waveform-player library currently offers no public method to - * do this, hence the workaround. Context: - * - * - The title and artist elements are guaranteed to exist based on - * `initWaveformPlayer` and implementation details of the library. + * Update the metadata of a WaveformPlayer element to reflect current props. * - * - The image/artwork element may not exist if the track loaded upon init had - * no defined artwork, but in these cases we create the entire WaveformPlayer - * instance. See `hasImage`. + * `loadTrack()` owns full track swaps, but it also resets the audio element. + * This keeps same-source metadata edits lightweight in the editor. * * @param {Object} instance - The waveform player instance. * @param {Object} metadata - The track metadata. @@ -37,11 +28,17 @@ function updatePlayerMetadata( instance, { title, artist, image, imageAlt } ) { instance.titleEl.textContent = title ?? ''; } updateSeekControlLabel( instance, title || __( 'Seek' ) ); - if ( instance.artistEl ) { + + if ( typeof instance.syncArtist === 'function' ) { + instance.syncArtist( artist || '' ); + } else if ( instance.artistEl ) { instance.artistEl.textContent = artist ?? ''; instance.artistEl.style.display = artist ? '' : 'none'; } - if ( instance.artworkEl && image ) { + + if ( typeof instance.syncArtwork === 'function' ) { + instance.syncArtwork( image || null, imageAlt || '' ); + } else if ( instance.artworkEl && image ) { instance.artworkEl.src = image; instance.artworkEl.alt = imageAlt || ''; } @@ -82,12 +79,6 @@ export function WaveformPlayer( { // Ref for the WaveformPlayer instance const playerRef = useRef(); - // Due to how WaveformPlayer is implemented, the artwork element within the - // player element only exists when an image was present when the player was - // created. Recreate the player when one is added or removed so that - // element is created or torn down. - const hasImage = !! image; - // WaveformPlayer needs an audio source on init, but the source may change // throughout its lifetime. const hasSrc = !! src; @@ -115,8 +106,7 @@ export function WaveformPlayer( { const player = initWaveformPlayer( element, { src: metadataRef.current.src, title: metadataRef.current.title, - artist: - metadataRef.current.artist || EMPTY_ARTIST_PLACEHOLDER, + artist: metadataRef.current.artist, image: metadataRef.current.image, imageAlt: metadataRef.current.imageAlt, waveformStyle, @@ -151,7 +141,7 @@ export function WaveformPlayer( { playerDestroy?.(); }; }, - [ onEndedEvent, hasSrc, waveformStyle, hasImage ] + [ onEndedEvent, hasSrc, waveformStyle ] ); useEffect( () => { @@ -177,6 +167,7 @@ export function WaveformPlayer( { metadataRef.current.artist, { artwork: metadataRef.current.image, + artworkAlt: metadataRef.current.imageAlt, } ); if ( ! wasPlaying ) {