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

Support styles outside of style tag #78

Closed
jh3y opened this issue Sep 23, 2022 · 12 comments
Closed

Support styles outside of style tag #78

jh3y opened this issue Sep 23, 2022 · 12 comments

Comments

@jh3y
Copy link
Contributor

jh3y commented Sep 23, 2022

Currently using the polyfill with CSS styles only works if the styles are inlined into the head:

<style>
  .trigger { view-timeline-name: --trigger; }
  .el { animation-timeline: --trigger; }
</style>

If you put those styles in a file like style.css and link them with:

<link rel="stylesheet" href="style.css" />

The polyfill fails silently because it doesn't consider loaded styles.

Until that's supported, might it be worth adding a console.warn that any styles not inlined won't be detected? Happy to open a PR for this.

@jh3y

@AndrewKGuan
Copy link

Does it require all styles to be inline or just the ones that are related to the animating elements? But either way, may need support to external stylesheets for larger scaled sites to make this polyfill useable.

@bramus
Copy link
Collaborator

bramus commented Aug 18, 2023

Just the scroll-animation related ones.

@clementmas
Copy link

You should include this crucial info in the readme.

@webGeniusDK
Copy link

I have all animation-timeline related css inside style tags in the head of my page (lots of other external css), but I still get the warning: "Non-Inline StyleSheets detected: ScrollTimeline polyfill currently only supports inline styles within style tags"

@bramus
Copy link
Collaborator

bramus commented Oct 2, 2023

That’s expected behavior (for now): the polyfill detects that you have external stylesheets, so warns you about it potentially not working. Awaiting actual support for parsing of external stylesheets, the wording could use a minor tweak to make it more clear that things could potentially not work.

@webGeniusDK
Copy link

Ok.. Thanks. So you're saying that the polyfill might not work at this moment, if the document has other external stylesheets, with styles not related to this at all? (Just need to understand. I am very grateful for you work)

@kevers-google
Copy link
Collaborator

If the styles in the external stylesheet are not related to animations, then it should not be a problem; however, we show the warning since unless parsed via the polyfill, we simply don't know if there are animation related properties lurking inside.

The external stylesheet is currently problematic if it contains any of the following properties:
animation-timeline <-- browsers not supporting scroll-driven animations expect this to be a document timeline.
animation-range <-- the property is exclusive to scroll-driven animations
animation-name <-- used as a hook to intercept the start of the animation and swap out its timeline.

The animation-name property being problematic implies that the animation shorthand is as well for the same reason.

Finally, timeline offsets in keyframes are problematic in external stylesheets, as these keyframe rules cannot be applied without reinterpretation by the polyfill.

I hope this clarifies things.

@webGeniusDK
Copy link

Ok... removed all animation related styles in external stylesheets, and still it doesn't work. Just writing this so you know it. I will wait untill your work is further ahead. Thanks again!

@calinoracation
Copy link
Contributor

calinoracation commented Jan 18, 2024

I've started a super naive exploration into experimentally parsing the external stylesheets. Can't say it's actually working on my test yet, it parses much of it, but isn't exactly fully working yet.

function handleLinkedStylesheet(sheet: HTMLLinkElement) {
  fetch(sheet.href).then(async (response) => {
    const newSheet = new CSSStyleSheet();
    const result = await response.text();
    let newSrc = parser.transpileStyleSheet(result, true);
    newSrc = parser.transpileStyleSheet(newSrc, false);
    document.adoptedStyleSheets = [...document.adoptedStyleSheets, newSheet];
    sheet.remove();
  });
}

document.querySelectorAll('link').forEach((tag) => handleLinkedStylesheet(tag));

// and also above that by the <style> tag handler
if (addedNode instanceof HTMLLinkElement) {
   handleLinkedStylesheet(addedNode);
}

@bramus
Copy link
Collaborator

bramus commented Feb 9, 2024

Can't say it's actually working on my test yet, it parses much of it, but isn't exactly fully working yet.

The logic in the code itself looks good. What I think is preventing it from working is the fact that the CSS Parser included in the source can’t deal with some nested structures. See #133, #100, and #80 for examples where it fails.

Note that even in case the parser would properly support the entire CSS syntax, the polyfill still wouldn’t start working in all cases because to properly do this you basically need to implement an entire cascade, watch media queries, etc. See The Dark Side of Polyfilling CSS for an talk/post on this.

Furthermore, note that the code won’t work with cross-origin (non-CORS) stylesheets because the browser prevents you from fetching these.

@flackr
Copy link
Owner

flackr commented Feb 9, 2024

I haven't fully tested this out, but I uploaded a PR which is basically a version of #78 (comment) with some modification to avoid attempting to fetch cross origin stylesheets which are likely to fail.

@calinoracation
Copy link
Contributor

@flackr I wonder if an allow list would be possible? For our use we serve from a different origin for stylesheets but have cors setup to support it. Perhaps a way to manually initialize the polyfill with that list could work?

Was thinking especially now that vite is in place that a little sprinkle of types might enable that type of thing.

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

No branches or pull requests

8 participants