| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
|
Sorry, something went wrong.
|
/cc @cyberalien Maybe we need this at iconify utils instead only here, this way we can reuse this at UnoCSS preset-icons and also at iconify, can you take a look at this 🙏 ? |
Sorry, something went wrong.
npm i https://pkg.pr.new/unplugin-icons@433 commit: 28b40fd |
Sorry, something went wrong.
|
I'm not sure its even worth the effort. This is a one time use case for a custom icon. Why not just embed icon without unplugin? |
Sorry, something went wrong.
|
@cyberalien I agree embedding directly is a fine workaround for a one-off asset. My thinking was more about custom collections in general: they can include non-square SVGs like logos, wordmarks, flags, or other artwork where width: ...; height: auto; should follow the viewBox ratio. To me that nudges custom collections from "bring your SVG icons" a bit closer to "bring your SVGs", without adding much complexity. This PR doesn’t add a new API or change defaults, it just makes the existing customization path honor explicit width/height = unset|none by omitting those attributes on the root <svg>. So the normal icon case stays unchanged, and only users opting into that behavior are affected. |
Sorry, something went wrong.
|
You are right. I didn't realise Unplugin Icons generates square icons only. That's bad and needs fixing. |
Sorry, something went wrong.
|
Did more tests, sizing is broken if one attribute is set. No attributes: height is set to 1.2em, width is calculated using icon's width/height ratio. So the only scenario where it works correctly. If height is set, width is completely broken: calculated using icon's width/height ratio from 1.2em, not from height attribute. If width is set, height is broken, same as above. unset not only doesn't work, it sets both width and height to 1.2em, ignoring icon's width/height ratio. Tested by replacing icons in Vue example to fa6-regular/comments, which is a 640 x 512 icon, so it has 1.25 width/height ratio. |
Sorry, something went wrong.
|
Just to be clear, you tested that with this branch? |
Sorry, something went wrong.
No, I've tested with default to make sure things are broken. Now I've tested with this branch:
|
Sorry, something went wrong.
|
I confirmed that behavior locally, and your summary matches what the code is doing. Looking at the code path, I think if this is fixed in @iconify/utils at the shared loader level, unplugin-icons probably wouldn’t need its own implementation change for it. unplugin-icons is basically forwarding through loadNodeIcon() and then compiling the returned SVG, so this branch is mainly compensating for the current upstream loader behavior around unset. I’ll take a look at solving it upstream instead, since that seems like the cleaner place to address it. |
Sorry, something went wrong.
|
@cyberalien @iconify/utils has a specific test that contradicts your expectations: test('loadIcon with unset width', async () => {
const result = await loadNodeIcon('flat-color-icons', 'up-right', {
customizations: {
additionalProps: {
width: 'unset',
},
},
});
expect(result).toBeTruthy();
expect(result && result.includes('width="')).toBeFalsy();
expect(result && result.includes('height="1em"')).toBeTruthy();
expect(result && result.includes('height="')).toBeFalsy();
});So, it expects the height to be the default 1em, even when we pass width: 'unset'. Is it ok to change this test? It might be a "breaking" change in that sense. |
Sorry, something went wrong.
|
Yes, if whole parsing logic is fixed, that test can be fixed too. That test was intentional, but I don't remember what it was fixing. |
Sorry, something went wrong.
|
Merged pull request to Utils and tested it, works correctly with main branch, without this pull request. So it was fixed in Utils. I've also pushed a PR that replaces square icons with non-square icons in Vue example to test generated components: #434 |
Sorry, something went wrong.
|
Related to this issue: can anyone test @iconify/utils@dev, which contains fix for this issue, with UnoCSS? I've tried doing it, but experiencing massive amount of errors trying to build UnoCSS from main branch, cannot even get to testing dev version of utils. Tried fixing some build errors, ran into more and more other build errors. |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
I've tested with fa6-regular:comments, which is 640x512 icon
Yes |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Description
Fixes collection-specific suppression of generated SVG dimensions so custom SVG collections can preserve non-square artwork sizing, such as horizontal brand logos.
Today, unplugin-icons always ends up emitting root width/height attributes for generated components unless sizing is disabled globally with scale: 0. That works well for normal icons, but it breaks CSS sizing patterns like width: 11rem; height: auto; for custom logo artwork because the outer <svg> keeps a square intrinsic size.
This PR keeps the existing customization API and makes iconCustomizer() and query params honor width: 'unset' / height: 'unset' (and 'none') by removing the root <svg> dimensions after Iconify renders the SVG.
Included in this PR:
Linked Issues
Fixes #353
Additional context
Reviewers may want to focus on the suppression semantics in src/core/loader.ts, especially the one-sided cases:
Validation I ran locally: