astro-icon icon indicating copy to clipboard operation
astro-icon copied to clipboard

Consider running iconify's `replaceIDs` function on SVGs

Open charlie-hadden opened this issue 1 year ago • 2 comments

Hi again and sorry for the wave of issues here. This is another one for v1.

Some of the SVGs I'm using reference elements by ID. This works fine when there's a single SVG on the page, but with multiples it's possible for the IDs to collide causing issues with things such as masks. Iconify provides a function which resolves this for me: https://iconify.design/docs/icon-components/svg-framework/replace-ids.html

I currently have a pnpm patch which adds the import and replaces https://github.com/natemoo-re/astro-icon/blob/e66e3adc8747f2b5750ae036957d7ae00a54b2ce/packages/core/components/Icon.astro#L104 with

const normalizedBody = replaceIDs(renderData.body);

This is enough to fix the issue for me by generating unique IDs for each. I'd be happy to submit a PR for this unless it's more convenient for somebody else to do so, but before I do, is there any preference on what prefix to use? Using replaceIDs as above generates ID's like so:

image

Thanks!

charlie-hadden avatar Jul 11 '23 16:07 charlie-hadden

How are you referencing these ids if they're random? You can also use the prefixIds plugin to add prefixes. Something like:

  plugins: [
    'cleanupIds',
    {
      name: 'prefixIds',
      params: {
        prefix() {
        this.counter = this.counter || 0;

        return `id-${this.counter++}`;
      }
      }
    }
  ]

I'm going to consider adding an option to enable this. I think it could cause unwanted issues for people looking to directly reference the ids.

stramel avatar Jul 14 '23 04:07 stramel

I've set up another demo to explain what I mean: https://github.com/charlie-hadden/astro-icon-reproduction/tree/replace-ids

If you give it a try then please use pnpm as there's a patch for astro-icon there to pass the svgo options through (as per https://github.com/natemoo-re/astro-icon/pull/129).

If you run that project then you'll see there are two SVGs displayed. In astro.config.mjs, if you uncomment the cleanupIds plugin then you'll see the issue. There's some other comments there with examples of prefixIds and why, as far as I can tell at least, that won't work for this use case. I'm not an expert with svgo though, so it's possible there's something I'm missing.

I then have another branch which has a pnpm patch to add the replaceIDs call, and you can see the difference in results: https://github.com/charlie-hadden/astro-icon-reproduction/tree/replace-ids-patched

This issue was also quite confusing for me when I first ran across it, because at that point I didn't have any custom svgo config at all - the default preset is enough to trigger the issue with these SVGs.

Hopefully that explains what I mean a little better but let me know if there's anything you'd like me to expand on.

charlie-hadden avatar Jul 14 '23 11:07 charlie-hadden