css-anchor-positioning
css-anchor-positioning copied to clipboard
Allow polyfilling imperatively for a single style element
Description
This change provides an alternative way for authors to apply polyfill: instead of letting the polyfill to lookup stylesheets and
apply the polyfill, the second argument takes a <style> element and the function can apply polyfill based on its content.
This is particularly useful for components like custom elements, as often the stylesheets are added dynamically to the document, and the currently "auto" polyfilling makes it challenging to apply the changes at the right time.
Related Issue(s)
https://github.com/oddbird/css-anchor-positioning/issues/228
Deploy Preview for anchor-position-wpt canceled.
| Name | Link |
|---|---|
| Latest commit | b4915fa8539651608cd5d75cfceddeb82b9bc343 |
| Latest deploy log | https://app.netlify.com/sites/anchor-position-wpt/deploys/670d51be6e94ea000870c2cc |
Deploy Preview for anchor-polyfill ready!
| Name | Link |
|---|---|
| Latest commit | b4915fa8539651608cd5d75cfceddeb82b9bc343 |
| Latest deploy log | https://app.netlify.com/sites/anchor-polyfill/deploys/670d51beb5ed570008254c4f |
| Deploy Preview | https://deploy-preview-256--anchor-polyfill.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
I'm okay with this approach. I wonder if we want to accept an array of elements, and we could also support
<link>tags as well as<style>? Basically using the same functionality as the existing polyfill, but allowing users to opt into a specified list of CSS sources.
I think an array that supports <link> tags as well as <style> makes a lot of sense.
This does omit inline styles- could we add another boolean flag includeInline that defaults to true, and then adds the response of fetch:fetchInlineStyles()? Or since the inline style impact is scoped to just ones that include anchor, should we just always add the inline styles?
It would be nice to have a demo, but that's a bit of a challenge in the existing single page setup.
Thanks for the comments. I'll add them in.
I think an array that supports tags as well as
This does omit inline styles- could we add another boolean flag includeInline that defaults to true, and then adds the response of fetch:fetchInlineStyles()? Or since the inline style impact is scoped to just ones that include anchor, should we just always add the inline styles?
Sounds good. I'll pass in the element array into both fetchCSS() and fetchInlineStyle() to build the styleData object.
I updated the code and it's ready for review:
- Changed the argument of
polyfill()to accept an object for configuration, with backward compatibility - You can pass in an array of elements for polyfilling, and imperative polyfill uses the same functions as auto polyfill
- Added demo and tests
Also, maybe I should rename "imperative" to "manual"? Let me know if "manual" sounds better and less confusing.
Is it a legitimate use case for people to pass in explicit
I think that's a valid use case. I'll add this. My only concern is, if the elements already contains some elements that have inline styles, then you pass in includeInlineStyles: false, an author may expect the elements with inline styles in elements not get polyfilled. But this can probably be clarified by documentation. Or maybe we can give includeInlineStyles a different name for more clarity.
Do you have any concerns about running this on 2 discrete sets of elements separately? It seems like it should work, as long as there aren't references between the 2 sets of elements.
Yeah this should work, and this is our intended use since multiple components in our library would need to manual polyfill separately. I also added some test coverage: https://github.com/oddbird/css-anchor-positioning/pull/256/commits/912c553c6e03502324ce9e4575e766321a4cff06.
The only caveat though, is that, if the elements in different sets rely on the same anchor-name, then all polyfill() calls need to include the element that contains this anchor-name declaration. I'm not sure if there's a map somewhere that keeps a record of the anchor name and the generated custom property name. It would be nice to be able to reference an already transformed anchor name.
Added includeInlineStyles: https://github.com/oddbird/css-anchor-positioning/pull/256/commits/798561f9dbe8d38ac5c740d3fb3aaafd2de0b0b3
Is it a legitimate use case for people to pass in explicit
I think that's a valid use case. I'll add this. My only concern is, if the
elementsalready contains some elements that have inline styles, then you pass inincludeInlineStyles: false, an author may expect the elements with inline styles inelementsnot get polyfilled. But this can probably be clarified by documentation. Or maybe we can giveincludeInlineStylesa different name for more clarity.
I have a slightly different thought on how this could work that may make it more clear (either way, documentation will be useful).
We could call it discoverInlineStyles or transformInlineStyles or ..., have it default to true, and separate it from whether or not elements are passed in. In other words, we default to always finding all the inline styles, but a user can opt out because they are passing in the elements they know have inline styles OR because they know they don't have any inline styles with *anchor.
Would that be simpler for users?
Is it a legitimate use case for people to pass in explicit
I think that's a valid use case. I'll add this. My only concern is, if the
elementsalready contains some elements that have inline styles, then you pass inincludeInlineStyles: false, an author may expect the elements with inline styles inelementsnot get polyfilled. But this can probably be clarified by documentation. Or maybe we can giveincludeInlineStylesa different name for more clarity.I have a slightly different thought on how this could work that may make it more clear (either way, documentation will be useful).
We could call it
discoverInlineStylesortransformInlineStylesor ..., have it default to true, and separate it from whether or notelementsare passed in. In other words, we default to always finding all the inline styles, but a user can opt out because they are passing in the elements they know have inline styles OR because they know they don't have any inline styles with*anchor.Would that be simpler for users?
Yeah agreed, this would align better with when there's no argument passed in. But maybe we can name the option as excludeInlineStyles? So when it's not specified, it's falsy.
- If you click the imperative button, and then apply the polyfill to the whole page, the polyfill crashes. This is somewhat related to [BUG] Polyfill fails if a
<link>fails to load or returns non-CSS content #255 as well as applying the polyfill to dynamic elements #91, so I'm not concerned here.
I'm seeing this too. I think it relates to #253, as the error I see seems to be when the polyfill tries to fetch the already-polyfilled style:
Security Error: Content at http://localhost:3000/ may not load data from blob:http://localhost:3000/8749e98f-f55a-4153-9249-f6bba301e298.
I'm not sure if there's an easy way to read/fetch a blob url, or whether we should explicitly just skip those links?
- If you click the imperative button, and then apply the polyfill to the whole page, the polyfill crashes. This is somewhat related to [BUG] Polyfill fails if a
<link>fails to load or returns non-CSS content #255 as well as applying the polyfill to dynamic elements #91, so I'm not concerned here.I'm seeing this too. I think it relates to #253, as the error I see seems to be when the polyfill tries to fetch the already-polyfilled style:
Security Error: Content at http://localhost:3000/ may not load data from blob:http://localhost:3000/8749e98f-f55a-4153-9249-f6bba301e298.I'm not sure if there's an easy way to read/fetch a blob url, or whether we should explicitly just skip those links?
Not quite a fix but this would at least let other styles be polyfilled:
try {
// fetch css and add to array
const response = await fetch(data.url.toString());
const css = await response.text();
return { ...data, css } as StyleData;
} catch {
return { ...data, css: ''} as StyleData;
}
@jamesnw @jgerigmeyer Thanks for the reviews, both. Have addressed all comments. Ready for another round :)
This works great! @marchbox I opened PR #259 to update the docs with the new configuration options- I'd appreciate your eyes on that to make sure it makes sense. Thanks!
Thank you! Will take a look at the documentation changes today.