feat: improve the `VtmnIcon` in order to used SVG

Tlahey commented 1 year ago

Hello,

In order to increase the performance of vitamin, it is necessary to change the way icons are retrieved.

Currently : We inject vitamix.css with the woff files which contain all the icons of the application. This means that it is possible to load icons that you will never use.

The solution : The objective would be to use the svg files individually to recover only the image that we want.

Technical representation of the modification:

{#await import(`@vtmn/icons/dist/vitamix/svg/${value}.svg`) then { default: svg }}
  <img style={componentStyle} src={svg} class={componentClass} {...$$restProps} />
{/await}

Because of this, the preprocess is able to intelligently copy/inject the svg.

The only problem here is that it is not able to colorize the icon. Since this one is downloaded, the only way to do it is to pass on a filter (used a website like https://isotropic.co/tool/hex-color-to-css-filter/)

Example :

filter: invert(42%) sepia(93%) saturate(1352%) hue-rotate(87deg) brightness(119%) contrast(119%);

This will require generating colors based on design and theme.

The thread is open to find another solution or discuss this one.

Ping @jquinc30 , @lauthieb , @thibault-mahe

lauthieb commented 1 year ago

Hello @Tlahey,

Thanks for your message. Do you think it could be possible to avoid breaking changes for this issue?

And what do you think about adding fill="currentColor" or stroke="currentColor" on SVG elements (<svg>, <path>, <g>, <circle>, etc.)? Allowing control of these colors through CSS. (instead of manipulating filters as you propose).

Unfortunately, we need to be focused for now and cannot give energy on our side on this implementation. But feel free to discuss it there. But what it's sure, it's that's a good point we need to take care of for our next implementation.

Tlahey commented 1 year ago

@lauthieb also not possible with currentColor

Because the svg is downloaded and not inline on the page.

Tlahey commented 1 year ago

I've found a solution but I'm not really fan ...

<script>
  let svgContent;
  import(`@vtmn/icons/dist/vitamix/svg/${value}.svg`).then(module => {
    fetch(module.default)
      .then(response => response.text())
      .then(data => {
        svgContent = data;
      });
  });

</script>

{#if svgContent}
  <div class={componentClass} style={componentStyle} {...$$restProps}>
    <svg xmlns="http://www.w3.org/2000/svg">{@html svgContent}</svg>
  </div>
{/if}

Here 2 thinks :

In order to apply style, we have to set the svg inline
We have to wrap the svg with another svg in order to apply the color (the html doesn't apply the css when he is loaded)

jquinc30 commented 1 year ago

I've found another way with mask-image:

<html>

<head>

</head>
<style>
    .icon {
        --svg: url("https://raw.githubusercontent.com/Decathlon/vitamin-web/main/packages/sources/icons/src/generated/vitamix/svg/home-fill.svg");

        width: 32;
        height: 32;
        background-color: black;

        -webkit-mask-image: var(--svg);
        mask-image: var(--svg);
    }

    .blue {
        background-color: blue;
    }

</style>

<body>
    <div class="icon"></div>

    <div class="icon blue"></div>
</body>

</html>

I haven't done a wide testing but, at least, it looks good in my:

chrome
safari
firefox

Tlahey commented 1 year ago

Indeed, it can be a good solution ! Tested with storybook and it works fine :)

jquinc30 commented 1 year ago

Regarding performance, even if the mask-image proposition is far better than the current implementation using font icons:

Load only used icon (instead of all at once)
Better caching (before, one icon change bursted the cache for all other icons in the sprite)

My favorite solution is still the <img src="xxx.svg" as it have the same benefits of above + also add the ability to:

Load only when required [enable native lazyloading]

Tlahey commented 1 year ago

By adding this lines in the default theme, I'm able to work with svg from image and changed the colors :

  --vtmn-semantic-color_content-primary-filter: invert(6%) sepia(13%) saturate(4111%) hue-rotate(160deg) brightness(99%) contrast(102%);
  --vtmn-semantic-color_content-secondary-filter: invert(36%) sepia(11%) saturate(835%) hue-rotate(168deg) brightness(90%) contrast(87%);
  --vtmn-semantic-color_content-tertiary-filter: invert(48%) sepia(9%) saturate(830%) hue-rotate(170deg) brightness(92%) contrast(83%);
  --vtmn-semantic-color_content-action-filter: invert(18%) sepia(54%) saturate(5673%) hue-rotate(186deg) brightness(93%) contrast(101%);
  --vtmn-semantic-color_content-active-filter: invert(29%) sepia(97%) saturate(1219%) hue-rotate(174deg) brightness(99%) contrast(101%);
  --vtmn-semantic-color_content-inactive-filter: invert(66%) sepia(12%) saturate(348%) hue-rotate(173deg) brightness(89%) contrast(83%);
  --vtmn-semantic-color_content-negative-filter: invert(18%) sepia(46%) saturate(5676%) hue-rotate(345deg) brightness(105%) contrast(91%);
  --vtmn-semantic-color_content-warning-filter: invert(68%) sepia(74%) saturate(6607%) hue-rotate(355deg) brightness(105%) contrast(102%);
  --vtmn-semantic-color_content-positive-filter: invert(39%) sepia(13%) saturate(5969%) hue-rotate(101deg) brightness(117%) contrast(73%);
  --vtmn-semantic-color_content-information-filter: invert(33%) sepia(59%) saturate(1491%) hue-rotate(171deg) brightness(99%) contrast(101%);
  --vtmn-semantic-color_content-accent-filter: invert(6%) sepia(55%) saturate(1278%) hue-rotate(163deg) brightness(92%) contrast(103%);
  --vtmn-semantic-color_content-visited-filter: invert(36%) sepia(7%) saturate(2155%) hue-rotate(247deg) brightness(87%) contrast(89%);
  --vtmn-semantic-color_content-primary-reversed-filter: invert(100%) sepia(100%) saturate(0%) hue-rotate(0deg) brightness(100%) contrast(100%);
  --vtmn-semantic-color_content-action-reversed-filter: var(--vtmn-semantic-color_content-primary-reversed-filter);
  --vtmn-semantic-color_content-visited-reversed-filter: invert(74%) sepia(13%) saturate(638%) hue-rotate(247deg) brightness(79%) contrast(89%);
  --vtmn-semantic-color_content-background-brand-primary-filter: var(--vtmn-semantic-color_content-information-filter);

I need to create this vars also for the dark mode

Edit: For dark mode

  --vtmn-semantic-color_content-primary-filter: invert(5%) sepia(95%) saturate(751%) hue-rotate(162deg) brightness(96%) contrast(103%);
  --vtmn-semantic-color_content-secondary-filter: invert(95%) sepia(10%) saturate(90%) hue-rotate(169deg) brightness(94%) contrast(90%);
  --vtmn-semantic-color_content-tertiary-filter: invert(47%) sepia(18%) saturate(400%) hue-rotate(170deg) brightness(94%) contrast(92%);
  --vtmn-semantic-color_content-action-filter: invert(25%) sepia(30%) saturate(5532%) hue-rotate(180deg) brightness(95%) contrast(101%);
  --vtmn-semantic-color_content-active-filter: invert(28%) sepia(86%) saturate(1541%) hue-rotate(176deg) brightness(101%) contrast(101%);
  --vtmn-semantic-color_content-inactive-filter: invert(64%) sepia(10%) saturate(398%) hue-rotate(173deg) brightness(91%) contrast(87%);
  --vtmn-semantic-color_content-negative-filter: invert(21%) sepia(56%) saturate(3597%) hue-rotate(341deg) brightness(104%) contrast(99%);
  --vtmn-semantic-color_content-warning-filter: invert(45%) sepia(84%) saturate(3191%) hue-rotate(355deg) brightness(104%) contrast(101%);
  --vtmn-semantic-color_content-positive-filter: invert(59%) sepia(62%) saturate(5080%) hue-rotate(102deg) brightness(97%) contrast(73%);
  --vtmn-semantic-color_content-information-filter: invert(33%) sepia(100%) saturate(1209%) hue-rotate(174deg) brightness(87%) contrast(103%);
  --vtmn-semantic-color_content-accent-filter: invert(5%) sepia(41%) saturate(1959%) hue-rotate(167deg) brightness(93%) contrast(102%);
  --vtmn-semantic-color_content-visited-filter: invert(34%) sepia(7%) saturate(2309%) hue-rotate(247deg) brightness(89%) contrast(85%);
  --vtmn-semantic-color_content-primary-reversed-filter: invert(100%) sepia(99%) saturate(0%) hue-rotate(61deg) brightness(115%) contrast(100%);
  --vtmn-semantic-color_content-action-reversed-filter: var(--vtmn-semantic-color_content-primary-reversed-filter);
  --vtmn-semantic-color_content-visited-reversed-filter: invert(64%) sepia(8%) saturate(1027%) hue-rotate(247deg) brightness(92%) contrast(87%);
  --vtmn-semantic-color_content-brand-primary-filter: var(--vtmn-semantic-color_content-information-filter);

Tlahey commented 1 year ago

Lot of issues with VtmnIcon as svg.

All the css are based on span and class vtmx. We have to add css in order to takes also the img.
For components like VtmnButton, we can added icons for information but, with the police + span, the colors is inherited from the parent. With the img, it is not longer possible. We have to re-think the solution about that !

Tlahey commented 1 year ago

After a long discussion with @jquinc30 and @manglard , we think the move to svg is more complicated that expected.

TLTR : First, we have to move the project as type module for jest (because we have to used {#await synthax into svelte and svelte-jester doesn't like that). The other solution should be remove all tests, but it's not possible. For more information, please check https://github.com/svelteness/svelte-jester and the part "In ESM mode"

Second, 2 ways to injects the SVG.

We can just used the src from the await in order to inject the svg as img. With this solution, we can only apply a color with a filter (see topics above). On top of that, we have to change also all the CSS of the existing components in order to change the way to stylized the icons. (⚠️ today, the style applied on icons is only for a span with vtmx typo).
We can also used the src from the await and fetch this data in order to apply the src on the document. With this solution, the bad point is, we have to used {@html in order to inject the svg. But we can apply style without filter ! Also on the same way that the first point, we have to changed all the CSS of the existing components in order to apply the correct style / color

Third, the existing components CSS

All the existing components used CSS in order to inject icons with span / font. There are no existing CSS to inject icons as SVG / img.

Conclusion : Move the current VtmnIcon to SVG is a long process and will be a hard rework. But we can think about that with vtmn-play in order to enhancer the perf on the images.

I will open a thread on the vtmn-play project in order to add a VtmnIcon component which is import the svg and not the font.

For informations, ping @lauthieb @thibault-mahe

Decathlon / vitamin-web

feat: improve the `VtmnIcon` in order to used SVG #1445