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 Styles: Show style preview when hovered or focused #34522

Merged
merged 20 commits into from
Nov 10, 2021

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Sep 3, 2021

Resolves #33933

Description

Hark! This PR attempts the following based on the explorations in #33933:

  1. Updates the styles switcher in the block inspector to use buttons.
  2. Displays previews in a dedicated Slot when hovering over styles buttons in the block inspector.
  3. Updates the style switcher in the block toolbar to a menu with check icons.
  4. Hides the default styles picker dropdown unless a non-default style has been previously saved. This is because the selecting default styles will one day be moved to Global Styles. See comments below.

Testing

Before switching to this branch, head to the Block Editor and insert a block that has block style variations. The Image Block under the TT1 theme is a satisfactory choice.

But, if you get time, also check other blocks, e.g., Image, Buttons, Social Links, Table and Quote blocks, in several base themes too. Thank you!

Select anything other than "Default" as the default style in the dropdown. For example, select "Rounded".

Save the post and checkout this branch.

Refresh the post page and select the Image Block you have just inserted.

In the Styles Panel in the block inspector, the block previews should appear when you hover or focus over the block style variation buttons in the inspector.

Nov-05-2021.09-50-44.mp4

The preview should hide itself when the mouse (or focus) leaves the corresponding button. It should be positioned in the top right corner. Try with a mouse and tabbing keys for extra points! 💯

Clicking or hitting SPACE or ENTER keys should activate the style for the selected block, and make the preview... disappear! 🪄

Ensure that the previews do not show in narrow viewports.

Screen Shot 2021-09-21 at 12 00 26 pm

Switch styles using the "Change block type or style" menu in the block toolbar.

Screen Shot 2021-09-20 at 2 05 10 pm

The styles should change as you'd expect. The selected style should be marked with a check icon.

Inserting subsequent Image Blocks should display your chosen default style.

Using the default style picker dropdown, selecting either the "Default" or "Not set" values should set the default style and cause the dropdown picker to hide itself.

Now when Inserting subsequent Image Blocks, you won't see the default style picker.

Using your console skillz add some extra text to one of the style buttons. If the text overflows, you should see an ellipsis:

Narrow width Wide width
Screen Shot 2021-10-01 at 2 48 51 pm Screen Shot 2021-10-01 at 2 47 51 pml

Be sure to check in a right-to-left language, such as Arabic:

Screen Shot 2021-11-05 at 9 53 09 am

Run the unit tests and wait for 🟢

npm run test-unit packages/block-editor/src/components/block-styles/test/utils.js

Types of changes

UI and UX changes to the style switcher in both the block inspector and toolbar.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@ramonjd ramonjd added the [Status] In Progress Tracking issues with work in progress label Sep 3, 2021
@ramonjd ramonjd self-assigned this Sep 3, 2021
@github-actions
Copy link

github-actions bot commented Sep 3, 2021

Size Change: +600 B (0%)

Total Size: 1.09 MB

