vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
452 stars 83 forks source link

Auto-convert the existing set of icons to a 24px grid #316

Open probert94 opened 5 years ago

probert94 commented 5 years ago

I just tryed to use a custom icon-collection and noticed, that my icons were bigger then the vaadin-icons.
After searching for the reason for a while, I noticed, this:

/* Vaadin icons are based on a 16x16 grid (unlike Lumo and Material icons with 24x24), so they look too big by default */
[part] ::slotted(iron-icon[icon^="vaadin:"]) {
    padding: 0.25em;
    box-sizing: border-box !important;
}

The icons from the vaadin-collection get a padding of 0.25em, if used inside a vaadin-button, because they are based on a 16x16px grid instead of a 24x24px grid. So I changed my custom icons to use a 24x24px grid and they are now the same size, if used inside a vaadin-button. However, if I use only the icon or if I switch the theme to material, my icons are now smaller then the vaadin-icons.

So to use custom icons in the lumo-theme, you currently have to use a 16x16px grid and use a padding: 0.25em.

In my opinion, this styleshould not be part of the vaadin-buttons lumo-theme, as the different grid-size does not only affect the icons inside the vaadin-button and it does not only affect vaadin-buttons in lumo-theme.

web-padawan commented 5 years ago

Thanks for the issue. We do realise that this kind of CSS is kind of a workaround, to hide the complexity from Vaadin platform users (especially those using Flow counterparts in Java).

If you are only using Web Components part, and are not afraid of getting your hands dirty with some tweaks, I would recommend creating the custom element extension like this:

class IconButtonElement extends Vaadin.ButtonElement {
  static get is() {
    return 'icon-button';
  }
}
customElements.define(IconButtonElement.is, IconButtonElement);

Then, you could use style module like that:

<dom-module id="icon-button-styles" theme-for="icon-button">
  <template>
    <style include="lumo-button">
      /* custom CSS for icons goes here */
    </style>
  </template>
</dom-module>

In case if you are using Polymer 3, the process is pretty much the same, but you should use customElements.get('vaadin-button') to get the reference to the ButtonElement.

We will re-evaluate this issue and try to come up with better assumptions, but I guess we won't be able to drop this kind of styles completely for now. If you have any idea, feel free to submit a PR.

web-padawan commented 5 years ago

@jouni would be nice to get your feedback on this, what would be the better approach?

probert94 commented 5 years ago

@web-padawan I played around with the icons for a while now and I guess, the real issue are the vaadin-icons, the workaround in the vaadin-button seems legit.
However, this workaround should then probably be added to the material-themed vaadin-button too?

Anyways, the only, real solution to this problem would probably be to base them on a 24*24px grid, like most other icon-libraries do.

jouni commented 5 years ago

Anyways, the only, real solution to this problem would probably be to base them on a 24*24px grid, like most other icon-libraries do.

That has been the plan for quite some time, but it has been mixed up with a plan to redraw the icons as well, to take advantage of the larger size/grid. We might want to split those into separate versions, so that we first auto-convert the existing icons to a 24px grid and make a new major version. Then we can start working towards redrawing the icons. At the moment I feel like we should combine Lumo and Vaadin icons at that point.