Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a replacement for <custom-style> #2642

Open
tomivirkki opened this issue Sep 27, 2021 · 8 comments
Open

Add a replacement for <custom-style> #2642

tomivirkki opened this issue Sep 27, 2021 · 8 comments
Labels
enhancement New feature or request no-polymer Removing Polymer from Vaadin public APIs

Comments

@tomivirkki
Copy link
Member

tomivirkki commented Sep 27, 2021

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 tomivirkki added the no-polymer Removing Polymer from Vaadin public APIs label Sep 27, 2021
@lkraav
Copy link
Contributor

lkraav commented Sep 29, 2021

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

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

@web-padawan
Copy link
Member

See also #499

@Haprog
Copy link
Contributor

Haprog commented Sep 29, 2021

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

@web-padawan
Copy link
Member

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.

@tomivirkki
Copy link
Member Author

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.

@lkraav
Copy link
Contributor

lkraav commented Jan 5, 2023

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

    const $tpl = document.createElement('template');
    $tpl.innerHTML = `<style>${font.toString().replace(':host', 'html')}</style>`;
    document.head.appendChild($tpl.content);
    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 added a commit to conversionxl/aybolit that referenced this issue Jan 5, 2023
@web-padawan
Copy link
Member

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

const style = document.createElement('style');
style.innerHTML = `${color.toString()} ${typography.toString()}`;
document.head.appendChild(style);

@web-padawan
Copy link
Member

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

export const addGlobalThemeStyles = (id, ...styles) => {
const styleTag = document.createElement('style');
styleTag.id = id;
styleTag.textContent = styles
.map((style) => style.toString())
.join('\n')
.replace(':host', 'html');
document.head.insertAdjacentElement('afterbegin', styleTag);
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request no-polymer Removing Polymer from Vaadin public APIs
Projects
None yet
Development

No branches or pull requests

4 participants