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

Update icons (22x22 + generation script) #346

Open
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@bpierre
Copy link
Member

bpierre commented Mar 29, 2019

image

  • All the icon dimensions are now exactly 22x22.
  • Some icons colors, alignments and vector lines have been fixed too.
  • A new script, scripts/generate-icons, is used to optimizing the icons SVGs and create the corresponding components.
  • The gallery now automatically displays all icons (no need to update the page every time we add an icon).
Update icons (22x22 + generation script)
- All the icon dimensions are now exactly 22x22.
- Some icons colors, alignments and vector lines have been fixed too.
- A new script, scripts/generate-icons, is used to optimizing the icons
  SVGs and create the corresponding components.

@bpierre bpierre requested a review from sohkai Mar 29, 2019

@sohkai

sohkai approved these changes Apr 1, 2019

Copy link
Member

sohkai left a comment

👍 This is going to be quite the breaking change 😅.

What do you think about adding a bit of scripting to close out #295? This script is also super fast 🏎!

Also wondering if #96 is relevant anymore; it seems like we could just maintain all icons with 8B9396?

And finally, since this is a breaking change, it seems like a good time to rename anything we might want to rename: #32.


I did also notice that running optimize:svg resulted in a few super small changes to a few of the icons 😉.

/>
<Arrow
style={{
transformOrigin: '10px 9.5px',

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 1, 2019

Member

I think this is already declared in the styled component

"icons:build": "svgr --no-semi --replace-attr-value '#8B9396=currentColor' --no-title -d src/icons/components src/icons/svg",
"optimize:svg": "find ./src -name *.svg -exec svgo --config '{ \"plugins\": [ { \"removeDesc\": {\"removeAny\": true} }, { \"removeTitle\": true }, { \"removeViewBox\": false } ] }' {} \\;",
"icons:build": "node scripts/generate-icons",
"optimize:svg": "find ./src/components -name *.svg -exec svgo --config '{ \"plugins\": [ { \"removeDesc\": {\"removeAny\": true} }, { \"removeTitle\": true }, { \"removeViewBox\": false } ] }' {} \\;",

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 1, 2019

Member

It looks like we could potentially remove optimize:svg since we run the optimizer again in scripts/generate-icons, but I do like having this so committing only the optimized version is easier 👍.

@bpierre bpierre referenced this pull request Apr 7, 2019

Merged

Finance filters refresh #767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.