-
Notifications
You must be signed in to change notification settings - Fork 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
Assembler: Unset white background (try 2) #89381
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~306 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and looks good to me 👍
How about removing the style from the head
element and re-adding them like the previous approach?
In the previous approach I had to use This also causes a regression issue where the pattern list preview would not have the background color applied. Another approach we could do is to use JavaScript to remove the <style> injected by the Iframe component then re-add it, but I think the solution is not great either. WDTY? |
This is what I mean 😆 I'm fine with both and I assume it could be resolved after upgrading the package soon! |
Let's merge this quick fix first, and we can time-box the other proposed solution 🙂 |
Fixes #89324
Proposed Changes
This PR fixes the issue where the background colors from global styles are not applied in the block renderer in the Assembler. This is caused by https://github.com/WordPress/gutenberg/pull/59377/files, where the background is explicitly set to white.
I don't feel great about this approach due to the performance penalties and that we are imposing CSS specificity, but since it's a short-term solution, I think it's okay we'll just remove it at a later date.
Note that the issue only affects Safari.
Testing Instructions
Pre-merge Checklist