-
-
Notifications
You must be signed in to change notification settings - Fork 516
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
feat: Flatten icons #2135
feat: Flatten icons #2135
Conversation
This is pretty breaking changish. |
Any chance this one will be merged/published before 1.0? |
@wellguimaraes: Not sure if we should do this before v1.0, as @jguddas has stated this could potentially break a lot of apps that currently rely on the unflattened paths (e.g. to animate or fill only certain parts of icons). 🤔 |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This enables the implementation of additional optimizations without modifying the original SVG files.
Transform shapes to paths and merge all paths together. This reduces the icon size and solves the issue of overlapping strokes when using a transparent color.
Add a custom plugin to convert rectangles with rounded corners to paths. Hopefully, this may be removed when svg/svgo#1963 or another similar PR is merged in SVGO.
Don't close. |
IdeaMerge all the paths that are overlapping into a single element, keep everything else that is not connected separate. |
While detecting and merging overlapping shapes would be a nice compromise, I have two objections. How doable is it?Detecting overlapping shapes is non trivial. SVGO won't do it for us, so that would require quite a bit of work. If someone is motived to work on the feature, that's great. But I have neither the time, the inclination, nor the competences to do it myself. Does it serve a real purpose?Yes, exposing the SVG icon as a list of distinct shapes may allow users do implement custom behaviors such as applying different colors. But are there people currently doing that? I'd be interested to hear from them. IHMO, trying to treat certain parts of icons differently is already taking the risk to experience a breaking change anytime you update the package, regardless of the build process. In order to apply specific colors to specific shapes, you need to know the order of the shapes in the SVG, which is not guaranteed to remain the same. In fact, of the 22 icons that have been modified in the past three weeks:
Anytime an icon is updated, it may break any assumption one may have about it other than "it should still look mostly the same (assuming there wasn't an issue in the original icon that we had to fix)". So, as long as updating the build doesn't significantly alter an icon's appearance, I don't think it is more of a breaking change than any other update. The scope may be larger but the impact on individual icons is the same. |
I can whip something up for that, we already have custom sorting and optimization logic.
I need some idiomatic way to sort things for my custom optimization code to work properly i.e. we already have and need sorting. The current code is less than perfect (see #1938).
I really want to actually support this. My proposal would be to merge touching paths and sort from smallest to biggest bounding box. |
I think I was a little bit too pessimistic and spoke too fast.
I had a quick look at
I probably need to reconsider how the optimizations are run, then. My idea was to run the optimizations during the build process, because it removes the need to update individual icon files, and icon designer would remain free to organize their shapes however they want. However, I understand that you want to enforce a canonical order in the pre-commit optimizations, in order to keep the icon structure stable, prevent filling larger shapes from hiding smaller ones, and allow users to make some assumptions about the icons. Merging shapes after they've been sorted may go against some of those goals. Maybe I should keep the optimizations out of the build process and put the optimization pass in the pre-commit optimizations. SuggestionRevert all the changes (in particular the change to run optimizations during the build process) and add the optimization pass that you suggested: For each shape, test whether it intersects any other shape. If it does, convert both to Cons:
Pros:
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
Related to #1687.
What is the purpose of this pull request?
Description
This PR modify the package build scripts to flatten the exported SVG icons.
While source icons are made up of several shapes (
<line>
,<rect>
,<circle>
, etc), all the shapes now get converted to<path>
and merged together in a single<path>
element during the build.As mentioned in #1687, this has two benefits:
Implementation
We use the Convert Shape to Path and Merge Paths SVGO plugins to convert shapes to paths and merge the paths together. Currently SVGO cannot optimize rectangles with rounded corners (svg/svgo#1963), so this PR includes a custom plugin to do that.
Impact on icon size
In most cases, converting shapes to
<path>
reduce the icon's size, especially for icons made up of many shapes (e.g.sliders-horizontal.svg
). In some cases, we observe a small increase of the size (e.g.layout-grid.svg
), because circles and rounded rectangles are more compact than paths. Overall the output size is reduced by ~13%.Impact on the build time
One would assume that processing each icon for every build target would be bad for the build time. However, I haven't observed any significant increase of the average build time on my machine (it actually went slightly down).
Impact on icon appearance
Icons should appear the same. However there are two noticeable differences.
When using a semi-transparent stroke color, the icon now has a consistent appearance instead of showing overlaps.
Edges are a little bit smoother in places where several shapes converge. This isn't visible to the naked eye.
Before Submitting