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

Site Editor: Emotion-based CSS-in-JS styles don't appear to be loaded into the editor canvas #30553

Closed
andrewserong opened this issue Apr 7, 2021 · 5 comments · Fixed by #31010

Comments

@andrewserong
Copy link
Contributor

Description

It looks as though the CSS-in-JS based styles generated for components via Emotion aren't being copied to the editor canvas in the FSE site editor. As an example, in the Jetpack plugin, components like ExternalLink (updated to use CSS-in-JS in #25751) are used frequently for the placeholder state of blocks, however the Emotion styles don't appear to be copied over to the editor iframe in use in the site editor. A good example is the external link icon used in the Jetpack Google Calendar block placeholder state, or the width of the message field in the Jetpack Contact Info block, which uses the TextareaControl component (which renders width: 100% via Emotion)

Is there a way for us to sync the Emotion rendered styles to the editor canvas? CC: @sarayourfriend since I saw you introduced CSS-in-JS to the ExternalLink component in #25751 and @ellatrix as I see you've done a lot of work on cloning styles to be loaded into the iframe in the editor.

Step-by-step reproduction instructions

Expected behaviour

Components rendered in the placeholder state for blocks in the site editor to have consistent styling as to how they're rendered within the post/page editor.

Actual behaviour

When rendered in the iframe in the site editor, the styles generated via the Emotion-based CSS-in-JS approach don't appear to be copied over.

Screenshots or screen recording (optional)

Before (in the post/page editor)

image

After (in the site editor)

image

Note: the large external link icon. In the post/page editor, the following CSS is active (but it doesn't look like it is in the site editor):

image

Example for the TextareaControl component

Note that the width: 100%; style generated by Emotion isn't being copied over:

Before (post editor) After (site editor
image image

Steps to recreate

  • On a site running the Gutenberg plugin, Jetpack plugin, and a blocks-based theme (e.g. TT1-blocks):
  • Add a post, and insert the Google Calendar block, and note the size of the external link icon
  • Add a Contact Form block, and note the size of the message field
  • Go to the site editor and insert the above blocks into a footer template part, and note that the styles for those components (ExternalLink, TextareaControl) aren't being copied over to the editor canvas.

WordPress information

  • Gutenberg version: 10.3.1
  • Are all plugins except Gutenberg deactivated? Gutenberg + Jetpack plugin (to use custom blocks)
  • Are you using a default theme (e.g. Twenty Twenty-One)? TT1-blocks

Device information

  • Device: Desktop
  • Operating system: macOS
  • Browser: Chrome 89
@sarayourfriend
Copy link
Contributor

sarayourfriend commented Apr 7, 2021

Emotion doesn't support styling iframes, however there's a PR open to integrate a new style system here #30509 that does support iframes through a new provider that gets rendered into the iframe, see the documentation here: https://github.com/ItsJonQ/g2/blob/97ef23ebe56a4968701a36ce6697f75803700134/packages/styles/src/components/style-frame-provider.js#L199

Would this be a suitable solution to this problem? I'm not sure what to do to solve this problem in the near/immediate term given Emotion's lack of iframe support.

Of course one solution is to revert the CSS-in-JS-ification of ExternalLink and other components, but that's a bandaid of course.

@andrewserong
Copy link
Contributor Author

Thanks for the update @sarayourfriend! Yes, I think that sounds like an ideal long-term solution if the styles are being copied into the iframe (looks quite clever!)

Of course one solution is to revert the CSS-in-JS-ification of ExternalLink and other components, but that's a bandaid of course.

That's a good point, if the timing on landing the new style system doesn't work out with getting plugins ready for the new full site editor, then it might be worth pursuing reverts for those common components just to get things working nicely. But, it'd be great to avoid the stop-gap if we don't have to.

Do you see the new style system landing / being integrated to these components fairly soon / within the full site editing testing timeframe, or is it more of a longer-term proposal?

@sarayourfriend
Copy link
Contributor

sarayourfriend commented Apr 8, 2021

I'm hoping it'll land pretty soon, but as you can see from the #30509 PR, there's a great deal of discussion. Converting everything to use the new style system is priority number 1 at the moment for the new style system and component system project, with some preliminary steps that will take us to get there.

The problem with reverting CSS-in-JS, however, is that some things were never written in SASS to begin wtih, like Card, Flex, Text, AnglePickerControl, to name a few (if I'm remembering the list correctly there's more to that too). So I don't think that's actually a viable solution for many use cases, but is for some.

Would like to pull in @youknowriad here probably just so that Riad is also aware of some urgency around fixing this problem. Maybe he has some other suggestions as well 🙂

@andrewserong
Copy link
Contributor Author

The problem with reverting CSS-in-JS, however, is that some things were never written in SASS to begin wtih, like Card, Flex, Text, AnglePickerControl, to name a few (if I'm remembering the list correctly there's more to that too). So I don't think that's actually a viable solution for many use cases, but is for some.

Great point @sarayourfriend. I don't think it'll be too hard for us to override some of these issues within Jetpack as a short-term fix for the affected blocks. Nice to see you got #30509 merged in, looks like things are coming along, so hopefully we won't need to do a direct revert in Gutenberg for those components 🙂🤞

@andrewserong
Copy link
Contributor Author

Thanks for fixing this up!

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 a pull request may close this issue.

3 participants