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

Block Variations Transforms: Display as icon buttons if each block variation has a unique icon #40036

Conversation

andrewserong
Copy link
Contributor

What?

Following on from a comment #39710 (comment) by @jasmussen, this PR looks at swapping out the Transform to Variation dropdown list in the block inspector for a list of buttons (similar to orientation and justification controls).

Because it's possible that there are blocks out in the wild that utilise variation transformations where each variation does not have a unique icon, I've preserved the dropdown list as a fallback in the case that each variation does not have a unique icon.

Why?

The Transform to Variation dropdown doesn't immediately signal to a user what they can do with each variation, hopefully switching to icons makes it a little clearer.

How?

  • Check that each variation has a unique icon, and if so, render a set of buttons for each variation.
  • Remove internal state for the __experimentalBlockVariationTransforms component and switch to using the getActiveBlockVariation selector, as the current approach didn't appear to play well with the Group block's variations which involve an isActive check that is more tolerant of empty / default values than the existing checks (this fixes a bug where when a Group block was initially inserted, the Group button wasn't highlighted)

Testing Instructions

  1. Insert a Group block, and check that each of the transform buttons works correctly
  2. From the Row block, switch the orientation to be vertical — this should update the block to be the Stack variation, and the Stack variation should be automatically selected in the list of buttons
  3. Also test with another block that uses transforms, e.g. add a Next Post block and check that you can switch between that and the Previous post variation.
  4. For thoroughness, in the Group block's variations.js file (this line), try commenting out the group icon. Reload the editor and check that the Transform to Variation dropdown menu works as before.

Screenshots or screencast

Screengrab of usage (note that switching orientation immediately updates the selected variation):

block-variations-buttons

Screenshots:

Group block Next post block
image image

@github-actions
Copy link

github-actions bot commented Apr 5, 2022

Size Change: +1.03 kB (0%)

Total Size: 1.22 MB

Filename Size Change
build/block-editor/index.min.js 149 kB +174 B (0%)
build/block-editor/style-rtl.css 15.4 kB +36 B (0%)
build/block-editor/style.css 15.4 kB +38 B (0%)
build/block-library/blocks/group/editor-rtl.css 333 B +174 B (+109%) 🆘
build/block-library/blocks/group/editor.css 333 B +174 B (+109%) 🆘
build/block-library/editor-rtl.css 10.1 kB +108 B (+1%)
build/block-library/editor.css 10.1 kB +108 B (+1%)
build/block-library/index.min.js 174 kB +143 B (0%)
build/components/index.min.js 223 kB +8 B (0%)
build/edit-site/index.min.js 46.7 kB +34 B (0%)
build/edit-site/style-rtl.css 7.75 kB +15 B (0%)
build/edit-site/style.css 7.73 kB +15 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 993 B
build/admin-manifest/index.min.js 1.24 kB
build/annotations/index.min.js 2.77 kB
build/api-fetch/index.min.js 2.27 kB
build/autop/index.min.js 2.15 kB
build/blob/index.min.js 487 B
build/block-directory/index.min.js 6.49 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 59 B
build/block-library/blocks/avatar/style.css 59 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 445 B
build/block-library/blocks/button/editor.css 445 B
build/block-library/blocks/button/style-rtl.css 560 B
build/block-library/blocks/button/style.css 560 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 103 B
build/block-library/blocks/code/style.css 103 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-template/style-rtl.css 127 B
build/block-library/blocks/comment-template/style.css 127 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-query-loop/editor-rtl.css 95 B
build/block-library/blocks/comments-query-loop/editor.css 95 B
build/block-library/blocks/cover/editor-rtl.css 546 B
build/block-library/blocks/cover/editor.css 547 B
build/block-library/blocks/cover/style-rtl.css 1.56 kB
build/block-library/blocks/cover/style.css 1.56 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 353 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 961 B
build/block-library/blocks/gallery/editor.css 964 B
build/block-library/blocks/gallery/style-rtl.css 1.51 kB
build/block-library/blocks/gallery/style.css 1.51 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 731 B
build/block-library/blocks/image/editor.css 730 B
build/block-library/blocks/image/style-rtl.css 529 B
build/block-library/blocks/image/style.css 535 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 199 B
build/block-library/blocks/latest-posts/editor.css 198 B
build/block-library/blocks/latest-posts/style-rtl.css 447 B
build/block-library/blocks/latest-posts/style.css 446 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 708 B
build/block-library/blocks/navigation-link/editor.css 706 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/view.min.js 375 B
build/block-library/blocks/navigation/editor-rtl.css 2.03 kB
build/block-library/blocks/navigation/editor.css 2.04 kB
build/block-library/blocks/navigation/style-rtl.css 1.93 kB
build/block-library/blocks/navigation/style.css 1.92 kB
build/block-library/blocks/navigation/view.min.js 2.85 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 260 B
build/block-library/blocks/paragraph/style.css 260 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/style-rtl.css 446 B
build/block-library/blocks/post-comments-form/style.css 446 B
build/block-library/blocks/post-comments/style-rtl.css 521 B
build/block-library/blocks/post-comments/style.css 521 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 721 B
build/block-library/blocks/post-featured-image/editor.css 721 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 323 B
build/block-library/blocks/post-template/style.css 323 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 370 B
build/block-library/blocks/pullquote/style.css 370 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 397 B
build/block-library/blocks/search/style.css 398 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 140 B
build/block-library/blocks/separator/editor.css 140 B
build/block-library/blocks/separator/style-rtl.css 233 B
build/block-library/blocks/separator/style.css 233 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 759 B
build/block-library/blocks/site-logo/editor.css 759 B
build/block-library/blocks/site-logo/style-rtl.css 181 B
build/block-library/blocks/site-logo/style.css 181 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.37 kB
build/block-library/blocks/social-links/style.css 1.36 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 226 B
build/block-library/blocks/tag-cloud/style.css 227 B
build/block-library/blocks/template-part/editor-rtl.css 149 B
build/block-library/blocks/template-part/editor.css 149 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 934 B
build/block-library/common.css 932 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 11.3 kB
build/block-library/style.css 11.3 kB
build/block-library/theme-rtl.css 689 B
build/block-library/theme.css 694 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/blocks/index.min.js 46.9 kB
build/components/style-rtl.css 14.9 kB
build/components/style.css 14.9 kB
build/compose/index.min.js 11.2 kB
build/core-data/index.min.js 14.5 kB
build/customize-widgets/index.min.js 11 kB
build/customize-widgets/style-rtl.css 1.39 kB
build/customize-widgets/style.css 1.39 kB
build/data-controls/index.min.js 663 B
build/data/index.min.js 8.64 kB
build/date/index.min.js 32 kB
build/deprecated/index.min.js 518 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.58 kB
build/edit-navigation/index.min.js 15.8 kB
build/edit-navigation/style-rtl.css 4.04 kB
build/edit-navigation/style.css 4.05 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-post/index.min.js 29.6 kB
build/edit-post/style-rtl.css 7.06 kB
build/edit-post/style.css 7.06 kB
build/edit-widgets/index.min.js 16.3 kB
build/edit-widgets/style-rtl.css 4.4 kB
build/edit-widgets/style.css 4.39 kB
build/editor/index.min.js 38.4 kB
build/editor/style-rtl.css 3.71 kB
build/editor/style.css 3.71 kB
build/element/index.min.js 4.29 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 6.62 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.66 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.79 kB
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.83 kB
build/keycodes/index.min.js 1.41 kB
build/list-reusable-blocks/index.min.js 1.75 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.94 kB
build/notices/index.min.js 957 B
build/nux/index.min.js 2.12 kB
build/nux/style-rtl.css 751 B
build/nux/style.css 749 B
build/plugins/index.min.js 1.98 kB
build/preferences/index.min.js 1.2 kB
build/primitives/index.min.js 949 B
build/priority-queue/index.min.js 611 B
build/react-i18n/index.min.js 704 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.69 kB
build/reusable-blocks/index.min.js 2.24 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 11.2 kB
build/server-side-render/index.min.js 1.61 kB
build/shortcode/index.min.js 1.52 kB
build/token-list/index.min.js 668 B
build/url/index.min.js 1.99 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 280 B
build/widgets/index.min.js 7.21 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.07 kB

compressed-size-action

@jasmussen
Copy link
Contributor

Thank you, the inspector controls work excellently well and look great:
transforms

Designwise, that's cool 👍 👍

I was a bit confused in testing this as you can probably glean from the GIF above: I select two paragraphs and transform them to a row, but they still appear the same in the canvas. This is very probably an issue in trunk, but it surprised me a bit. The color itemgroup in the multiselect toolbar is also broken — also very likely a trunk issue. Do you know if any of these issues are tracked or just me misunderstanding the row block behavior?

In any case, assuming those are trunk issues, this one just needs a code review to land! Thank you!

@andrewserong
Copy link
Contributor Author

andrewserong commented Apr 5, 2022

Thanks for testing @jasmussen!

I was a bit confused in testing this as you can probably glean from the GIF above: I select two paragraphs and transform them to a row, but they still appear the same in the canvas.

In this case, I think that's expected — the paragraphs selected there have content that's large enough that with the default Allow to wrap to multiple lines they get rendered in a way that looks like it's stacked. It's a bit clearer if we toggle the control in the sidebar:

row-toggle-wrap-to-multiple-lines

There's probably a few different ways we could potentially handle that in the future, but I think it might touch on min / max width ideas, so I can't quite think of a good existing issue for it 🤔

The color itemgroup in the multiselect toolbar is also broken — also very likely a trunk issue.

Yes, I can also replicate this on trunk, assuming it's what you're seeing (the controls work, but the spacing is broken):

image

From a quick skim I couldn't find an existing issue for it, I've opened one here: #40040 — I'm happy to look into that later this week if no-one beats me to it (I'm wrapping up for the day now)


Since you're happy with this PR from a design perspective, I'll add this to the WP 6.0 board to see if we can get it in in time. Thanks for testing!

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Quick green check from design, but would still encourage a confidence boost in the code! Thanks again.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @andrewserong!

@mtias
Copy link
Member

mtias commented Apr 5, 2022

@andrewserong @jasmussen yes, the default looks incorrect for most uses cases here. See #39651 (comment)

The expectation is things are rendered in a row, otherwise there's no visible change between stack and row, which is super confusing.

@mtias
Copy link
Member

mtias commented Apr 5, 2022

I like the heuristics of the icons, it could work very elegantly! Curious if it breaks apart for social icons and embeds.

@ramonjd
Copy link
Member

ramonjd commented Apr 5, 2022

It's working and looking as described in the editor and frontend.

✅ Insert a Group block, and check that each of the transform buttons works correctly
✅ From the Row block, switch the orientation to be vertical — this should update the block to be the Stack variation, and the Stack variation should be automatically selected in the list of buttons
✅ Also test with another block that uses transforms, e.g. add a Next Post block and check that you can switch between that and the Previous post variation.
✅ For thoroughness, in the Group block's variations.js file (this line), try commenting out the group icon. Reload the editor and check that the Transform to Variation dropdown menu works as before

Apr-06-2022.08-51-25.mp4

With the icon commented out:

Screen Shot 2022-04-06 at 8 42 11 am

Here's the difference I'm seeing between stack and row and default. How they work is better demonstrated with background colors.

2022-04-06 08 50 10

I guess one could argue that it's strange that the transform switches between stack and row when we toggle orientation - seems to make one or the other redundant at first glance.

@andrewserong
Copy link
Contributor Author

andrewserong commented Apr 6, 2022

Curious if it breaks apart for social icons and embeds.

We currently don't expose the transform to variations for social links (or for embeds), but if we do switch it on, then with this PR it looks like the following:

I was originally wondering if we'd need to add a cap to the logic in this PR (e.g. if it'd feel too overwhelming having that many icons), but I actually kinda like it! Happy to explore switching it on (or adding a cap) in a separate PR if folks think it's worthwhile (embeds currently don't all have unique icons so it mightn't be immediately possible there — it renders as a dropdown list by default — so we might need to do some icon wrangling 🤔)

