Block API: Add support for icons for block categories #10651

tyxla · 2018-10-16T14:17:49Z

Description

This PR adds support for icons for block categories. In addition to supporting Dashicons, the idea is to introduce support for external components, functions and svg icons, in order to allow for third party plugins to be flexible with the icons for the categories they register.

Since <BlockIcon /> already supported all those kinds of methods to define an icon, we're abstracting the functionality to a separate component, and reusing it here and in <BlockIcon />.

How has this been tested?

Checkout this PR and build Gutenberg.
Open the inserter in a post.
Look at the "Common" category.
Verify you can see the Dashicon to the right of the "Common" category name.
Save at least one reusable block.
Verify the reusable blocks category icon is displayed to the right of the category name.
Verify that block categories without icons are unchanged.
Verify existing block icons in the inserter are unchanged.
Verify unit tests pass:
- BlockIcon tests - npm run test-unit packages/editor/src/components/block-icon/
- Icon tests - npm run test-unit packages/components/src/icon/
- All unit tests - npm run test-unit
- All tests - npm test

Screenshots

Inserter with an icon on the "Common" block category:
~~https://cldup.com/Eupr3BAnyc.png~~

"Reusable" category in the inserter with one or more existing reusable blocks:

Inserter with a custom SVG icon on a custom category:
~~https://cldup.com/etFPNTYIK3.png~~

(see Automattic/wp-calypso#27878 for testing it)

Types of changes

Introduced a new <RawIcon /> component and exposed it.
Added tests and documentation for <RawIcon />.
Updated <BlockIcon /> to use <RawIcon />.
Updated the PanelBody to use <RawIcon /> for block categories.
Now passing category icon from inserter menu to <PanelBody />.
Now registering the default categories with null icons (no icons).
Now displaying the panel body icons on the right side of the panel body title.
Moved and updated some tests.
TO BE DELETED BEFORE MERGE - added a dashicon to the "Common" block category (204582b).
Renamed <RawIcon /> to <Icon />.
Made rendering of Dashicon default to a size of 20x20, while keeping 24x24 for the rest of the icon types.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

Notes

Since we've added a dashicon to the "Common" block category (204582b) this needs to be deleted before merging.

mtias · 2018-10-16T14:22:22Z

Thanks for working on this. Can we shift the icons to be rendered on the right so that text always aligns visually on the left?

ZebulanStanphill · 2018-10-16T16:48:27Z

What if the text was always rendered with space on the left for an icon, so both the icons and the text would always line up?

folletto · 2018-10-16T17:30:23Z

For reference, this is an old mockup I prepared to avoid the issue with icon alignment:

The icon on the right avoids having different indentations for the various categories, and at the same time optimizes space. :)

mtias · 2018-10-16T17:35:44Z

Note: we should also update:

tyxla · 2018-10-17T08:42:38Z

Good call, folks - I like the suggestion for the icon on the right side. Updated the PR, here are some screenshots:

Inserter with an icon on the "Common" block category:

"Reusable" category in the inserter with one or more existing reusable blocks:

Inserter with a custom SVG icon on a custom category:

This is ready for another look.

youknowriad · 2018-10-23T12:46:21Z

packages/components/src/raw-icon/index.js

