Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audio Block: Refactor setting panel to use ToolsPanel #69324

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

shimotmk
Copy link
Contributor

What?

Part of #67813

Make the settings panel more consistent with other blocks

Testing Instructions

  1. Open a post or page.
  2. Insert a audio block
  3. Open the settings panel
  4. You can see that you can set it up just like before.

Testing Instructions for Keyboard

Screenshots or screencast

Before After
audio-before audio-after
audio-block.mp4

refactor-audio-block-control-panel
Copy link

github-actions bot commented Feb 26, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: shimotmk <shimotomoki@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@shimotmk shimotmk changed the title Audio Block: Refactor setting panel Audio Block: Refactor setting panel to use ToolsPanel Feb 26, 2025
@t-hamano t-hamano added [Type] Code Quality Issues or PRs that relate to code quality [Block] Audio Affects the Audio Block labels Feb 27, 2025
Copy link
Contributor

@carolinan carolinan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resetting the options to false and undefined looks correct to me.

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Although not directly related to the purpose of this PR, can you fix the following browser console error? This error occurs the first time you insert a block, apply media, and enable "Autoplay" or "Loop":

diff --git a/packages/block-library/src/audio/edit.js b/packages/block-library/src/audio/edit.js
index 47e73b4ea7..5b2737873f 100644
--- a/packages/block-library/src/audio/edit.js
+++ b/packages/block-library/src/audio/edit.js
@@ -190,7 +190,7 @@ function AudioEdit( {
                                                        __nextHasNoMarginBottom
                                                        label={ __( 'Autoplay' ) }
                                                        onChange={ toggleAttribute( 'autoplay' ) }
-                                                       checked={ autoplay }
+                                                       checked={ !! autoplay }
                                                        help={ getAutoplayHelp }
                                                />
                                        </ToolsPanelItem>
@@ -208,7 +208,7 @@ function AudioEdit( {
                                                        __nextHasNoMarginBottom
                                                        label={ __( 'Loop' ) }
                                                        onChange={ toggleAttribute( 'loop' ) }
-                                                       checked={ loop }
+                                                       checked={ !! loop }
                                                />
                                        </ToolsPanelItem>
                                        <ToolsPanelItem
9e253326d8c23852e0da379bcc1fc0e1.mp4

@shimotmk
Copy link
Contributor Author

shimotmk commented Mar 3, 2025

Thanks for the review
I fixed it

@t-hamano
Copy link
Contributor

t-hamano commented Mar 3, 2025

Note: The currently failing E2E tests are not caused by this PR. See https://core.trac.wordpress.org/ticket/56481#comment:45

Once this issue is resolved, let's run the E2E tests again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Audio Affects the Audio Block [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants