Playlist block #50664
Playlist block #50664
Conversation
carolinan
added
New Block
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/blocks.php ❔ lib/experimental/editor-settings.php ❔ lib/experiments-page.php |
|
I noticed that too. Having said that, it feels like it should work without it. |
|
I tried setting the |
|
I wonder if what happens in the block should be 1:1 with the media library? Ideally, when one uploads media we should detect and extract as much as possible from the metadata of the file. Then when we read that info pre-populate the info. But in the case of a playlist, maybe the editing needs to stay in the block as block atributes? Try to get anything from the media and then rely solely on the block. To me this is a less power user way to manage a playlist - the block being the truth - compared to managing files and metadata in the WordPress media library. There are many downsides to what I propose here, the main being that the same media reused with poor metadata will need to re-input the details in all the various places. Nevertheless maybe we can move on with this block holding details in block data and mull on fixing how media REST works in parallel? For instance the demo file has all this stuff readily available. Edits could be stored in the block?
|
|
Some good thoughts @draganescu. There's also a distinction of editing something globally or locally. If you're using the same track in two different playlists, should edits to the fields persist across both playlists? There's the possibility the user might want to present the same track differently. The file block has some precedence for this in how it works. Also I guess other media blocks already work like this with captions and alt text (though I think that's been criticized quite a lot). |
|
One of the reasons why I did not like using the attributes was that it is too much data, which to me feels like it increases the risk of the block breaking by user error. Before: Current: The |
@carolinan I can see why the trimmed down markup is less prone to breaking, but I would argue that it's not a concern strong engough to guide a whole lot of architecting around it.
Is this about manual editing of block markup? Because if it is, that should not be something we can even prevent. |
|
Ok I will try to rephrase. The main reason why I made it dynamic is because it is my opinion that if I update an attachment it should have the same content everywhere. I did not add that to the other comment because it had already been brought up. |
It's a well founded opinion based on years of how WordPress worked for so long. It also makes a lot of sense for normalized interaction - referenced truth source. However, I don't fully subscribe because blocks are not always in a direct relationship to the data they reference. Usually the blocks that reference data make it or try to make it very explicit that you're editing things that will reflect everywhere (template parts, patterns, navigation menus). I don't think we want that kind of UX for a playlist - e.g. you change the tracks and then you get a multi entity saving flow to announce you're actually editing attachment entities (reflected in the media library). It just seems a playlist block is like an image gallery, like a cover, like media / text, things that make content consumption better but are not an entity presenting a structure (say like a navigation menu). What do you think? Are my arguments downplaying attachment editing enough to allow us to move towards a block based data and see later if any other things actually don't work? There have been instances of static blocks turned dynamic too. |
There was a problem hiding this comment.
I've only inspected the frontend code, including the generated PHP and the view.js file. Great job 🙂
Using the HTML API to read the iAPI context of the tracks is very clever; we hadn't thought of it until now! Maybe we'll use that pattern in the Gallery block as well.
A few comments:
- I've noticed that you've used some imperative logic to control the frontend and in general, it's better to do everything in a declarative manner as it leads to less bugs.
- It seems there's a bug that always selects the first song even if you click on the others. I'm not sure how I ended up in that unstable state, nor how to fix it.
Screen.Recording.2024-11-28.at.08.35.16.mov
- To fix this bug and make things more declarative, it would be good to generate an array containing unique IDs for each track on the server, which can be used to know which audio is playing, which one is next, etc.
- It also seems there’s a bug when selecting an image in the editor. I can't select an image and have it appear on the frontend.
Screen.Recording.2024-11-28.at.08.43.25.mov
- I've also noticed that the
aria-labeldoesn't change when the Track changes. I understand that this is a bug, right?
I've made a small refactoring to try to fix these issues and implement the populated server array.
|
|
||
| // Add the markup for the current track | ||
| $html = '<div><div class="wp-block-playlist__current-item">'; | ||
| if ( $show_images && $current_image ) { |
There was a problem hiding this comment.
We always need to add the IMG element; otherwise, it will only be added if the first track contains an image. But if the first track doesn't contain an image and the following tracks contain images, those images won't be visible.
There was a problem hiding this comment.
Displaying an empty image tag is not a good practice.
There was a problem hiding this comment.
It is hidden when there is no image.
data-wp-bind--hidden="!state.currentTrack.image"
| $html .= | ||
| '<img | ||
| class="wp-block-playlist__item-image" | ||
| src={ ' . esc_url( $current_image ) . '} |
There was a problem hiding this comment.
The server directive processing of the Interactivity API takes care of adding the attributes to the HTML. There's no need to add them manually.
| src={ ' . esc_url( $current_image ) . '} |
(This applies to other places in the code, not just here)
There was a problem hiding this comment.
@carolinan It would be great if you could take a look at this refactoring, as I'm not sure if I've modified the behavior in any way.
Again, great work, this is a pretty cool block 🙂
PS: I haven't checked the communication between different playlist blocks. For example, if you play one, it should pause another. However, it's something that wouldn't be difficult to add.
| $media_id = $attributes['id']; | ||
| $attachment_meta = wp_get_attachment_metadata( $media_id ); | ||
| $wrapper_attributes = get_block_wrapper_attributes(); | ||
| $url = wp_get_attachment_url( $media_id ); |
There was a problem hiding this comment.
I switched to wp_get_attachment_url because $attachment_meta['url'] was giving me problems, but feel free to revert.
| // Finds the unique id of the current track and populates the playlist array. | ||
| $p = new WP_HTML_Tag_Processor( $content ); | ||
| $playlist_tracks = array(); | ||
| while ( $p->next_tag( 'button' ) ) { | ||
| $track_context = $p->get_attribute( 'data-wp-context' ); | ||
| $track_unique_id = json_decode( $track_context, true )['id']; | ||
| $state = wp_interactivity_state( 'core/playlist' ); | ||
| $playlist_tracks[] = $track_unique_id; | ||
| if ( | ||
| isset( $state['tracks'][ $track_unique_id ]['media_id'] ) && | ||
| $state['tracks'][ $track_unique_id ]['media_id'] === $current_media_id | ||
| ) { | ||
| $current_unique_id = $track_unique_id; | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm using your pattern to populate the array with all the tracks and the current track.
I haven't put any guard against undefined/null values, so it would be great if you could take a look. Thanks!
|
Thank you for the review @luisherranz. The image is not implemented, hence
As part of making the block dynamic I wanted to save the image to the attachment, and I had not started. |
|
It becomes confusing for me when I have a few people asking me to revert parts that make the block dynamic and different people making changes to the code that well, makes it more dynamic. -With "dynamic" I mean the parts where the block is fetching the data from the attachment at all times, instead of using block attributes. |
If you're referring to the Interactivity API, there's no problem. If the block is static, you just need to inject the directives using the HTML API, and that's it. The rest should work the same. |
@carolinan it is confusing! We're all sharing arguments I hope - the point is not to merge is to solve the problem in the best way we can today. @luisherranz I am arguing that we make the block not ask for attachment data on the front end but store track data as local attributes. My argument is here and here, basically:
Do you think that if the playlist block's front end behaviour is only related to its UI but the data comes from block attributes it is wrong? |
One interesting part of this is that, for a time during development, every track needed to be unique: you couldn't add it more than once, and the post id for the attachment could be used for this. |
I don't have a strong opinion about that (you'll probably have better luck asking other people), but I wonder: what does saving audio metadata as attributes have to do with whether the block is static or dynamic? |
It not a direct result, but if the block is dynamic because we need to save and get data from attachment entities on render, then having the data in block markup makes the need to be dynamic not a need anymore. |
|
Ok, thanks for the clarification. If you ever end up making this a static block, please do not serialize the directives of the Interactivity API. Inject them dynamically using the HTML API. |
Can you re-write that in plain English? |
|
Since there has not been any other input on the mechanics of the block for the past 5 days, I will continue to re-add the attributes.
|
What I mean is that if you finally convert this block into a static block, please don't add the directives to the content stored in the database (this is commonly known as "serializing"). Instead of doing that, use a |
|
I am still not sure I understand. I have pushed an update but I will not be able to continue until after the new year. |
Imagine that you have this dynamic block (simplified). <div data-wp-interactive="myPlugin" data-wp-init="actions.someInitialization">
<button data-wp-on--click="actions.someHandler" data-wp-text="state.buttonText"></button>
</div>Theoretically, if you want to move it to a static block, you could almost copy and paste the markup in the const save = () => (
<div data-wp-interactive="myPlugin" data-wp-init="actions.someInitialization">
<button data-wp-on--click="actions.someHandler" data-wp-text="state.buttonText"></button>
</div>
);But if you do that, there will have to be deprecations every time a change o refactoring is made to the frontend implementation. For that reason, instead of that, I recommend not storing the directives in the database, meaning they shouldn't exist in the const save = () => (
<div>
<button></button>
</div>
);And add them dynamically with a filter in PHP using the HTML Tag Processor (simplified). add_filter( 'render_block_my-plugin/my-block', function ( $content ) {
$p = WP_HTML_Tag_Processor( $content );
$p->next_tag( 'DIV' );
$p->set_attribute( 'data-wp-interactive', 'myPlugin' );
$p->set_attribute( 'data-wp-init', 'actions.someInitialization' );
$p->next_tag( 'BUTTON' );
$p->set_attribute( 'data-wp-on--click', 'actions.someHandler' );
$p->set_attribute( 'data-wp-text', 'state.buttonText' );
return $p->get_updated_html();
} );Let me know if this clarifies things or if there's still something you don't understand 🙂 |
|
Thank you. |
|
Oh, sure. You can use the render callback as well 🙂 |
What?
Adds a playlist and it's inner playlist track block.
Closes #805
Screenshots or screencast
Note: this video has no sound.
playlist-november-20.mp4
Why?
The playlist block has been requested since 2017. Without the block, a playlist can be added by using the classic block, but that is overly complicated.
How?
For this block, I started with one media type, audio, rather than trying to build it for both audio and video because it would increase the complexity.
I pictured that the playlist should work similarly to the "classic" WordPress playlist.
Meaning:
This PR does not connect the playlist block to the Media Library playlist options nor adds a new media frame.
To do:
Known issues:
The markup of the current track (at the top of the block) is different depending on the settings and track data.
This means that the WP_HTML_Tag_Processor used in index.php does not always find the right element to add the context to (The context used by the interactivity API).
Uploaded files sometimes miss the length.
Testing Instructions
Audio file for testing: https://wpthemetestdata.files.wordpress.com/2008/06/originaldixielandjazzbandwithalbernard-stlouisblues.mp3
Enable the experimental blocks option from the Gutenberg > Experiments plugin menu.
Upload a couple of audio files through the Media Library.
Edit the files in the Media Library and add title, artist and album to some but not all.
Add a featured image to at least oner file.
Insert the playlist block in the editor.
View the front of the website.
Screen Readers
Description of the block:
The block has three different sections.
At the top, there is an image, for example, an album cover, which is set to decorative with an empty alt attribute.
Next to the image is information about the current track: Title, album, and artist.
Below is a native audio HTML element with the standard controls for playing, pausing, volume, etc.
Below the audio element is the tracklist.
It is a list with buttons inside. Each list item and button represents a track and the button text is the song title, artist, and the length of the track.
The currently selected track: The one that is at the top of the block, has
aria-current=true.Activating a button selects that track.
Instructions
(I have only tested these steps with VoiceOver on mac)
Enable the experimental blocks option from the Gutenberg > Experiments plugin menu.
Upload a couple of audio files through the Media Library.
Edit the files in the Media Library and add title, artist and album to some but not all.
Add a featured image to at least oner file.
Add a playlist block in the block editor.
Focus is moved to an upload button in the block's placeholder.
Instructions about uploading an audio file or selecting one from the media library are announced.
Pressing the right arrow key or the tab key moves the focus from the Upload button to the Media Library button. Each button opens its respective file selection flow.
Confirm that it is possible to select multiple files.
When clicking on the button with the text "Select", the modal for the Media Library is closed.
After adding the audio files, the focus is moved away from the block to the editor canvas and one has to navigate to the block again to select it. (I found this confusing, but learned that other media placeholders do the same)
Navigate to the block in the editor.
The block name is announced.
Pressing the down arrow key or the right arrow key moves focus to the audio element inside the playlist block.
The element type, the track title, the artist name, and the album name are announced if this information is available. Some tracks will only have a title. The same information that is announced is printed above the audio element, available to sighted users.
Pressing the spacebar or Enter key will play the track, and pressing the spacebar or Enter again will pause the track.
Let me know if playing the track in the editor should be disabled.
Pressing the down arrow key will move the focus to the button of the first track in the tracklist. The screen reader should announce the title, the band, the length of the track (if available), and the instructions "Select to play this track".
Pressing the down arrow key again will select the next track in the tracklist, and so on, until you reach the last track.
Replacing or adding tracks
Because the tracks are inner blocks, there will be two block toolbars where you can edit tracks.
From the block toolbar of the inner block, you can replace the individual, single track.
From the block toolbar of the parent block, the playlist, you can add multiple new tracks at once.
Press Shift + Tab to open the block toolbar.
Use the arrow keys to navigate to the button named "Edit" or "Replace" respectively.
Activating the button opens a dropdown with two options: Open Media Library, and Upload.
Each option opens its respective file selection.
When the selection is complete, the screen reader announces that the media has been replaced.
(This is not accurate if you are adding a file, not replacing it, but I don't know if this can be solved?)
The focus is returned to the Edit button.
Block Sidebar
Both the playlist block and the inner track block has options in the block sidebar.
The Playlist has options both in the settings tab and styles tab.
Some of these options are visual, such as padding and margin and hiding the decorative image.
Optionally, test the first option in the settings tab, which is a toggle used to hide and show the tracklist:
With the block selected, press the tab key to move the focus to the sidebar.
Navigate to the block settings sidebar, past the Close button and the button that opens the Settings tab,
and the button that opens the Settings panel.
Find the checkbox option for "Show tracklist". Uncheck the checkbox.
Press the tab key repeatedly until you reach the "Skip to the selected block" button.
With the block selected, confirm that using the down arrow key does not move focus to the tracklist, since it has been turned off in the option.
The track has options in the settings tab.
With the block selected, press the tab key to move the focus to the sidebar.
Navigate to the block settings sidebar, past the Close button and the button that opens the Settings tab,
and the button that opens the Settings panel.
There will be three text input fields where you can update artist, album and title.
Below is an option for selecting an album cover image from the Media Library.