Filename Size Change
build/block-editor/index.min.js 138 kB +462 B (0%)
build/block-editor/style-rtl.css 14.4 kB +64 B (0%)
build/block-editor/style.css 14.4 kB +65 B (0%)
build/edit-post/index.min.js 29.4 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 931 B
build/admin-manifest/index.min.js 1.09 kB
build/annotations/index.min.js 2.7 kB
build/api-fetch/index.min.js 2.17 kB
build/autop/index.min.js 2.08 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.2 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 58 B
build/block-library/blocks/audio/editor.css 58 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/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 470 B
build/block-library/blocks/button/editor.css 470 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 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/code/theme-rtl.css 131 B
build/block-library/blocks/code/theme.css 131 B
build/block-library/blocks/columns/editor-rtl.css 206 B
build/block-library/blocks/columns/editor.css 205 B
build/block-library/blocks/columns/style-rtl.css 503 B
build/block-library/blocks/columns/style.css 502 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.17 kB
build/block-library/blocks/cover/style.css 1.17 kB
build/block-library/blocks/embed/editor-rtl.css 488 B
build/block-library/blocks/embed/editor.css 488 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 322 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 977 B
build/block-library/blocks/gallery/editor.css 982 B
build/block-library/blocks/gallery/style-rtl.css 1.6 kB
build/block-library/blocks/gallery/style.css 1.59 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/editor-rtl.css 159 B
build/block-library/blocks/group/editor.css 159 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 114 B
build/block-library/blocks/heading/style.css 114 B
build/block-library/blocks/home-link/style-rtl.css 247 B
build/block-library/blocks/home-link/style.css 247 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 507 B
build/block-library/blocks/image/style.css 511 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 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/latest-posts/style-rtl.css 528 B
build/block-library/blocks/latest-posts/style.css 527 B
build/block-library/blocks/list/style-rtl.css 94 B
build/block-library/blocks/list/style.css 94 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 642 B
build/block-library/blocks/navigation-link/editor.css 642 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 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/style-rtl.css 195 B
build/block-library/blocks/navigation-submenu/style.css 195 B
build/block-library/blocks/navigation-submenu/view.min.js 343 B
build/block-library/blocks/navigation/editor-rtl.css 1.93 kB
build/block-library/blocks/navigation/editor.css 1.93 kB
build/block-library/blocks/navigation/style-rtl.css 1.5 kB
build/block-library/blocks/navigation/style.css 1.49 kB
build/block-library/blocks/navigation/view.min.js 2.74 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 377 B
build/block-library/blocks/page-list/editor.css 377 B
build/block-library/blocks/page-list/style-rtl.css 117 B
build/block-library/blocks/page-list/style.css 117 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 273 B
build/block-library/blocks/paragraph/style.css 273 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 347 B
build/block-library/blocks/post-comments-form/style.css 347 B
build/block-library/blocks/post-comments/style-rtl.css 492 B
build/block-library/blocks/post-comments/style.css 493 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 396 B
build/block-library/blocks/post-featured-image/editor.css 397 B
build/block-library/blocks/post-featured-image/style-rtl.css 156 B
build/block-library/blocks/post-featured-image/style.css 156 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 391 B
build/block-library/blocks/post-template/style.css 392 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 60 B
build/block-library/blocks/post-title/style.css 60 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 378 B
build/block-library/blocks/pullquote/style.css 378 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 262 B
build/block-library/blocks/query-pagination/editor.css 255 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 187 B
build/block-library/blocks/quote/style.css 187 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 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 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 245 B
build/block-library/blocks/separator/style.css 245 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 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 770 B
build/block-library/blocks/site-logo/editor.css 770 B
build/block-library/blocks/site-logo/style-rtl.css 165 B
build/block-library/blocks/site-logo/style.css 165 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 824 B
build/block-library/blocks/social-links/editor.css 823 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB
build/block-library/blocks/social-links/style.css 1.32 kB
build/block-library/blocks/spacer/editor-rtl.css 307 B
build/block-library/blocks/spacer/editor.css 307 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 146 B
build/block-library/blocks/tag-cloud/style.css 146 B
build/block-library/blocks/template-part/editor-rtl.css 560 B
build/block-library/blocks/template-part/editor.css 559 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 815 B
build/block-library/common.css 812 B
build/block-library/editor-rtl.css 9.88 kB
build/block-library/editor.css 9.88 kB
build/block-library/index.min.js 161 kB
build/block-library/reset-rtl.css 474 B
build/block-library/reset.css 474 B
build/block-library/style-rtl.css 10.3 kB
build/block-library/style.css 10.3 kB
build/block-library/theme-rtl.css 668 B
build/block-library/theme.css 673 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/blocks/index.min.js 46 kB
build/components/index.min.js 213 kB
build/components/style-rtl.css 15.3 kB
build/components/style.css 15.3 kB
build/compose/index.min.js 10.9 kB
build/core-data/index.min.js 12.8 kB
build/customize-widgets/index.min.js 11.2 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 614 B
build/data/index.min.js 7.16 kB
build/date/index.min.js 31.5 kB
build/deprecated/index.min.js 428 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.44 kB
build/edit-navigation/index.min.js 15.9 kB
build/edit-navigation/style-rtl.css 3.76 kB
build/edit-navigation/style.css 3.76 kB
build/edit-post/classic-rtl.css 492 B
build/edit-post/classic.css 494 B
build/edit-post/style-rtl.css 7.12 kB
build/edit-post/style.css 7.12 kB
build/edit-site/index.min.js 28 kB
build/edit-site/style-rtl.css 5.32 kB
build/edit-site/style.css 5.32 kB
build/edit-widgets/index.min.js 16.4 kB
build/edit-widgets/style-rtl.css 4.17 kB
build/edit-widgets/style.css 4.18 kB
build/editor/index.min.js 37.5 kB
build/editor/style-rtl.css 3.78 kB
build/editor/style.css 3.77 kB
build/element/index.min.js 3.21 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 6.49 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 424 B
build/i18n/index.min.js 3.6 kB
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.72 kB
build/keycodes/index.min.js 1.3 kB
build/list-reusable-blocks/index.min.js 1.85 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.91 kB
build/notices/index.min.js 845 B
build/nux/index.min.js 2.03 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.83 kB
build/primitives/index.min.js 921 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/redux-routine/index.min.js 2.63 kB
build/reusable-blocks/index.min.js 2.19 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 10.8 kB
build/server-side-render/index.min.js 1.52 kB
build/shortcode/index.min.js 1.48 kB
build/token-list/index.min.js 562 B
build/url/index.min.js 1.82 kB
build/viewport/index.min.js 1.02 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 7.11 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@mtias mtias added the Needs Design Feedback Needs general design feedback. label Sep 8, 2021
@mtias
Copy link
Member

mtias commented Sep 8, 2021

I think we should skip showing the preview on mobile for a first pass. We can think further about how best to represent it there in a follow up.

@mtias mtias added the [Feature] Theme Style Variations Related to style variations provided by block themes label Sep 8, 2021
@pablohoneyhoney
Copy link

This is what I see

styles

Which surfaces a few questions and notes:

  • Resting state could have more coherent greys –lighter on border (Grey 400, perhaps), dark on text.
  • No need for thicker border on selected state if the resting/active different is prominent with color?
  • The active/resting paradigm doesn’t work well in hover animation (see recording) as elements like the text jumps.
  • The hover/preview animation is not natural (see recording) when I try to preview both. It has a transition (delays, scaling up) that we could avoid and refine, and mimic more the other previews (like Inserter).

Screen Shot 2021-09-08 at 12 03 42 PM

Also default styles dropdown is a legacy flow we should likely update to an ellipsis, as @mtias noted here. Connected to #34574

cc @shaunandrews

@ramonjd
Copy link
Member Author

ramonjd commented Sep 13, 2021

Thanks for testing, folks!

The active/resting paradigm doesn’t work well in hover animation (see recording) as elements like the text jumps.
The hover/preview animation is not natural (see recording) when I try to preview both. It has a transition (delays, scaling up) that we could avoid and refine, and mimic more the other previews (like Inserter).

The animation is baked into the PopOver component. You're right, it's not ideal. I wasn't sure about it either, but I used it for the first pass.

I suppose we could extend PopOver to make the animation optional, or we could try adding a dedicated slot fill in the layout for sidebar previews and other flyouts.

I did notice that there were similar jumpy effects in the block inserter at times, but not at all so dramatic.

I'll keep playing.

I think we should skip showing the preview on mobile for a first pass. We can think further about how best to represent it there in a follow up.

Great! Thanks for the confirmation.

@mtias
Copy link
Member

mtias commented Sep 13, 2021

I suppose we could extend PopOver to make the animation options, or we could try adding a dedicated slot fill in the layout for sidebar previews and other flyouts.

We need to redo the animations there in general as part of the adoption of framer motion. It's fine to leave that separately.

@ramonjd ramonjd force-pushed the update/block-styles-preview-panel branch from 33503ea to 43897bd Compare September 14, 2021 05:54
@ramonjd
Copy link
Member Author

