natemoo-re / astro-icon

Inline and sprite-based SVGs in Astro made easy!
https://astroicon.dev
Other
995 stars 57 forks source link

[v1] feat: allow svgo options to be passed in #100

Closed stramel closed 1 year ago

stramel commented 1 year ago

This is a pretty simple new feature that will allow consumers to pass their own svgo options.

I did notice that specifically there was an issue where title is removed during the importDirectory call and prevents a consumer from being able to disable the removeTitle plugin in svgo to keep the title tag.

Fixes #68

changeset-bot[bot] commented 1 year ago

⚠️ No Changeset found

Latest commit: 184e4f2d2f108bd04a5f31374e58573e3f0882d4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api-astroicon ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2023 2:26am
astroicon ❌ Failed (Inspect) Jun 6, 2023 2:26am
charlie-hadden commented 12 months ago

Hey! I'm not sure if it's intentional or not, but it seems this doesn't quite get all the way. I see you've added the new option here:

https://github.com/natemoo-re/astro-icon/blob/184e4f2d2f108bd04a5f31374e58573e3f0882d4/packages/core/src/loaders/loadLocalCollection.ts#L13

But it looks like that never actually gets passed here: https://github.com/natemoo-re/astro-icon/blob/184e4f2d2f108bd04a5f31374e58573e3f0882d4/packages/core/src/vite-plugin-astro-icon.ts#L28

stramel commented 11 months ago

Oh, looks like it may have been a bad merge conflict resolution. Thank you for pointing this out @charlie-hadden!