AMoreaux / Sketch-Icons

πŸš€ Generate a dynamic icon library
https://sketchicons.com
MIT License
878 stars 45 forks source link

Enhancement: Respect icon position and size within SVG viewBox/canvas #2

Open bruskowski opened 6 years ago

bruskowski commented 6 years ago

Is it possible to adopt the position and size of the icon path/shape within the original SVGs viewBox/canvas?

In other words: When I have an icon on a 48x48px canvas which is actually only 40x40px large and an icon on a 48x48px canvas which extends to 44x44px and I import them using the Plug-In, the original proportions between the two icons are lost, as both are scaled to the size defined at import (minus padding).

I know Sketch throws this information away when importing SVGs.

A possible workaround might be: Adding a rectangle of the viewBox' size to every SVG before importing and deleting that rectangle afterwards.

Awesome plugin btw, might be really useful. Thanks and congrats!

AMoreaux commented 6 years ago

Hello Richard,

Thanks for your feedback. 

I’m not sure to understand. Could you record a gif or a video to help me understand your problem?

Thanks in advance πŸ™‚

bruskowski commented 6 years ago

Hi Antoine,

I tried to illustrate it, maybe this makes it clearer: image

Let me know if you have further questions!

AMoreaux commented 6 years ago

Hi Richard,

Thanks for illustrating your issue. I understand now. I will look into it and come back to you πŸ™‚

AMoreaux commented 6 years ago

@richardbruskowski I think I solved the problem with your idea. πŸ‘ I would like to test with your two icons. Could you send them to me please?

bruskowski commented 6 years ago

Cool! Sure, here you are: Archiv.zip

valentinchrt commented 6 years ago

Thanks for your feedback @richardbruskowski! Stay tuned, I'm pretty sure this feature will be in the next release πŸ˜‡πŸŽ…

mattiasj commented 6 years ago

Plus one for this enhancement!

AMoreaux commented 6 years ago

Thank you for your feedback. The last version (v3.0.2) solve the problem. Can you confirm it to me ?

mattiasj commented 6 years ago

Unfortunately, I get the same offset positions for the icons after importing the SVG files. Example:

skarmklipp 2017-12-25 18 07 36
mattiasj commented 6 years ago

Should I attach a zip with some test icons?

AMoreaux commented 6 years ago

Yes please.

mattiasj commented 6 years ago

Here we go! Icons.zip

AMoreaux commented 6 years ago

@mattiasj The problem was that there was a transparent square in the svg. (Illustrator add a rect automatically ? ) Now the use case is taken into account. Can you check on your side please. Do not hesitate to give me other feedback. πŸ˜‰

mattiasj commented 6 years ago

Thanks a lot! It works perfect! I have one small request, if possible I would like to name the color fill symbol according to our defaults (Color, instead of 🎨 color). I tried renaming with Rename It plugin but it needed to much manual work. Any ideas? Besides that β€” works like a charm :)

AMoreaux commented 6 years ago

I talk about it with the second guy of the project. I'll come back when we make a decision. πŸ™‚

mattiasj commented 6 years ago

Sounds good! Thanks! I will see if I can rename them for now :)

mattiasj commented 6 years ago
skarmklipp 2017-12-26 23 15 53

I have been trying to import more icons, but notice that without the bunding box rectangle some (more complex) shapes will get distorted. The color fill get applied to several shapes. I attach screenshot + example icon.

mattiasj commented 6 years ago

Icon.zip

AMoreaux commented 6 years ago

I have change the process to import and mask icons. With the new process (that you used) i keep the original structure of the svg. So to apply mask i must add it on all shape. I have change because some svg are corrupt with precedent method. But all methods are not perfect... I know it's not very beautiful... if you have suggestion to improve the process, i'm interested. For the box which is used to keep proportion, I add it in the import process. I have no bug with the icon user-alt-24.svg. Can you send me your rendering. And if you have others icons with wrong importation. I can watch them.

screen shot 2017-12-26 at 23 52 12
mattiasj commented 6 years ago

Hello! I get worse results now, than in the beginning. Will attach a .zip with one example icon β€”Β and the results with plugin vs how the desired end result would be. Having several color fills will make it hard to maintain for large design systems. Let me know if I can assist further. Arkiv.zip

AMoreaux commented 6 years ago

Hello! I maked a big update for the feature of import icons. Now for one icon you have one layer so one mask. Can you try it with your icons libs ? For the problem of the name of the layer mask, I will speak with the author of rename it to resolve it.

mattiasj commented 6 years ago

Hey @AMoreaux! The import works flawless now, and the symbol fills are great. I will try to find a plugin to batch rename the masks (until/unless) you implement an option to enter a custom name for the fill :) Thanks!

mattiasj commented 6 years ago

Unfortunately, we were close but not fully in goal. Looks like the fill layer gets slighly wrong dimension and is placed -1 -1 in dimensions. See screenshot. I imported 64x64 icons, the fill symbol gets 66x66 and is offset -1 -1 instead of 0 0.

skarmklipp 2017-12-28 12 55 10
AMoreaux commented 6 years ago

It's fixed. Thank you πŸ‘

mattiasj commented 6 years ago