ramonjd commented Sep 14, 2021

I'll need to spend some time investigating positioning too :)

Screen Shot 2021-09-14 at 3 53 56 pm

We need to redo the animations there in general as part of the adoption of framer motion. It's fine to leave that separately.

Great to know, thank you.

There's also the experimental Flyover component. It's experimental however.

To compare performance, I might play around with adding a custom slot for these previews in the layout.

@shaunandrews
Copy link
Contributor

shaunandrews commented Sep 14, 2021

I think we could probably just ditch the animation in this first pass. I'll see if I can workup a prototype to get a feel for how we could (or should?) use animation for this preview. Its worth noting that the design I worked up (https://shaunandrews.com/2021/08/thinking-through-switching-block-styles/) is largely based on the existing previews of blocks when hovered from the inserter sidebar; There's no animation there, and I appreciate the simplicity and speed.

Block-Style-Previews-on-Hover

I think the buttons should be taller. In my designs I made them 42px tall.

I think the hover state for unselected styles should use a 1px, blue border with blue text. For the current style (which should be indicated with a 2px, black border) shouldn't change when hovered.

I expect that the buttons would use the standard "blue ring" when keyboard-focused — right now there doesn't seem to be any keyboard focus. Similarly, I expect the preview to appear when the button is focused.

Its not clear to me when testing the PR, but I think its important that the preview is in a fixed position regardless of which style is currently being hovered or focused. Otherwise, I feel like its too much eye movement to try and "find" the preview when it appears.

We should replace the "Default Style" label and dropdown with a simple button-link at the top of the section.

image

This "Make Default" would only appear if you change the style to something other than the current default. There's likely some more nuance and detail I'm missing here, so I might workup a more comprehensive Figma prototype if there's confusion.

For the Block dropdown, I think we'll need to make sure the buttons align with the rest of the menu. I'd also be interested in moving the styles to the top of the menu.

image

@mtias
Copy link
Member

mtias commented Sep 15, 2021

Do you think in the dropdown they might make sense as regular menu item? It might be better for keyboard nav as well.

@shaunandrews
Copy link
Contributor

Maybe something like this for the dropdown menu?

image

@mtias
Copy link
Member

mtias commented Sep 15, 2021

Yep, that's what I had in mind, I think it works better, though maybe with the ticks on the right like in other places?

image

@ramonjd
Copy link
Member Author

ramonjd commented Sep 16, 2021

Thanks for the feedback again everyone!

I had some time to play around with this today.

