-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Print all variables - fix #28581 #28585
Conversation
Size Change: -53 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Correct. Note that this assumes that the
|
Awesome, I was wondering what had regressed. And it's very possible that I broke most of this myself, as I tried updating my experimental-theme.json to the new structure. It looks like this, and as noted, this PR doesn't fix the issue for me. Can you spot any obvious errors in my syntax? Thank you Ari! |
The JSON file you posted appears to be correct... I couldn't spot any obvious errors there 🤔 |
I tested just now with the latest version of TT1 Blocks (which includes the new |
@kjellr I just tested it with TT1-blocks too and it didn't work, but then I tried it by moving the TT1-Blocks theme from |
Yeah, I'd originally tried it in a subfolder, but the template parts didn't load in the front end. Now I'm running it directly in |
It sounds like what I'm experiencing also. Is there a particular additional step to take in addition to restarting npm run dev? |
cc @carolinan for testing since you're working on the TT1-blocks theme 🙏 |
Update: On Firefox the PR works fine, Chrome is where the issue can be consistently replicated. |
In Firefox after all the |
OK, I'm officially stuck and can't understand what goes on here... cc @nosolosw, do you have any idea what goes on here and why Chrome loads |
I can confirm that it works in Firefox, and also that it doesn't work in Chrome, Safari, or Microsoft Edge on MacOS. CC: @ellatrix as I know you worked on the iframe. |
👋 just saw the pings and found #28581 I've taken a quick look at the different pieces at play (iframe, editor styles hook, global styles hook, server styles) and this is what I see:
It seems the root issue is that iframe takes its stylesheet from the global variable |
Looks like the styles are not available when the iframe loads. Adding a gutenberg/packages/block-editor/src/components/iframe/index.js Lines 155 to 161 in 541141d
makes chrome work. To be more specific, |
Just opened a companion PR. That should take care of the problem that I described above. |
I've pushed what I had locally (a work in progress fix) at #28731 We need a different fix than what's proposed here. |
I completely agree, this is a bad PR. Closing it now that we have a better solution 👍 |
This is a PR that has brought attention to an important issue and pushed us to discover the root cause we need to fix. So, I'd say it's a good PR that doesn't get the opportunity of landing in |
Description
Fixes #28581 - global styles not getting added in the site editor.
How has this been tested?
See instructions in #28581
Types of changes
Removed the conditions so css-variables are always printed.
Checklist:
My code follows the accessibility standards.My code has proper inline documentation.I've included developer documentation if appropriate.I've updated all React Native files affected by any refactorings/renamings in this PR.