Move styles into specific waveform styles dropdown area - #80060
Conversation
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +114 B (0%) Total Size: 7.68 MB 📦 View Changed
|
01929b0 to
a672209
Compare
scruffian
left a comment
There was a problem hiding this comment.
This makes a lot of sense. We had it like this originally but moved it block styles to try to make the UI more generic. However as you point out this puts us into a corner if we want more layout options for the block in the future.
Let's bring it in.
| label={ __( 'Background' ) } | ||
| className="background-block-support-panel__inner-wrapper" | ||
| /> | ||
| <InspectorControls.Slot group="styles" /> |
There was a problem hiding this comment.
This change will affect all consumers using this slot group, won't it? Why is this reordering necessary?
By the way, for all block-specific settings, we use a panel labeled "Settings".
See: #67813
There was a problem hiding this comment.
This shouldn't have been there. Thanks for catching!
| const WAVEFORM_STYLE_OPTIONS = [ | ||
| { label: __( 'Bars' ), value: 'bars' }, | ||
| { label: __( 'Mirror' ), value: 'mirror' }, | ||
| { label: __( 'Line' ), value: 'line' }, | ||
| { label: __( 'Blocks' ), value: 'blocks' }, | ||
| { label: __( 'Dots' ), value: 'dots' }, | ||
| { label: __( 'Seekbar' ), value: 'seekbar' }, | ||
| ]; |
There was a problem hiding this comment.
It's not immediately clear what these terms refer to, so I think we need to add a translation context.
|
Flaky tests detected in a672209. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/29085218294
|
|
At the very least, I don't think we should reorder public API slots solely for the purpose of a specific block. |
I didn't review my AI work closely enough. Trying to move too quickly to get the Podcast Player ready. That was my fault. |
| label={ __( 'Background' ) } | ||
| className="background-block-support-panel__inner-wrapper" | ||
| /> | ||
| <InspectorControls.Slot group="styles" /> |
What?
Moves waveform style selection from general block styles pane into dedicated Waveform panel
Why?
For long-term, I think waveform selection needs to be a dropdown so we can reserve the Styles pane for traditional combination of styles, not restricted to one waveform setting.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Use of AI Tools