@shaunandrews What do you think about the following (yes it's rough), but my goal was to communicate:

  • which style is marked as default
  • the target style of the mark as default action

Sep-16-2021 15-57-24

For some reason the Button isn't aligning the icon to the right, not that it matters much for the demo 😄

Or perhaps, less offensively

less-offensive

Just to note that we're okay with overriding the default select dropdown for the defaultStylePicker supports flag?

I'll carry on with the transform drop down suggestions too.

🍺

@ramonjd
Copy link
Member Author

ramonjd commented Sep 16, 2021

Yep, that's what I had in mind, I think it works better, though maybe with the ticks on the right like in other places?

👍

This is what I have got so far:

pintxos

@ramonjd ramonjd force-pushed the update/block-styles-preview-panel branch from 43897bd to 976257f Compare September 16, 2021 12:23
@shaunandrews
Copy link
Contributor

I definitely tried with the check on the right, but thought it felt unbalanced; It breaks up the alignment of the text labels from the block list above it.

image

That said, I don't have a strong opinion; Its probably better to keep with the consistency of having the check on the right, like in other menus.

--

I'm not sure we really need an icon to indicate the default style. I did explore some ideas, but it never really went anywhere:

image

Instead, I think it might make sense to always show the default style first. Then, if you change the default the button would move to the top of the list:

Make Default Style

--

I have a slight preference for having the "Make default" button at the top of the section, as it avoids changing the height of the accordion. If that's difficult (or impossible) than I could see it living at the bottom, under the list of styles:

image

--

overriding the default select dropdown for the defaultStylePicker

I'm not sure, but I'd expect the dropdown to be deprecated in favor of the new button outlined above.

@ramonjd
Copy link
Member Author

ramonjd commented Sep 17, 2021

Thanks again for the feedback and the thought you put into the icons, even if we're not showing any.

I have a slight preference for having the "Make default" button at the top of the section, as it avoids changing the height of the accordion. If that's difficult (or impossible) than I could see it living at the bottom, under the list of styles:

Well, it is easier to show it at the bottom, only because the Panel component's title element is there, and we might have the float it. :)

Instead, I think it might make sense to always show the default style first. Then, if you change the default the button would move to the top of the list:

You got it!

Here's what we have in the latest commit 8429cdd

default-make-button

Working on the previews next... more to come.

@ramonjd ramonjd removed the [Status] In Progress Tracking issues with work in progress label Sep 20, 2021
@ramonjd ramonjd marked this pull request as ready for review September 20, 2021 04:43
@mtias
Copy link
Member

mtias commented Sep 20, 2021

We should place the "make default" inside the ellipsis menu (which replace the collapse arrow) and include a reset within.

@mtias
Copy link
Member

mtias commented Sep 20, 2021

That said, I don't have a strong opinion; Its probably better to keep with the consistency of having the check on the right, like in other menus.

Right, I think the second version is alright. We have some older mockups that had the entire styles panel in a flyout menu, which would reduce any visual conflicts with the transforms area. We could follow up with that.

@skorasaurus skorasaurus mentioned this pull request Feb 3, 2022
@youknowriad
Copy link
Contributor

Hi There! I was working on some unrelated site editor refactoring and noticed this new Slot used to show block style previews. So I was wondering why it was needed in the first place, my guess is that it's needed because we want to position the previous at the top left of the sidebar, am I right?

I was wondering if a treatment like we do for color popovers would work better and make the code simpler. This is to show the preview as a popover on the left of the "Styles" component. This would avoid the need for a new Slot which creates more boilerplate for block editors.

@ramonjd
Copy link
Member Author

ramonjd commented Oct 10, 2022

Thanks for swinging back to this one.

So I was wondering why it was needed in the first place, my guess is that it's needed because we want to position the previous at the top left of the sidebar, am I right?

I have deleted the memory of this PR from my brain 😄 but that sounds about right.

Something I do remember before resorting to slots was adding the component next to <BlockStyles /> in packages/block-editor/src/components/block-inspector/index.js, but I was struggling with making it float property outside the confines of the sidebar, so basically above the interface-skeleton__content (the editor canvas).

Slots were the most convenient and UX-friendly goto at the time.

I was wondering if a treatment like we do for color popovers would work better and make the code simpler. This is to show the preview as a popover on the left of the "Styles" component. This would avoid the need for a new Slot which creates more boilerplate for block editors.

I think the new color popover came a bit after this PR (?), but if you think there's an improvement to be made by removing <BlockStyles.Slot scope="core/block-inspector" /> and combining the two, by all means.

@youknowriad
Copy link
Contributor

I think the new color popover came a bit after this PR (?), but if you think there's an improvement to be made by removing <BlockStyles.Slot scope="core/block-inspector" /> and combining the two, by all means.

Yeah, I do think there's something we can do here but only if we're ok with the popover showing up at the left of the "styles" component but not at the top left, it would be aligned with the "styles" component.

@ramonjd
Copy link
Member Author

ramonjd commented Oct 10, 2022

if we're ok with the popover showing up at the left of the "styles" component but not at the top left, it would be aligned with the "styles" component.

I'd say if it works similar to the color popover then it would be totally acceptable. You can quote me. 😄 Thank you!!

@mtias
Copy link
Member

mtias commented Oct 10, 2022

@youknowriad I'd be fine with aligning by the style component if it simplifies the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Theme Style Variations Related to style variations provided by block themes [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block Styles: Show style preview when hovered or focused
10 participants