@andrewserong
Copy link
Contributor Author

Thanks for the reviews, folks! 🙇 I'll merge this in once tests pass.

I guess one could argue that it's strange that the transform switches between stack and row when we toggle orientation - seems to make one or the other redundant at first glance.

I don't mind the duplication, personally, in that it highlights the explicit relationship between orientation and the block variation — so, whichever control a user updates (variation or orientation) it signals a bit more the expected behaviour. If it's too much, we could always look at hiding the orientation control in a follow-up if need be.

@ramonjd
Copy link
Member

ramonjd commented Apr 6, 2022

I don't mind the duplication, personally, in that it highlights the explicit relationship between orientation and the block variation — so, whichever control a user updates (variation or orientation) it signals a bit more the expected behaviour. If it's too much, we could always look at hiding the orientation control in a follow-up if need be.

Sounds good. Definitely not a blocker. Thanks for the explanation too!

@andrewserong andrewserong merged commit c8ba761 into trunk Apr 6, 2022
@andrewserong andrewserong deleted the update/block-variation-transforms-to-use-buttons-when-unique-icons-are-used branch April 6, 2022 00:50
@github-actions github-actions bot added this to the Gutenberg 13.0 milestone Apr 6, 2022
@andrewserong
Copy link
Contributor Author

andrewserong commented Apr 6, 2022

Tiny follow-up bug fix for the Group block variation's isActive check: #40065 (it wasn't quite inclusive enough to include the "Inherit" toggle for the group variation). (Edit: merged)

@andrewserong
Copy link
Contributor Author

I've also opened up a fix for the spacing of the color items in the multi-select inspector in #40071

@jasmussen
Copy link
Contributor

I was originally wondering if we'd need to add a cap to the logic in this PR (e.g. if it'd feel too overwhelming having that many icons), but I actually kinda like it! Happy to explore switching it on (or adding a cap) in a separate PR if folks think it's worthwhile (embeds currently don't all have unique icons so it mightn't be immediately possible there — it renders as a dropdown list by default — so we might need to do some icon wrangling 🤔)

That's indeed a curious one! It does mak it way easier to select the variation you want, which to an extent is quite useful. But it's also a lot of logos 😅

@andrewserong
Copy link
Contributor Author

But it's also a lot of logos 😅

It sure is! Definitely not an urgent one to figure out (I was mostly just curious to see how it'd look), but I suppose if we did want to use icon buttons to enable switching between variations there, we could introduce a show / hide mechanism (e.g. if greater than a certain threshold then have the buttons collapsed somehow).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block [Package] Block editor /packages/block-editor [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants