ladle icon indicating copy to clipboard operation
ladle copied to clipboard

Styles inserted via CSSStyleSheet API don't survive cloning in iframe mode

Open bensmithett opened this issue 2 years ago • 6 comments

Describe the bug

When cloning style nodes in iframe mode (#196) styles inserted using the CSSStyleSheet API don't make it across.

I'm using Fela which uses CSSStyleSheet.insertRule.

StackOverflow answer that suggests looping through to copy each rule: https://stackoverflow.com/a/56156757

If I get some time before you get to this I'll poke around to see if there's a better way. Maybe something like grabbing document.styleSheets and using adoptedStyleSheets in the iframe might work?

Reproduction

https://stackblitz.com/edit/ladle-pxajhf?file=src%2Fbasic.stories.jsx

The text should be blue. When you set the story width it's red.

Environment

I don't think it's environment specific but I'm testing on

  • OS: macOS
  • Browser: chrome & firefox
  • Version: latest

bensmithett avatar Jul 29 '22 06:07 bensmithett

Yea, I had a similar issue with styletron.

I had to write a bit hacky code to "retarget" the engine so it starts adding styles to the correct document, you should be able to do something very similar since Fela is pretty much identical:

https://github.com/uber/baseweb/blob/master/.ladle/components.tsx#L23-L40

Frankly, I couldn't come up with a more elegant solution other than just reloading the page when turning on/off the iframe. If anyone has some ideas... adoptedStyleSheets looks interesting - it could simplify this code but you'll still need to handle the CSS in JS engine targeting the correct document after the switch. Btw, you can't observe mutations for CSSStyleSheet - so no ad-hoc synchronization is possible.

tajo avatar Jul 29 '22 08:07 tajo

Thanks for the pointer, here's how I adapted that for Fela in a way that lets you switch the frame on/off without needing to reload:

function getDocument(story) {
  const iframe = document.querySelector(`[title='Story ${story}']`)
  return iframe && iframe.contentDocument ? iframe.contentDocument : document
}

export const Provider ({ children, globalState }) => {
  const doc = getDocument(globalState.story)
  const felaRenderer = useMemo(() => createRenderer(), [doc])

  return (
    <RendererProvider renderer={renderer} targetDocument={doc}>
      {children}
    </RendererProvider>
  )
}

This could be nicer without needing to write the custom getDocument...

  • Possible to directly pass in the current story document on globalState or something?
  • ...or maybe even just put a consistent identifier on the iframe (e.g id='ladle-iframe') so you don't need to target the story title?

you'll still need to handle the CSS in JS engine targeting the correct document after the switch

I might be misunderstanding adoptedStylesheets but if the idea is that documents/shadow roots can share live updating stylesheets, you could possibly avoid that. Just always target the root document, then adopt in the iframe when you have an iframe? But it'd only work with constructed stylesheets which probably isn't much good for most people.

bensmithett avatar Jul 31 '22 00:07 bensmithett

Possible to directly pass in the current story document on globalState or something?

I had a version passing it into the Provider at one point. Also, there's probably only a single iframe anyway, so you could just query that.

I might be misunderstanding adoptedStylesheets but if the idea is that documents/shadow roots can share live updating stylesheets

That would be great. I will test it out. That would remove any need to custom setup CSS in JS libs.

tajo avatar Aug 01 '22 20:08 tajo

Tried to use adoptedStylesheets. It seems you can't grab <style> element from the parent and do iframe.document.adoptedStyleSheets = [styleElement.sheet].

It throws:

Uncaught DOMException: Failed to set the 'adoptedStyleSheets' property on 'Document': Can't adopt non-constructed stylesheets.

The adapted sheet needs to be created/shared through

new CSSStyleSheet()

So that rules out some type of seamless integration on the background. Not sure if most CSS in JS libraries even provide CSSStyleSheet instance directly instead of just adding style DOM nodes in a set container.

Also, this whole thing doesn't work in Safari.

tajo avatar Aug 04 '22 21:08 tajo

Good to know, appreciate you giving it a try 🙏

I'm happy with the workaround. Would you like me to PR a docs update mentioning it or something?

bensmithett avatar Aug 04 '22 22:08 bensmithett

Yea, I think it would nice to document it here with some examples: https://ladle.dev/docs/width

I am also considering to provide some onIframeChange callback passed through the Provider to simplify the user setup.

tajo avatar Aug 05 '22 00:08 tajo

I believe a project using Chakra UI which uses emotion for styles also suffers from this problem.

twhitbeck avatar Oct 14 '22 11:10 twhitbeck

I have a solution for this now, should work for all CSS in JS libs without any strange workarounds by users. Will be released in V3.

tajo avatar Sep 11 '23 06:09 tajo