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

Add store icon #23867

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Add store icon #23867

merged 1 commit into from
Aug 12, 2020

Conversation

jameskoster
Copy link
Contributor

Closes #23835

Adds a new "Store" icon to the Icons package, based on the existing icon for "Home":

Figma here.

const store = (
<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
<Path
fillRule="evenodd"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming both this and the cliprule are needed for this to work? I only see a single Path so it must be a compound shape, I just always get confused when I see these exotic properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, so do I. I always remove them and test the output to be sure they are required. In this case, sections of the "awning" in the icon become filled unless the property is included.

@jasmussen jasmussen self-requested a review August 4, 2020 09:19
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.

This is a good design, and the store looks like it exists in the same city as the house. 👍 👍

As we add icons to the component, it seems to make sense to lay down some ground rules for what goes into the compent and what doesn't. Notably this PR simply adds the icon to the component, it doesn't also use it. So it begs the question what goes in and what doesn't.

One benefit of an icon component like this is it enables a consistency across projects using these, instead of having to do a mix and match of various things. So it feels like it would be okay to add some icons, even if they aren't also immediately used by the block editor. So let's lay down some ground rules, and we can revisit, refine, and iterate these as we go. Suggestion:

  • If the icon is needed in the project, it should go in as fast as can be reviewed.
  • If the icon is not used in the project, the icon should be generic in subject and in the interest of (and free for) 3rd parties to use. In this case, the PR should gather some design feedback (via the label) and then be added if seen as useful.

In that vein, I can see a ton of folks wanting a store icon. Let's get this in.

@noahshrader noahshrader removed the Needs Figma Update Needs an update to Figma for design purposes label Aug 6, 2020
@jasmussen jasmussen merged commit ca9c8ec into WordPress:master Aug 12, 2020
@jasmussen
Copy link
Contributor

Thank you Jay.

@github-actions github-actions bot added this to the Gutenberg 8.8 milestone Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a store icon
4 participants