web-components icon indicating copy to clipboard operation
web-components copied to clipboard

Add a replacement for <custom-style>

Open tomivirkki opened this issue 4 years ago • 8 comments

The <custom-style> element, used for including main document-level styles, is a Polymer Web Component and so needs a replacement.

Example:

<custom-style>
  <style include="lumo-color"></style>
</custom-style>

It's already possible to use ESM for importing Vaadin styles and then include them in the main document with js, but the end result is quite verbose:

import { color } from '@vaadin/vaadin-lumo-styles';

const tpl = document.createElement('template');
tpl.innerHTML = `<style>${color.toString()}</style>`;
document.head.appendChild($tpl.content);

One option would be to extend registerStyles to also support main document-level styling:

import { registerStyles } from '@vaadin/vaadin-themable-mixin';
import { color } from '@vaadin/vaadin-lumo-styles';

registerStyles(undefined, color);
or
registerStyles('document', color);
or
registerStyles(document, color);

tomivirkki avatar Sep 27 '21 13:09 tomivirkki

Indeed, for this we implemented a registerGlobalStyles() in registerStyles() spirit

https://github.com/conversionxl/aybolit/blob/a75e8e67c35d99c80aa92fdea16d14dae6a11fdc/packages/cxl-lumo-styles/src/utils.js

lkraav avatar Sep 29 '21 09:09 lkraav

See also #499

web-padawan avatar Sep 29 '21 09:09 web-padawan

I wonder if this should also support using ApplyShim (for legacy migration cases)?

Haprog avatar Sep 29 '21 10:09 Haprog

I wonder if this should also support using ApplyShim (for legacy migration cases)?

We do not use CSS mixins in our components and do not recommend anyone to use them. IMO there is no need for it at the moment, and hopefully we can avoid doing that.

web-padawan avatar Sep 29 '21 11:09 web-padawan

Also note that Polymer's <custom-style> will continue to work as before (assuming it gets imported).

If someone specifically wants to use <custom-style> with <style include="some-vaadin-style"> they can still do so by importing the style-modules adapter.

tomivirkki avatar Sep 29 '21 14:09 tomivirkki

Was struggling a bit today trying to phase out iron-icon in favor of vaadin-icon.

https://discord.com/channels/732335336448852018/1060531737794453575/1060531737794453575

Finally figured out with less Polymer loaded, some <custom-style> elements also needed a refactor.

  1. It's a bit unclear, why https://github.com/vaadin/web-components/blob/3f5113c82ebf30a229eecb7a7319cbf5ddf9160f/packages/vaadin-lumo-styles/typography.js#L138-L140 import @vaadin/vaadin-lumo-styles/typography.js does inject its <style> tag into head element, but it doesn't have any actual effect.

  2. From opener here One option would be to extend registerStyles to also support main document-level styling:

If this would land, I'd be able to retire our registerGlobalStyles util then.

lkraav avatar Jan 05 '23 13:01 lkraav

@lkraav Thanks for the feedback. I agree that the current code looks confusing. Here is what you would need to do:

https://github.com/vaadin/web-components/blob/99e4df4b7a81b61c9fde564c7004cef229fcd505/packages/vaadin-lumo-styles/test/autoload.js#L8-L10

web-padawan avatar Jan 06 '23 07:01 web-padawan

This can be addressed by the following helper added in #5666 (currently marked as internal):

https://github.com/vaadin/web-components/blob/212637f447da1d6ef3a696ac274df9659becbebf/packages/vaadin-themable-mixin/register-styles.js#L14-L23

web-padawan avatar Apr 02 '24 12:04 web-padawan