Default rendering of icons is too tall

[chriscalo]

Say we have the following <template>:

<template>
  <button>
    <FileUploadOutline width="20" height="20"/>
    <span>Import Transactions</span>
  </button>
</template>

The icon renders roughly the following markup:

<span role="img" class="mdi">
  <svg>
    …
  </svg>
</span>

Unfortunately, the icon doesn't end up being 20×20 in size. It ends up slightly larger in height. The <svg> element has the correct dimensions, but the containing <span> ends up being a little taller at 23px.

It seems this is due to the <svg> defaulting to display: inline and vertical-align: baseline. This means when rendered in an inline layout context, the bottom edge of the <svg> is lifted up until it aligns with the baseline of the line of text that it is on. In my opinion, this is almost never what you want for icons.

I found two simple fixes that make this artifact go away:

.mdi {
  display: flex;
}

or:

.mdi > svg {
  vertical-align: middle;
}

It seems reasonable that these icons should:

1. default to rendering as a perfect square, and
2. always render at the exact dimension passed in via the width and height props.

The CSS above seems to fix this, and it would be great if the icons had this styling by default so users wouldn't have to apply it themselves.

Thoughts?

[therufa]

Hi @chriscalo, Thanks for your remarks and the recommendation for improvement! I'll take a look at this issue and get back to you ASAP.

[chriscalo]

Thanks, @therufa. Don't rush on my account. I have a simple enough workaround in place and just wanted to suggest an improvement.

It's probably worth thinking through the implications like:

• how easily can this be overridden or turned off?
• backwards compatibility: upon upgrading, might people see the new layout behavior as "something broke"?

[therufa]

Hi @chriscalo , I took your solution to the problem and added the styles you suggested to icons.css. It should be available now in version 1.4.5 of the package. Thanks for your contribution and your patience 😅

[chriscalo]

Thanks for the quick fix!

[chriscalo]

I just noticed that you added both CSS rules. From my brief testing a month ago, I think only one of the two is needed.

Related question: It appears the CSS isn't included by default. Is it a bad idea to apply these style fixes for all users of the library for any reason? I was thinking it might work to include a <style src="./path/to/icons.css"/> in template.js. Thoughts?