It worked perfect, and now I have all my icons set up with color fill. 16px, 24px, 32px, 48px and 64px. This would be impossible without this plugin. MUCH appreciated!

AMoreaux commented 6 years ago

Awesome! Thank you very much for your feedback. πŸ˜„

valentinchrt commented 6 years ago

Glad to hear that @mattiasj! Thanks so much for your help! πŸ™

Regarding the renaming, I'm wondering if it's a good solution to integrate this feature into our plugin. I'm afraid to make Sketch Icons plugin more complicated.

I think Rename It should have the functionality to search and rename without having to select a layer. We will suggest they add this feature, I think it's a better solution for everyone.

mattiasj commented 6 years ago

My pleasure! I agree that you should keep the plugin focused on one job. I renamed my color symbols easily with https://github.com/sonburn/symbol-instance-renamer to my preference. Easy and neat solution!

valentinchrt commented 6 years ago

Hey @mattiasj,

In fact, you can batch rename the masks with Rename It.

Here's how to do it:

  1. Import your icons and apply color mask with Sketch Icons
  2. Select all with Cmd + A
  3. Go to Plugins -> Rename It -> Find and Replace Layers Names
  4. Find: 🎨 color
  5. Replace: Color

That's it!

Have a nice weekend, Valentin

rename-it-select-all
bruskowski commented 6 years ago

Hi!

I do experience slight shifts in position when importing icons. Maybe a rounding/precision issue? A quick test suggests that this might only occur with paths who do not sit on exact integer value pixels:

screenshot

The zip file includes the whole experiment: offset-on-import.zip

AMoreaux commented 6 years ago

hi, I understand the problem. I try to fix it in the next release.

AMoreaux commented 6 years ago

Hi @richardbruskowski, @mattiasj. If you want to try the last version contact me on twitter. We search feedbacks.

bruskowski commented 6 years ago

Hi @AMoreaux I just checked v4 with the same setup. ( https://github.com/AMoreaux/Sketch-Icons/issues/2#issuecomment-356651144 ) Still experiencing a shift when importing icon-test-glyph-2 as described above.

sigtm commented 6 years ago

Just wanted to cheer you guys on – if position within the viewBox was preserved, this plugin would immediately save me a ton of time :) In case it's helpful in any way, here is a production SVG from an icon set I'm working on:

https://www.dropbox.com/s/1bnt11mxrigxxen/calendar.svg?dl=0

It is pretty similar in structure to Google's Material icons (production SVG versions). You'll notice that the calendar is slightly off center, so it aligns with +, -, etc type icons with a 1px stroke while still having a even numbered artboard size. When I import with Sketch Icons, it appears to just center the shape on the artboard, placing it on a .5 pixel value.

Let me know if I can be of any help!

AMoreaux commented 6 years ago

Hi @sigtm , if you use this version https://drive.google.com/open?id=1A9oHw2_0VvmErXElk_yLBJZJvuXmn9Te and you import icon without mask, the position in the viewbox will be keep.

sigtm commented 6 years ago

@AMoreaux indeed it is, nice one! Watching the repo now, looking forward to including it in my workflow as soon as it works with masking as well πŸ‘

sigtm commented 6 years ago

@AMoreaux is this implemented in production now? I just did a quick test and it seemed to work as expected, so just curious if there's a reason this issue isn't closed before I set it loose on my precious production files :)

AMoreaux commented 6 years ago

Hi @sigtm I tested with @richardbruskowski 's icons (above) and it worked on my side. Can you try with your icons and confirm everything is fine. Thank you πŸ™‚

bruskowski commented 6 years ago

@AMoreaux @sigtm I ran some quick tests and every icon I looked at closely apparently was imported with precisely correct positioning. πŸ‘

sigtm commented 6 years ago

@AMoreaux They seem to be positioned correctly on my end πŸ‘

Tinncso commented 5 years ago

Hi, I still have this issue with the latest version v5.0.0. (Same issue that @richardbruskowski had first)

qimmiq commented 5 years ago

hey there, is there also the possibility to import icons with their original viewBox? for example 84x72 + the position an ratio? ...

tijmenvangurp commented 5 years ago

Hi @sigtm , if you use this version https://drive.google.com/open?id=1A9oHw2_0VvmErXElk_yLBJZJvuXmn9Te and you import icon without mask, the position in the viewbox will be keep.

Does not work on my machine: I get MOJavaScriptException: ReferenceError: Can't find variable: MSBackgroundColorView Would realy love the have the feature that respects the viewbox / boundingbox of an SVG.

qimmiq commented 5 years ago

Hi @sigtm , if you use this version https://drive.google.com/open?id=1A9oHw2_0VvmErXElk_yLBJZJvuXmn9Te and you import icon without mask, the position in the viewbox will be keep.

Does not work on my machine: I get MOJavaScriptException: ReferenceError: Can't find variable: MSBackgroundColorView Would realy love the have the feature that respects the viewbox / boundingbox of an SVG.

@tijmenvangurp

Mathieu from Sketch provided me with a plugin yesterday. https://github.com/mathieudutour/import-svg-as-artboard ... works fine for importing svgs with their excisting viewBox/ Artboardsize