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 API: Support custom icons for blocks #1870

Merged
merged 3 commits into from
Jul 26, 2017
Merged

Block API: Support custom icons for blocks #1870

merged 3 commits into from
Jul 26, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Jul 12, 2017

Closes #1327

This pull request seeks to enhance the block icon property to support custom icons, not limiting block implementer to the icons include in the Dashicons set. With these changes, an icon can now be assigned as an element or component.

Testing instructions:

Verify tests pass:

npm run test-unit blocks/block-icon

There is currently no usage of custom icons, but you can apply changes locally and observe effect if you choose. For example:

diff --git a/blocks/library/image/index.js b/blocks/library/image/index.js
index 739293d9..11a120d0 100644
--- a/blocks/library/image/index.js
+++ b/blocks/library/image/index.js
@@ -23,7 +23,7 @@ const { attr, children } = query;
 registerBlockType( 'core/image', {
        title: __( 'Image' ),
 
-       icon: 'format-image',
+       icon: <Dashicon icon="format-image" />,
 
        category: 'common',

@aduth aduth added the [Feature] Block API API that allows to express the block paradigm. label Jul 12, 2017
@@ -0,0 +1,19 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Interesting; why did you choose to put it in "editor"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting; why did you choose to put it in "editor"?

It's the editor using it. My thinking is: components in blocks are used in block implementations, components in editor are specific to editor rendering (like here), and components in components are shared between contexts.

Where did you have in mind?

Copy link
Member

Choose a reason for hiding this comment

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

This is on the boundary that I'd probably consider to be block specific (since it is defined as block API). I'd consider the inserter to be editor, but the block icon to be part of blocks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved in c1b98ed

@jasmussen
Copy link
Contributor

Tested, got this:

screen shot 2017-07-26 at 09 06 20

Not sure if that means tests pass.

Interestingly, in this branch, I see an empty inserter:

screen shot 2017-07-26 at 09 06 50

The "Blocks" and "Embed" tabs show blocks as they should, and searching also works fine. It's just the recents one that's empty.

@aduth aduth force-pushed the add/icon-component branch from c1b98ed to 701e104 Compare July 26, 2017 12:17
@aduth
Copy link
Member Author

aduth commented Jul 26, 2017

Tested, got this:

Oops, this is because the component was moved from editor/ to blocks/ and I neglected to update the original testing instructions. The correct command now is:

npm run test-unit blocks/block-icon

Interestingly, in this branch, I see an empty inserter:

I believe this is due to the branch's old age. This was an issue in #1897, resolved in #1920. I've confirmed it in this branch and then fixed upon rebase.

@aduth aduth merged commit 44ad46a into master Jul 26, 2017
@aduth aduth deleted the add/icon-component branch July 26, 2017 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants