weightless icon indicating copy to clipboard operation
weightless copied to clipboard

Use inline SVG instead of Material-Icons font for wl-expander

Open brettwillis opened this issue 5 years ago • 2 comments

Consider this scenario:

  • I use my own set of inline SVG for icons so I can share and bundle them within my app
  • I use your wl-expander

Now I need to load the entire Material Icons font just for the glyph on the wl-expander. I also have issues around the Material Icons font being a render-blocking resource for the page (reported by Lighthouse), and I also have to deal with Flash-of-un-styled-text, etc in slow network conditions.

If you use inline SVG, now the icon is bundled with the source, and I don't need to worry about loading the font, or any of the accompanying issues.

For example:

html`<svg width="24" height="24" viewBox="0 0 24 24"><path d="M16.59 8.59L12 13.17 7.41 8.59 6 10l6 6 6-6z"/><path d="M0 0h24v24H0z" fill="none"/></svg>`

Instead of:

<!-- Required only for the wl-icon in the wl-expansion -->
<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">

And (in expansion.ts):

import "../icon";
...
html`<wl-icon id="icon">${this.icon}</wl-icon>`

I guess the down-side is that there may be increased file size for projects that do use wl-icon and the Material Icons font.

Perhaps you would export a set of SVG icons that are used within Weightless so that they can also be shared/bundled in the parent project.

Thoughts? Is this good or have I got the wrong idea here?

brettwillis avatar Jun 13 '19 22:06 brettwillis

By far the best option as I see it is to make all icons in other components slots - it gives way more flexibility to anyone using the components. Given how many different options there are for icons, trying to find a solution that fits everyone without just delegating seems like a fool's errand. Defaults are another thing, of course - filling the slot with wl-icon seems like a sensible default. Given that, it seems like a good solution in general.

Of note is that the select component uses inline SVG as you describe (but, by contrast, has no option to customise it at all).

Lattyware avatar Jun 14 '19 06:06 Lattyware

Ok, that sounds reasonable.

So then the action required here is to move the <wl-icon> into the default/fallback content of the indicator slot.

Current code is:

<div id="indicator">
  <slot name="indicator"></slot>
  ${this.icon != null ? html`<wl-icon id="icon">${this.icon}</wl-icon>` : nothing}
</div>

Meaning that there is no way to override the <wl-icon>.

Is it also reasonable to omit import "../icon"; (to reduce code size if using a different approach) and leave it up to the parent project to import it... seeing as we have to manually import the font anyway.

brettwillis avatar Jun 16 '19 21:06 brettwillis