WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.5k stars 4.2k forks source link

Replace ButtonGroup usage with ToggleGroupControl #65339

Open mirka opened 1 month ago

mirka commented 1 month ago

Part of #65338

All of our remaining usage of the ButtonGroup component is visually outdated as a segmented control and often inaccessibly implemented. We should correct this by replacing them with ToggleGroupControl.

How to submit PRs

To pick up a task, write your name by the checklist item or post a comment to this issue.

Please include before/after screenshots in the PR.

hbhalodia commented 1 month ago

Hi @mirka, I want to work on the first issue mentioned in the description.

Thank You,

hbhalodia commented 1 month ago

Hi @mirka - Have raised the PR for the task 1 - https://github.com/WordPress/gutenberg/pull/65386

Thank You,

spadeshoe commented 1 month ago

@mirka I would like to work on the packages/block-editor/src/hooks/layout.js part of this issue. I am unsure of what component in the Editor LayoutTypeSwitcher is affecting so I have been unable to test that my change has not broken anything. I thought it might be the type size selector. See screenshot.

Screenshot 2024-09-17 at 2 47 30 PM
mirka commented 1 month ago

@spadeshoe Good question! I don't think this code path is actually reached in any of our core blocks. So you can force the condition like this:

diff --git a/packages/block-editor/src/hooks/layout.js b/packages/block-editor/src/hooks/layout.js
index 22d916d7b7..7a349ceb0b 100644
--- a/packages/block-editor/src/hooks/layout.js
+++ b/packages/block-editor/src/hooks/layout.js
@@ -264,7 +264,7 @@ function LayoutPanelPure( {
                        </>
                    ) }

-                   { ! inherit && allowSwitching && (
+                   { true && (
                        <LayoutTypeSwitcher
                            type={ blockLayoutType }
                            onChange={ onChangeType }

Then insert a Grid block, for example. And you'll see this:

Layout type switcher
spadeshoe commented 1 month ago

Thanks @mirka The conditional trick worked and I was able to test the update. I created the PR for the change: https://github.com/WordPress/gutenberg/pull/65498

@jeryj and I had done some digging Tuesday and discovered what you did, that this code is no longer used in core blocks. I have a branch that removes this code all together. I'm not sure what the protocol is for removing "deprecated" code, if it should be removed at all for backwards compatibility.

mirka commented 1 month ago

@spadeshoe Thanks for the PR, we'll get it reviewed shortly!

I had done some digging Tuesday and discovered what you did, that this code is no longer used in core blocks. I have a branch that removes this code all together. I'm not sure what the protocol is for removing "deprecated" code, if it should be removed at all for backwards compatibility.

Although it isn't used in any of the blocks in core, it's still a supported block API so we'll need to keep it 👍