ionic-team / ionicons

Premium hand-crafted icons built by Ionic, for Ionic apps and web apps everywhere 🌎
http://ionicons.com
MIT License
17.39k stars 2.06k forks source link

test: use optimized assets #1354

Closed liamdebeasi closed 1 month ago

liamdebeasi commented 1 month ago

When testing Ionicons manually there was an issue with our configuration where the index.html file served by the Stencil dev server never used the optimized SVG assets.

We've had issues in the past where the optimization script incorrectly changes the behavior of an SVG, so being able to validate that this script is working as intended is important.

The index.html used in testing loads asset from the build directory. The build script does copy optimized SVG assets to the build directory. The problem is that this causes the Stencil Dev Server to re-build. As part of the Stencil config we instruct it to copy all unoptimized SVGs to the build directory.

As a result, we have the following flow:

  1. Build script runs.
  2. Optimized SVGs are copied to build/src.
  3. Stencil dev server rebuilds.
  4. Unoptimized SVGs are copied to build/src overriding the optimized SVGs.

I've made the following changes to correct this:

  1. Removed the SVG copy step from the Stencil config.
  2. Updated the start script to optimize all SVG assets and copy them to the build directory. This ensures that optimized SVGs are being generated every time npm run start runs. Note that Stencil will not re-run this optimization step on each re-build. Devs need to continue to run npm run build.files after the initial build if they want to modify icons and see their changes.

Testing:

  1. Run npm run start
  2. On the testing page that loads, verify that svg elements inside of the Shadow DOM for ion-icon have the ionicon class.
  3. Run npm run build.files. Repeat step 2.

Note that on main, the ionicon class will not be present on the SVG elements indicating that the unoptimized SVG is being loaded.

The screenshot diffs here are correct and were capturing the original buggy behavior. The call (@) icon has color="tertiary" set and should always have been a cyan color. The heart outline color had --ionicon-stroke-width: 8px as opposed to the default 32px and should always have been thinner than normal.