-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Optimize SVG icon with 20 size viewBox to size 24 #50520
Comments
Thanks for the issue, this is an excellent bit of code quality we need to get in front of. The answer is simple: every single icon in the icon library should be 24x24 and with a 24x24 viewbox to match. If something doesn't match those metrics, it's either erroneous or not an icon (i.e. there could be edge cases where a particular component needs an SVG element for some functional piece, but then it shouldn't be in the icon library). Unfortunately there's still some catching up to do as far as consistency between the Figma icon library and what's shipping. Both in terms of icons missing and their naming, but also some older icons that need updating to the new style. #38850 is a key challenge that would be good to solve in terms of moving that forward. |
@jasmussen I have learned that there are already heading icons for all levels in the Figuma icon library: What do you think about shipping these icons to In the future, they may also be available in a toggle group control that selects the heading level in the global styles: |
It seems a mistake we haven't done that, let's definitely do so. I can help with the SVGs next week, I've a bit on my table today. |
@jasmussen Thank you so much! I would like to submit a PR as soon as possible. |
Some hard-coded icons discussed in this issue have been replaced with icons from the Subdirectorypackages/block-editor/src/components/block-list/subdirectory-icon.js This icon is used only in the mobile app. On the left side, there is an icon with the top, bottom, left and right sides reversed, which might be able to divert. ColorSelectorSVGIconpackages/block-editor/src/components/color-style-selector/index.js This icon is part of a child of the |
I think so too. In the following image, the paragraph block in the group block is selected, and the icon to the left of the subdirectory-icon seems to indicate the parent block. Furthermore, this icon is just an icon and does not appear to have any click events.
This is a great idea! I have yet to run a native Gutenberg app in development mode, but I would like to give it a try. |
What problem does this address?
All icons in the
@wordpress/icons
package are based on a 24-size viewBox. However, some hard-coded icons exist that have a 20-size viewBox.Below is a list of hard-coded icons with size 20, with the exception of the tests.
HeadingLevelIcon
component #52008What is your proposed solution?
My understanding is that if the height and width of the viewBox match, the drawing area is a square, so it makes no difference whether the size is 20 or 24. However, I am concerned that the values of the
width
/height
andviewBox
attributes do not match when the SVG is displayed at 24px size for an icon created based on a 20-size viewBox.An example is the block toolbar icon for the heading block:
I am not familiar with the design rules for icons in the Gutenberg project, but I am wondering if it would be a good idea to unify these icons at size 24.
The text was updated successfully, but these errors were encountered: