-
-
Notifications
You must be signed in to change notification settings - Fork 672
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
Hand-optimizations for size #1020
Conversation
I see |
@aschaap: I've found multiple instances where separate paths have been merged into a single one, this is something we've deliberately chosen not to do as such compound paths sometimes render incorrectly in Figma amongst others. (E.g. the two separate lines of arrows have been merged). |
Not sure we even have support for |
Interesting, I found you in #936 mentioning Figma also does not render zero-length paths with caps as dots. Is this decision to avoid compound paths (and any others like it) documented anywhere beyond the Readme/website? I.e., are there other Figma limitations to be aware of, and are there other similarly limited use cases to support? |
It isn't, but I guess we should probably add this (and some others) to the icon design guide, thanks for bringing this to my attention. In fact the guidelines are in dire need of some rewrite anyway: #650 |
Thank you for the quick response. I found a generic 'improve SVG support' https://forum.figma.com/t/improve-svg-support/2505 dating back to April 2021. Do you know whether there is an issue for the Figma SVG path problems? If Figma's SVG support is an outlier, would it make sense to adapt the package exporter to separate the combined paths instead? |
I'm working on automating some of those optimizations. My opinion:
You can already do things like decompounding and converting But I haven't built the Edit: it also fixes zero length paths. |
@jguddas : I agree with your opinion, but would add nuance to the preference of As for |
@aschaap Can you rebase, your PR. I opmized the rest of the icons, so this PR should be a bit cleaner now. |
@ericfennis Your optimization PR #1025 seems to only rearrange attributes? If you used a tool for this, would it make sense to solve all the merge conflicts it creates? It would seem easier to reapply them after this PR, unless I'm missing something. |
@aschaap I thought you were optimizing the icons by accident. So I thought to do it I a separate PR we could see you changes better. Are your changes now gone after rebasing? |
I optimized icons with the intention of doing so (along with learning more of the SVG Path spec). Rebasing does not eliminate my changes, and is taking time due to having to merge it with rearranged attributes. |
Some optimization types are separated into their own commit, in case these are found to be objectionable.
No new icons, just size reductions where the final result is effectively or exactly identical.
Relevant issue: #902