Displays issues with optimized outline SVGs

Question

Displays issues with optimized outline SVGs

Closed this issue 3 years ago · 18 comments

The react and vue components for outlined icons are displaying strangely for me.

When I reviewed #48, I was doing a lot of testing with the filled components and forgot to check the outline components. 🙁

Having done a little detective work, I have found that the /optimized/outline SVGs seem to look good, whereas the /vue/outline SVGs have the display problems.

Can anyone else replicate this problem on their end? I simply looking at the raw output from running npm prepublishOnly.

@johncantrell97 flagging this for you since you know this well 🙂

Answer 1 · 2021-07-26T17:20:01.000Z

So the 'issue' is definitely in the /optimized/outline svgs and not just the library components. If you look at the SVGO (the tool we are using to optimize/cleanup the svg files) configuration file for outline we are adding a stroke of the currentColor to all of the svgs:

 - 'addAttributesToSVGElement':
      attributes:
        - 'stroke': 'currentColor'

This is what is causing them to look weird but I think it's just surfacing a problem with how the outline SVGs are being created.

This is typically how people manage what color the icons are but this is showing that if you try to set stroke color it will cause distortion to the actual icon.

For now simplest solution is to just remove those 3 lines above from the svgo.outline.yaml but I'd make sure you can still easily control the color of these icons without having the same issue.

Answer 2 · 2021-07-26T18:15:53.000Z

Thank you -- and actually, I concur that the issue is in /optimized/outline; I think I got those directory names swapped when I was copy/pasting them. I will try this out today and see what the results are.

Answer 3 · 2021-07-27T07:56:14.000Z

I can update the source icons in Figma and re-export them to avoid this issue. Could you please let me know how you set up those demo pages? I have no clue how to work with this new set up for the modules in the repo.

Answer 4 · 2021-07-27T08:01:17.000Z

And I may have accidentally published the vue module. I thought I could just test it based on this comment:

You can also run it locally after npm install to try it out and it will generate optimized svgs using svgo and then generate react + vue components into the corresponding directories.

Answer 5 · 2021-07-29T15:34:29.000Z

@GBKS No big deal, we'll fix in the next publish. It's numbered as a pre-release anyways!

For future reference, npm run prepublishOnly will generate the package locally. You can then move to another directory with a Vue project and run npm install <local directory path of Bitcoin-Icons/vue> to test out the module locally. Then npm publish when ready for the package to be available on NPM.

Answer 6 · 2021-07-29T22:07:55.000Z

I have been investigating this problem further. The suggestion from @johncantrell97 did not by itself fix the problem, however it was very illuminating for me and helped me to see more clearly the nature of the issue. Essentially, there are inconsistencies with how the icons are being exported out of Figma (stroke placement, fills, booleans, etc).

Ideally, I'd like to automate this as much as possible; I don't want us having to do a ton of manual flattening work on every icon, and I think it's good they remain in an editable state. So, I am in-progress making adjustments to the Figma plugin. I have been adjusting it to perform some post-processing on the vectors when generating the export page so that they will be more uniform. @GBKS I will submit a PR when the new code is ready for review.

Answer 7 · 2021-07-30T06:15:41.000Z

Thanks for looking into this. Beware of automatically adjusting the icon vectors, as they might turn ugly. Bosch and I already did a bunch of work to get clean exports, like only using center-aligned strokes, I had hoped we'd be in good shape there. My approach to having source and export versions of the icons was for each icon to include both, but hide the source group so it doesn't get exported.

Answer 8 · 2021-08-19T12:17:12.000Z

@sbddesign can you let me know what those issues were you found? I'd like to get them fixed.

Answer 9 · 2021-08-19T13:05:25.000Z

I'll have to put that together for you. There were numerous little issues, so instead of building a list I was working on a script to fix it. I will try to get this script finalized or document a list of things that need fixed for you ASAP. Sorry for the delay.

Answer 10 · 2021-08-19T13:24:51.000Z

Sorry, can you please also provide step-by-step instructions on how to test this? I just tried without success:

npm run prepublishOnly from the bitcoin-icons/vue folder gets me a missing script: prepublishOnly message
Doing the same from the bitcoin-icons does something
npm install <local directory path of Bitcoin-Icons/vue> from my new Vue project I created with vue ui adds "@bitcoin-design/bitcoin-icons-vue": "file:../Bitcoin-Icons/vue" to the dependencies in package.json, but no files are added to the project
Running my test Vue app always shows Error: Can't resolve '@bitcoin-design/bitcoin-icons-vue/filled' because clearly something is not working correctly

I obviously don't understand how this system works.

Answer 11 · 2021-09-09T09:19:58.000Z

Here's video update to the progress on this. It requires both an update to the Figma plugin, and tweaks to many of the icons.

Answer 12 · 2021-09-10T08:22:24.000Z

Here's a new PR #57 with the fixed icons. Could you please test if they show up correctly in your test apps?

Answer 13 · 2021-09-10T14:09:00.000Z

Rebuilt the bitcoinicons.com site in Vue 3, using a locally pre-published version of the Vue module. All works smoothly. PR is here: GBKS/bitcoinicons.com#6

I am trying to systematically work through this process:

Ensure the Figma plugin does what we need, then merge the changes
Locally test the fixed icons with a pre-published version of the Vue module and a PR on bitcoinicons.com
If all looks good, publish the modules, then finalize the site update and get that live

Seems like a really fragile process, so I'd appreciate help with testing and ensuring this all works.

Answer 14 · 2021-09-10T18:44:20.000Z

Published the Vue module and implemented it on a bitcoinicons.com repo PR, see the preview.

This is in good shape now. I'll try to wrap it all up next week. Most likely I'll need help with React.

Answer 15 · 2021-09-13T06:48:44.000Z

Alright, did a few more fixes and also got the React module working. 0.1.6 is the current module version, you can get the react one here.

The live version of bitcoinicons.com also now uses the Vue module, making it easier to verify the icons render correctly.

Would be sweet if someone could verify that all works as desired now. Once that has happened and we've closed this issue, I'd like to revise the documentation with the latest changes, and maybe we can get started with that lookup table.

Answer 16 · 2021-09-16T17:06:50.000Z

I've tested in React and Vue, it works correctly. I do have some nitpicks about certain icons, but I'll open those as separate issues. I feel like this framework is working now.

Answer 17 · 2021-09-16T17:07:27.000Z

Thanks for all of this work @GBKS 🙏

Answer 18 · 2021-09-17T09:32:40.000Z

@sbddesign awesome, thanks for testing. There are some icon changes I was not happy with, but made more for the sake of getting export and rendering working properly. As those were fairly minor, I didn't want to hold things up further. Please open up those issues as you see fit.