+import { cloneElement, createElement, Component, isValidElement } from '@wordpress/element';
+import { Dashicon, SVG } from '../';
+
+function RawIcon( { icon = null, size = 24, className } ) {


I suspect the default size property here might have some unexpected side effects on the different types of block icons and the different places where we render them. Mind a sanity check here @jasmussen or @chrisvanpatten as you know more about all the specificities.

Interesting timing, I'm actually trying to fix a regression in #10938 (comment) related to this.

24 should be default, but it's okay if an SVG needs to override that and show a 20x20 icon.

If 24 is not default, then that's a bug, as is discussed in that link.

It looks like we were explicitly showing Dashicons as 20x20 while this PR makes them 24x24 ?

@youknowriad Yeah dashicons should show at 20x20 for weird sizing reasons, but material icons should show at 24x24

youknowriad · 2018-10-23T12:47:06Z

packages/components/src/raw-icon/index.js

@@ -0,0 +1,35 @@
+/**


👍 Nice little component. Why not just Icon, Thoughts on the name @aduth @mcsf

My expectations from the name and description don't really follow into the implementation. If it's "without styles", why do we apply a default width? And an unstyled variation seems more appropriate as a prop of a default component type, e.g. <Icon isUnstyled />.

Well, I consider the width an attribute, and not a style 🤷‍♂️

And regarding the component name - Icon felt a little too broad, and I'd probably expect an icon component to be more flexible in terms of how the icon could look. So I'd instead expect to have an Icon component that could use the RawIcon, but add on top of it.

What do y'all think?

I'd go for either Icon or, considering:

Icon felt a little too broad, and I'd probably expect an icon component to be more flexible in terms of how the icon could look

then I might go with UnstyledIcon.

I'm good with changing to UnstyledIcon, @aduth @youknowriad does that make sense to you?

I'd probably expect an icon component to be more flexible in terms of how the icon could look.

Can you elaborate on what you mean by this? I'm not sure I follow, and am having a hard time understanding what specifically we're implying in these multiple layers of icon components.

I personally still have a small preference for just Icon, if we need to make a distinction between styled and unstyled later, we can add it as a prop.

I've been thinking and started gathering a list of what <Icon /> would provide more than <UnstyledIcon />, but I didn't get too far with it.

I guess I'm good with renaming this one to <Icon />, it seems @mcsf and @youknowriad are good with <Icon />, @aduth does that sound well to you?

Works for me. 👍

Cool, thanks 👍

Renamed <RawIcon /> to <Icon /> in 3486d0b

I'd appreciate another look when y'all get a chance.

youknowriad · 2018-10-23T12:49:16Z

packages/components/src/panel/body.js

 							{ title }
+							{ icon && <RawIcon icon={ icon } className="components-panel__icon" size={ 20 } /> }


What's the impact of this change on other usage of PanelBody with icons? Are we ok with all panel body icons being moved to the right or should it be specific to the inserter?

Last time I checked, the only usages were in the inserter, and this PR has verified them all.

Whether we're planning to use the PanelBody for something else, and icons would make sense on the right in these cases - perhaps @mtias @jasmussen would know better.

It looks like it was introduced explicitly for this particular usage (Inserter), so yes, I think I'm fine with the change.

Yes, it was added for the reusable block feature.

youknowriad

At the moment custom categories are provided using a backend filter IIRC cc @gziolo, how do we provide SVGs from the backend? Do we have the corresponding JavaScript API to add these categories?

youknowriad · 2018-10-26T13:32:13Z

packages/block-library/src/embed/test/__snapshots__/index.js.snap

@@ -14,10 +14,10 @@ exports[`core/embed block edit matches snapshot 1`] = `
        aria-hidden="true"
        class="dashicon dashicons-embed-generic"
        focusable="false"
-        height="20"
+        height="24"


I think this is an issue actually. As outlined here #10651 (comment) Dashicons should be 20x20 by default.

Oh, that's a fair point. I'll reflect this in the component then, and will add some tests too.

Updated in c7959cf, thanks for the suggestion!

youknowriad

I didn't test but looks good to me code-wise :)

youknowriad · 2018-10-27T11:44:07Z

Tested and works well.

tyxla · 2018-10-29T09:27:23Z

@youknowriad thanks for merging this one!

However, it seems we forgot to remove a commit from the branch before merging. So I've filed a PR for this: #11191.

designsimply · 2018-10-31T00:30:05Z

I tested with WordPress 4.9.8 and Gutenberg 4.2.0-rc.1 and didn't see a Dashicon to the right of the "Common" category name. Is this because it's something that needs to be opted into using custom code? Note: I followed the testing steps quite literally and it's possible I'm missing something.

tyxla · 2018-10-31T09:22:25Z

@designsimply - it's expected that you don't see it. Adding the Common category icon was for testing purposes for this PR, and even though we merged it with this PR, we reverted it in #11191.

designsimply · 2018-10-31T16:26:15Z

Aha. I see. Thank you for the clarification!

Enchiridion · 2018-11-08T00:28:18Z

What if the text was always rendered with space on the left for an icon, so both the icons and the text would always line up?

I also would prefer the icon to be on the left. Then assign an icon to each of the default categories, just like the WP admin sidebar. Having them on the right is inconsistent with the rest of WP/Gutenberg UI, and doesn't make finding the category you want any easier since you can't quickly scan a single column.

tyxla mentioned this pull request Oct 16, 2018

Gutenberg: Add icon to Jetpack block category Automattic/wp-calypso#27878

Merged

mtias added [Type] Enhancement A suggestion for improvement. [Feature] Inserter The main way to insert blocks using the + button in the editing interface labels Oct 16, 2018

mtias added this to the 4.2 milestone Oct 22, 2018

youknowriad reviewed Oct 23, 2018

View reviewed changes

tyxla added 14 commits October 26, 2018 15:55

Components: Introduce RawIcon component

3cc078e

Components: Add tests for RawIcon

6d72e8a

Components: Add docs for RawIcon

7f7c859

Components: Expose RawIcon component

0280835

Editor: Update BlockIcon to use RawIcon for rendering

bd60c2f

Editor: Update Inserter menu to use RawIcon for block categories

63cc934

Editor: Update PanelBody to use RawIcon

748a84e

Register default block categories with null icons

48c0519

DELETE: Test adding a dashicon to default categories

fa4cd68

Use default icon value in return

2535794

Update BlockIcon tests

d54d1cd

Update embed snapshots

8f2a1fb

Document the size prop

a8f0b1c

Move panel icon after the title

d947c68

tyxla force-pushed the try/block-categories-icon-support branch from 8e44061 to d947c68 Compare October 26, 2018 13:17

Rename RawIcon to Icon

3486d0b

youknowriad reviewed Oct 26, 2018

View reviewed changes

Default Dashicons to 20x20, the rest to 24x24

c7959cf

youknowriad approved these changes Oct 26, 2018

View reviewed changes

Update the docs manifest

729fa98

youknowriad merged commit 87732a2 into WordPress:master Oct 27, 2018

tyxla deleted the try/block-categories-icon-support branch October 29, 2018 09:25

tyxla mentioned this pull request Oct 29, 2018

Revert addition of icon to Common block category #11191

Merged

4 tasks

ajitbohra mentioned this pull request Oct 29, 2018

Docs: custom block category add icon info #11213

Merged

iandunn mentioned this pull request Mar 1, 2019

Block category icons are distracting #14180

Closed

sirbrillig mentioned this pull request Jan 4, 2022

Improve @wordpress/components types for Animate, DropdownMenu, ResizableBox, and ToolbarButton DefinitelyTyped/DefinitelyTyped#57965

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block API: Add support for icons for block categories #10651

Block API: Add support for icons for block categories #10651

tyxla commented Oct 16, 2018 •

edited

Loading

mtias commented Oct 16, 2018

ZebulanStanphill commented Oct 16, 2018

folletto commented Oct 16, 2018

mtias commented Oct 16, 2018

tyxla commented Oct 17, 2018

youknowriad Oct 23, 2018

jasmussen Oct 23, 2018

youknowriad Oct 23, 2018

chrisvanpatten Oct 24, 2018

youknowriad Oct 23, 2018

aduth Oct 23, 2018 •

edited

Loading

tyxla Oct 23, 2018

mcsf Oct 24, 2018

tyxla Oct 25, 2018

aduth Oct 25, 2018

youknowriad Oct 26, 2018

tyxla Oct 26, 2018

aduth Oct 26, 2018

tyxla Oct 26, 2018

youknowriad Oct 23, 2018

tyxla Oct 23, 2018

youknowriad Oct 23, 2018

mtias Oct 23, 2018

youknowriad left a comment

youknowriad Oct 26, 2018

tyxla Oct 26, 2018

tyxla Oct 26, 2018

youknowriad left a comment

youknowriad commented Oct 27, 2018

tyxla commented Oct 29, 2018

designsimply commented Oct 31, 2018

tyxla commented Oct 31, 2018

designsimply commented Oct 31, 2018

Enchiridion commented Nov 8, 2018

		{ title }
		{ icon && <RawIcon icon={ icon } className="components-panel__icon" size={ 20 } /> }

Block API: Add support for icons for block categories #10651

Block API: Add support for icons for block categories #10651

Conversation

tyxla commented Oct 16, 2018 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

Notes

mtias commented Oct 16, 2018

ZebulanStanphill commented Oct 16, 2018

folletto commented Oct 16, 2018

mtias commented Oct 16, 2018

tyxla commented Oct 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth Oct 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad commented Oct 27, 2018

tyxla commented Oct 29, 2018

designsimply commented Oct 31, 2018

tyxla commented Oct 31, 2018

designsimply commented Oct 31, 2018

Enchiridion commented Nov 8, 2018

tyxla commented Oct 16, 2018 •

edited

Loading

aduth Oct 23, 2018 •

edited

Loading