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

Updates to Declarative Shadow DOM adopedStyleSheets #905

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

Conversation

KurtCattiSchmidt
Copy link

This PR updates the Shadow DOM stylesheet Sharing document based on feedback from TAG and WHATWG.

In particular:

  • The @sheet section is expanded
  • More declarative modules are spec'd out
  • A section on polyfills was added

Copy link
Member

@dandclark dandclark left a comment

Choose a reason for hiding this comment

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

LGTM with some minor feedback, thanks!

| Module type | Script Module | Declarative Module |
| -------------- | -------------------------------------------------------- | ------------------------------------------------------------------------------------|
| JavaScript | `import { foo } from "./bar.js";` | TODO |
| CSS | `import foo from "./bar.css" with { type: "css" };` | `<script type="css-module" specifier="/bar.css"> ... </script>` |
Copy link
Member

Choose a reason for hiding this comment

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

For all of these, the "Script Module" column shows how the module would be consumed, but the "Declarative Module" column demos how the module would be defined.
It might be more consistent to have the "Declarative Module" instead show how the module would be consumed, e.g. <template shadowrootmode="open" adoptedstylesheets="/foo.css">.

The specification of `@sheet` could be modified to split the *definition* of stylesheets from the *application* of the style rules. With this modification, `@sheet` would *define* a stylesheet with its own set of rules, but not *apply* the rules automatically. This would allow for defining stylesheets in a light DOM context and applying them only to the shadow roots.
With this behavior, the following example would have a a gray background and blue text only within the Shadow DOM:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
With this behavior, the following example would have a a gray background and blue text only within the Shadow DOM:
With this behavior, the following example would have a gray background and blue text only within the Shadow DOM:

</template>
</template>
```
Text within both shadow roots in the above example should be blue due to the `adoptedstylesheets` at each Shadow DOM layer. Note that it is not currently possible to export stylesheets *out* of shadow roots.
Copy link
Member

Choose a reason for hiding this comment

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

Something I noticed here is that this approach doesn't separate the action of applying of the sheet to a shadow and of making it available to nested shadows. So, say I have a shadow A and A contains a nested shadow B. If I want to define a style in the light DOM and apply it only to shadow B, I can't do that; to make it available to B I have to pass it to A in adoptedstylesheets which will also apply it to A.

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't need to block sharing this doc but is maybe worth a note; we have more thinking to do here.

Choose a reason for hiding this comment

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

From offline chat - Requiring @import in shadow DOM (same as light DOM) seems like it'd solve this, and make the usage feel more consistent.

@@ -348,7 +425,7 @@ This proposal will add a new markup-based `adoptedstylesheets` property that clo
:host {
color: red
}
</style>
</style>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
</style>
</style>

(nit- stray space)

@@ -362,7 +439,7 @@ Web authors can use the `adoptedstylesheets` property on the `<template>` elemen
<!-- -->
</template>
```
One requirement of this approach is that the current `adoptedStylesheets` JavaScript property would need to lift the “constructable” requirement for `adoptedStylesheets`. This was recently agreed upon by the CSSWG but has not been implemented yet: [ Can we lift the restriction on constructed flag for adoptedStylesheets?](https://github.com/w3c/csswg-drafts/issues/10013#issuecomment-2165396092)
One requirement of this approach is that the current `adoptedStylesheets` JavaScript property would need to lift the “constructable” requirement for `adoptedStylesheets`. This was recently agreed upon by the CSSWG but has not been implemented yet: [ Can we lift the restriction on constructed flag for adoptedStylesheets?](https://github.com/w3c/csswg-drafts/issues/10013#issuecomment-2165396092)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
One requirement of this approach is that the current `adoptedStylesheets` JavaScript property would need to lift the “constructable” requirement for `adoptedStylesheets`. This was recently agreed upon by the CSSWG but has not been implemented yet: [ Can we lift the restriction on constructed flag for adoptedStylesheets?](https://github.com/w3c/csswg-drafts/issues/10013#issuecomment-2165396092)
One requirement of this approach is that the current `adoptedStylesheets` JavaScript property would need to lift the “constructable” requirement for `adoptedStylesheets`. This was recently agreed upon by the CSSWG but has not been implemented yet: [ Can we lift the restriction on constructed flag for adoptedStylesheets?](https://github.com/w3c/csswg-drafts/issues/10013#issuecomment-2165396092)

stray space

```html
<script>
function supportsDeclarativeAdoptedStyleSheets() {
return document.createElement('template')["adoptedStyleSheets"] == undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return document.createElement('template')["adoptedStyleSheets"] == undefined;
return document.createElement('template')["adoptedStyleSheets"] != undefined;

```
For the Declarative CSS Modules approach, [one suggestion](https://github.com/whatwg/html/issues/10673#issuecomment-2453512552) is to bind the `<link>` tag's `href` attribute value to the CSS module identifier and add a new attribute (`noadoptedstylesheets`) to avoid double-applying stylesheets.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For the Declarative CSS Modules approach, [one suggestion](https://github.com/whatwg/html/issues/10673#issuecomment-2453512552) is to bind the `<link>` tag's `href` attribute value to the CSS module identifier and add a new attribute (`noadoptedstylesheets`) to avoid double-applying stylesheets.
There was also a [suggestion](https://github.com/whatwg/html/issues/10673#issuecomment-2453512552) for adding browser support to enable falling back to a normal `<link>` tag without the use of script, by binding the `<link>` tag's `href` attribute value to the CSS module identifier and adding a new attribute (`noadoptedstylesheets`) to avoid double-applying stylesheets.

Optional suggestion for clarifying the purpose of this alternative.

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.

3 participants