Skip to content

Commit 2afc2a5

Browse files
author
Fahim Murshed
committed
Fix React rendering, unmount memory leaks, and play button accessibility in the Playlist block
Fix React rendering, unmount memory leaks, and play button accessibility in the Playlist block.
1 parent 8dddee4 commit 2afc2a5

8 files changed

Lines changed: 201 additions & 72 deletions

File tree

changelog.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
== Changelog ==
22

3+
= 23.6.0-rc.2 =
4+
5+
6+
## Changelog
7+
8+
### Bug Fixes
9+
10+
#### Block Library
11+
- Fix React rendering, unmount memory leaks, and play button accessibility in the Playlist block. ([80468](https://github.com/WordPress/gutenberg/pull/80468))
12+
13+
314
= 23.6.0-rc.1 =
415

516

gutenberg.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Description: Printing since 1440. This is the development plugin for the block editor, site editor, and other future WordPress core functionality.
66
* Requires at least: 6.9
77
* Requires PHP: 7.4
8-
* Version: 23.6.0-rc.1
8+
* Version: 23.6.0-rc.2
99
* Author: Gutenberg Team
1010
* Text Domain: gutenberg
1111
*

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "gutenberg",
3-
"version": "23.6.0-rc.1",
3+
"version": "23.6.0-rc.2",
44
"private": true,
55
"description": "A new WordPress editor experience.",
66
"author": "The WordPress Contributors",

packages/block-library/src/playlist-track/edit.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,17 @@ const PlaylistTrackEdit = ( {
273273
</InspectorControls>
274274
<li { ...blockProps }>
275275
{ !! temporaryURL && <Spinner /> }
276-
<button
276+
<div
277277
className="wp-block-playlist-track__button"
278278
onClick={ () => setCurrentTrackClientId( clientId ) }
279+
onKeyDown={ ( event ) => {
280+
if ( event.key === 'Enter' || event.key === ' ' ) {
281+
event.preventDefault();
282+
setCurrentTrackClientId( clientId );
283+
}
284+
} }
285+
role="button"
286+
tabIndex={ 0 }
279287
aria-current={
280288
currentTrackClientId === clientId ? 'true' : 'false'
281289
}
@@ -323,7 +331,7 @@ const PlaylistTrackEdit = ( {
323331
{ length }
324332
</span>
325333
<span className="screen-reader-text">{ __( 'Play' ) }</span>
326-
</button>
334+
</div>
327335
</li>
328336
</>
329337
);

packages/block-library/src/playlist/edit.js

Lines changed: 81 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@ import clsx from 'clsx';
66
/**
77
* WordPress dependencies
88
*/
9-
import { useCallback, useEffect, useMemo, useState } from '@wordpress/element';
9+
import {
10+
useCallback,
11+
useEffect,
12+
useMemo,
13+
useRef,
14+
useState,
15+
} from '@wordpress/element';
1016
import {
1117
store as blockEditorStore,
1218
MediaPlaceholder,
@@ -121,8 +127,8 @@ const PlaylistEdit = ( {
121127
gradients.length > 0 || ! colorGradientSettings.disableCustomGradients;
122128
const waveformGradientValue = waveformGradient;
123129
const waveformBackgroundGradientValue = waveformBackgroundGradient;
124-
let waveformColorGradientChange;
125-
let waveformBackgroundColorGradientChange;
130+
const colorGradientChangeRef = useRef();
131+
const backgroundColorGradientChangeRef = useRef();
126132
const onUploadError = useCallback(
127133
( message ) => {
128134
createErrorNotice( message, { type: 'snackbar' } );
@@ -343,71 +349,89 @@ const PlaylistEdit = ( {
343349
[ setAttributes ]
344350
);
345351

346-
function updateWaveformColor( colorValue ) {
347-
const isSettingColor = colorValue !== undefined;
348-
if ( ! isSettingColor && waveformColorGradientChange === 'gradient' ) {
349-
waveformColorGradientChange = undefined;
350-
return;
351-
}
352+
const updateWaveformColor = useCallback(
353+
( colorValue ) => {
354+
const isSettingColor = colorValue !== undefined;
355+
if (
356+
! isSettingColor &&
357+
colorGradientChangeRef.current === 'gradient'
358+
) {
359+
colorGradientChangeRef.current = undefined;
360+
return;
361+
}
352362

353-
waveformColorGradientChange = 'color';
363+
colorGradientChangeRef.current = 'color';
354364

355-
setAttributes( {
356-
waveformColor: colorValue,
357-
waveformGradient: undefined,
358-
} );
359-
}
365+
setAttributes( {
366+
waveformColor: colorValue,
367+
waveformGradient: undefined,
368+
} );
369+
},
370+
[ setAttributes ]
371+
);
360372

361-
function updateWaveformGradient( gradientValue ) {
362-
const isSettingGradient = gradientValue !== undefined;
363-
if ( ! isSettingGradient && waveformColorGradientChange === 'color' ) {
364-
waveformColorGradientChange = undefined;
365-
return;
366-
}
373+
const updateWaveformGradient = useCallback(
374+
( gradientValue ) => {
375+
const isSettingGradient = gradientValue !== undefined;
376+
if (
377+
! isSettingGradient &&
378+
colorGradientChangeRef.current === 'color'
379+
) {
380+
colorGradientChangeRef.current = undefined;
381+
return;
382+
}
367383

368-
waveformColorGradientChange = 'gradient';
384+
colorGradientChangeRef.current = 'gradient';
369385

370-
setAttributes( {
371-
waveformGradient: gradientValue,
372-
waveformColor: undefined,
373-
} );
374-
}
386+
setAttributes( {
387+
waveformGradient: gradientValue,
388+
waveformColor: undefined,
389+
} );
390+
},
391+
[ setAttributes ]
392+
);
375393

376-
function updateWaveformBackgroundColor( colorValue ) {
377-
const isSettingColor = colorValue !== undefined;
378-
if (
379-
! isSettingColor &&
380-
waveformBackgroundColorGradientChange === 'gradient'
381-
) {
382-
waveformBackgroundColorGradientChange = undefined;
383-
return;
384-
}
394+
const updateWaveformBackgroundColor = useCallback(
395+
( colorValue ) => {
396+
const isSettingColor = colorValue !== undefined;
397+
if (
398+
! isSettingColor &&
399+
backgroundColorGradientChangeRef.current === 'gradient'
400+
) {
401+
backgroundColorGradientChangeRef.current = undefined;
402+
return;
403+
}
385404

386-
waveformBackgroundColorGradientChange = 'color';
405+
backgroundColorGradientChangeRef.current = 'color';
387406

388-
setAttributes( {
389-
waveformBackgroundColor: colorValue,
390-
waveformBackgroundGradient: undefined,
391-
} );
392-
}
407+
setAttributes( {
408+
waveformBackgroundColor: colorValue,
409+
waveformBackgroundGradient: undefined,
410+
} );
411+
},
412+
[ setAttributes ]
413+
);
393414

394-
function updateWaveformBackgroundGradient( gradientValue ) {
395-
const isSettingGradient = gradientValue !== undefined;
396-
if (
397-
! isSettingGradient &&
398-
waveformBackgroundColorGradientChange === 'color'
399-
) {
400-
waveformBackgroundColorGradientChange = undefined;
401-
return;
402-
}
415+
const updateWaveformBackgroundGradient = useCallback(
416+
( gradientValue ) => {
417+
const isSettingGradient = gradientValue !== undefined;
418+
if (
419+
! isSettingGradient &&
420+
backgroundColorGradientChangeRef.current === 'color'
421+
) {
422+
backgroundColorGradientChangeRef.current = undefined;
423+
return;
424+
}
403425

404-
waveformBackgroundColorGradientChange = 'gradient';
426+
backgroundColorGradientChangeRef.current = 'gradient';
405427

406-
setAttributes( {
407-
waveformBackgroundGradient: gradientValue,
408-
waveformBackgroundColor: undefined,
409-
} );
410-
}
428+
setAttributes( {
429+
waveformBackgroundGradient: gradientValue,
430+
waveformBackgroundColor: undefined,
431+
} );
432+
},
433+
[ setAttributes ]
434+
);
411435

412436
const colorSettings = [];
413437
if ( hasColors || hasGradients ) {

packages/block-library/src/playlist/view.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
initWaveformPlayer,
1111
logPlayError,
1212
setupPlayButtonArtwork,
13+
updatePlayButtonAccessibilityLabel,
1314
updateSeekControlLabel,
1415
} from '../utils/waveform-utils';
1516

@@ -88,6 +89,15 @@ const { state } = store(
8889
const shouldAutoPlay = !! existing?.url;
8990

9091
initPlayer( ref, track, shouldAutoPlay, context );
92+
93+
return () => {
94+
if ( ! ref.isConnected ) {
95+
const player = playerState.get( ref );
96+
player?.destroy?.();
97+
playerState.delete( ref );
98+
playlistPlayerState.delete( context.playlistId );
99+
}
100+
};
91101
},
92102
},
93103
},
@@ -138,6 +148,27 @@ function initPlayer( ref, track, shouldAutoPlay, context ) {
138148
track.image
139149
);
140150
}
151+
if ( track.title ) {
152+
existing.container.setAttribute(
153+
'data-title',
154+
track.title
155+
);
156+
} else {
157+
existing.container.removeAttribute( 'data-title' );
158+
}
159+
if ( track.artist ) {
160+
existing.container.setAttribute(
161+
'data-artist',
162+
track.artist
163+
);
164+
} else {
165+
existing.container.removeAttribute( 'data-artist' );
166+
}
167+
updatePlayButtonAccessibilityLabel(
168+
existing.container,
169+
existing.instance
170+
);
171+
141172
if ( shouldAutoPlay ) {
142173
existing.instance.play()?.catch( logPlayError );
143174
}

packages/block-library/src/utils/waveform-player.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
applyWaveformPlayerStyles,
1313
initWaveformPlayer,
1414
setupPlayButtonArtwork,
15+
updatePlayButtonAccessibilityLabel,
1516
updateSeekControlLabel,
1617
} from './waveform-utils';
1718

@@ -61,6 +62,18 @@ function updatePlayerMetadata(
6162
if ( showPlayButtonArtwork ) {
6263
setupPlayButtonArtwork( container, image );
6364
}
65+
66+
if ( title ) {
67+
container.setAttribute( 'data-title', title );
68+
} else {
69+
container.removeAttribute( 'data-title' );
70+
}
71+
if ( artist ) {
72+
container.setAttribute( 'data-artist', artist );
73+
} else {
74+
container.removeAttribute( 'data-artist' );
75+
}
76+
updatePlayButtonAccessibilityLabel( container, instance );
6477
}
6578

6679
/**

0 commit comments

Comments
 (0)