Skip to content

feat: implement initial version of CSSInjectionMixin #8934

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

Open
wants to merge 65 commits into
base: main
Choose a base branch
from

Conversation

web-padawan
Copy link
Member

@web-padawan web-padawan commented Apr 11, 2025

Description

Part of vaadin/platform#7453

Added CSSInjectionMixin that allows injecting CSS into components using the following syntax:

html, :host {
  --vaadin-item-css-inject: 1;
}

@media vaadin-item {
  /* These styles go into shadow root */

  [part="content"] {
    /* styles */
  }
}

Above styles can be injected either in a document or in a shadow root of some parent component.

Type of change

  • Feature

Note

Can be tested in /dev/lumo-injection/ pages using this branch:
https://github.com/vaadin/web-components/tree/proto/css-injection

@web-padawan web-padawan requested a review from vursen April 11, 2025 12:15
@web-padawan web-padawan force-pushed the feat/css-injection-mixin branch from f897fe5 to b9e5a3d Compare April 11, 2025 12:18
@web-padawan web-padawan requested a review from tomivirkki April 11, 2025 13:10
@web-padawan
Copy link
Member Author

@tomivirkki Marked you as a reviewer for this PR, it's optional but your feedback would be highly appreciated.

Comment on lines 21 to 22
// Add the new stylesheet at the beginning of the adoptedStyleSheets array
component.shadowRoot.adoptedStyleSheets = [styleSheet, ...adoptedStyleSheets];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This relies on the assumption that the new base styles will use @layer but maybe we could improve this:

  • Update finalizeStyles() method in ThemableMixin to store styles from static get styles() separately
  • Have some unified helper that would then apply styles in the right order when updating instance styles:
    • first, styles from static get styles()
    • next, styles injected by CSSInjector
    • finally, styles added with registerStyles()

If this sounds reasonable, I will prototype this in a separate branch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UPD: implemented this logic in a new commit and updated tests accordingly.

@web-padawan web-padawan force-pushed the feat/css-injection-mixin branch from bcd96d8 to 2c34760 Compare April 14, 2025 08:11
}

// Observe custom property that would trigger injection for this class
this.#styleObserver.observe(this.#rootHost, cssInjectPropName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also be unobserved in componentDisconnected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that maybe we could unobserve root host when there are no more components added to it (especially if it's not the document but a shadow root host). This could be implemented in a follow-up PR.

import { CSSInjector } from './src/css-injector.js';

/**
* @type {string[]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Set<string>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants