Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Icon in NavItem #267

Open
akiarostami opened this issue Feb 22, 2024 · 3 comments
Open

Icon in NavItem #267

akiarostami opened this issue Feb 22, 2024 · 3 comments

Comments

@akiarostami
Copy link

I am fairly new to svelte-us, so I hope I'm not missing something obvious.

I was trying to get NavItem to show an icon from Lucide, but I couldn't. I started looking at the source code of the components, and it seems that the way icons are handled in Buttons is very different from NavItem. This is the Icon part of NavItem:

{#if icon}
	<Icon path={icon} class={cls('mr-3 flex-shrink-0', settingsClasses.icon, classes.icon)} />
{/if}

and this is what I saw in Button:

{:else if icon}
    <span in:slide={{ axis: 'x', duration: 200 }}>
      {#if typeof icon === 'string' || 'icon' in icon}
        <!-- font path/url/etc or font-awesome IconDefinition -->
        <Icon
          data={asIconData(icon)}
          class={cls('pointer-events-none', settingsClasses.icon, classes.icon)}
        />
      {:else}
        <Icon class={cls('pointer-events-none', settingsClasses.icon, classes.icon)} {...icon} />
      {/if}
    </span>
  {/if}

One seem to be using path, and the other one checks for several options, except path.

My question is why we have two different ways to handle something that seems to have the same logic and are very similar? Also, is there a way to simply pass a component as Icon (there are many libraries doing that already, such as Licide, and there's no way to reimplement everything)?

I apologize in advance if I get this totally wrong. As I said, I'm just getting familiar with svelte-ux.

@techniq
Copy link
Owner

techniq commented Feb 22, 2024

Hey @akiarostami 👋, thanks for checking out Svelte UX. There are a few things here to mention 😁.

  • Icon supports a lot of use cases based on the prop passed
    • Icon path data: <Icon path="M12,4A4,..." />, typically loaded from an NPM package like @mdi/js and used <Icon path={mdiAccount} />. This is what I typically recommended (although see data below)
    • SVG strings: <Icon svg="<svg ... />" />
    • SVG url: <Icon svgUrl="https://api.iconify.design/mdi:account.svg" /> for hosted SVG icons. This is useful when you want the user to be able to select a font at runtime (or set in a database) but not load the full icon set (since you won't know what is used and thus can not rely on tree shaking)
    • slot: <Icon><svg ...></Icon>, useful for <svg> elemnts or web fonts such as Material Symbols

Lastly, there is <Icon data={...} /> which applies some heuristics to determine which of the above to use (except slot). This is nice because you can support most of the use cases mentioned (path, svg, and svgUrl) with a single prop, and is especially useful when Icon is nested in another component, like Button.

Along with all the above, when using Button you can also support passing all the props to icon as an object, not just the path/data/svg (which is what the {:else} is support...

<Button
  icon={{ data: mdiTrashCan, size: "2rem", style: "color: crimson" }}
  color="danger"
>
  Delete
</Button>

additionally, while we don't at the moment, we could support passing the icon as a slot...

<Button>
  <Icon slot="icon" data={mdiTrashCan} />
  Delete
</Button>

With probably way more context than you were looking for... regarding NavItem, the short answer is

  • It should be using <Icon data={...} /> instead of <Icon path={...} > internally to support the additional svg and svgUrl, but should also be setup to support
  • It should support passing all Icon props as an object
  • It should support passing an icon slot

Button should also be updated to support passing an icon slot... and generally review other <Icon> usage in other components to make sure it has the same support (where applicable)

@techniq
Copy link
Owner

techniq commented Feb 22, 2024

A few more things :)

You should be able to use icon component libraries via slots, but like mentioned in <Button>, we need to add one. One reason it wasn't added is just putting the icon before the main content in the default slot is 98% the same as if we did it via a slot prop (ignoring the slide transition when toggling the loading state of a button).

I'm typically not a big fan of using individual icon components per icon (ex. <UserIcon>) but instead prefer using paths (ex. <Icon data={userIcon} /> as the paths can be passed as props to other components and I find them flexible to style (and even combine multiple paths into the same icon). I have an example of this on the Icon docs.

Another thing worth mentioning with Button is while icon props as an object can be useful...

<Button
  icon={{ data: mdiTrashCan, size: "2rem", style: "color: crimson" }}
  color="danger"
>
  Delete
</Button>

mostly the same can be accomplished with classes props

<Button
  icon={mdiTrashCan}
  classes={{ icon: "text-danger-300 text-lg" }}
  color="danger"
>
  Delete
</Button>

@akiarostami
Copy link
Author

Thank you for the detailed answer, @techniq. Very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants