[v1] Allow string width and height for icons

[fflaten]

Thanks for releasing 1.0.0-next.4 which finally resolved the components export!

v1 currently sets both width and height on svg-element to icon defaults or 1em, but size/width/height props in Icon-component only supports number values. Would it be possible to extend it to number | string so we could set 1em, 1.5rem etc.?

[natemoo-re]

Yep, good call. I think 1em is also not supported in Safari? I'll have to double check that.

[fflaten]

Don't know, haven't tested attributes yet. Using rem through CSS atm which works on iOS Safari.

[nanarino]

This is all a side effect of using Sprite.

In the past (in v0), only width needed to be given, and height was automatically calculated based on the aspect ratio.
Now after migrating to v1, it can be solved by specifying font-size or completing height, although there are warnings 🤔.

In addition, #astroicon:{name} needs to be changed to #ai:local:{name}. This seems to be not mentioned in the migration document.

[natemoo-re]

In the past (in v0), only width needed to be given, and height was automatically calculated based on the aspect ratio.
Now after migrating to v1, it can be solved by specifying font-size or completing height, although there are warnings 🤔.

If you could share the specific warnings you're getting and where you see them, that would be very helpful!

In addition, #astroicon:{name} needs to be changed to #ai:local:{name}. This seems to be not mentioned in the migration document.

I'm not sure how you're using the id, but these aren't mentioned in the migration guide because the generated ids are not part of the public API. The [data-icon="name"] attribute is the stable, public way to reference an icon.

[nanarino]

the warning is that the problem with the previous version is gone now🥰 I only gave width before and now complete height.

I'm not sure how you're using the id, but these aren't mentioned in the migration guide because the generated ids are not part of the public API. The [data-icon="name"] attribute is the stable, public way to reference an icon.

https://github.com/nanarino/nanarinostyl/blob/f6061c6e4965df2496812ac21be856e68812e45c/src/components/kanban/emit-message.ts#L31

This is how I use it, I don’t know if there is a better way than how I use it.

[natemoo-re]

Interesting, I haven't really considered how client-side usage should work yet. I guess that's fine if you know that icon is already defined on the page somewhere.

[ShelbyJenkins]

@natemoo-re if you input width="1rem" the type linter reports an error that you're providing a string type to a number type.

interface Props extends HTMLAttributes<"svg"> {
  name: Icon;
  "is:inline"?: boolean;
  title?: string;
  size?: number | string;
  width?: number | string;
  height?: number | string;
}

Adding the union type of string resolves the issue.

[cbontems]

Hello,

Having the same linter errors when using a string for size, width or height, I did fix it by modifying the Props type this way:

interface Props extends HTMLAttributes<"svg"> {
  name: Icon;
  "is:inline"?: boolean;
  title?: string;
  size?: HTMLAttributes<"svg">["width"];
}

• width and height beeing already defined on HTMLAttributes<"svg"> as number | string | undefined | null it doesn't seem necessary to add them to the extended interface.
• then size type can be defined as the type of width.

In the current version, it is interesting to note that the infered type of normalizedProps includes width: number | string and height: number | string because of IconifyIconBuildResult defining widthand height as strings.

Happy to submit a PR if this helps.

[stramel]

This has been resolved.