Size Change: +82 B (0%)

Total Size: 1.39 MB

_{compressed-size-action}

Flaky tests detected in 0522400b4c672ddd0f872df53c7c9b94c7e00e03.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4807099223
📝 Reported issues:

• [Flaky Test] Can edit the post title #35259 in specs/editor/blocks/post-title.test.js
• [Flaky Test] last-inserted block should be given and keep the focus (small viewport) #48464 in specs/editor/various/inserting-blocks.test.js
• [Flaky Test] passes the search value in the main inserter when clicking Browse all #45332 in specs/editor/various/inserting-blocks.test.js

Closes: #37947.

It seems like it's causing some tests to be flaky with the following ouput:

    expect(jest.fn()).not.toHaveWarned(expected)

    Expected mock function not to be called but it was called with:
    ["wp-block-library-theme-css was added to the iframe incorrectly. Please use block.json or enqueue_block_assets to add styles to the iframe. JSHandle@node"],["wp-block-library-theme-css was added to the iframe incorrectly. Please use block.json or enqueue_block_assets to add styles to the iframe. JSHandle@node"]

I don't know why this is only happening on some attempts but not all though. Will be nice to resolve this before merging 🙂 .

#50122 - I have a fix ready for the @wordpress/warning that isn't working correctly.

Note to self: check if this fixes #50246. It does not.

So, one of the goals of this PR is that editorStyles are always present in the iframe, and not only when they have .wp-block classes (compat layer). I've tested this by creating a plugin that has front & editor styles with no .wp-block-* classes. This is working as expected.

Once the test plugin was activated and the block added to the editor, I tested that:

• trunk only has the front-end styles in the iframed editor (create-block-blockjson-styles-no-block-class-style-css stylesheet).
• this branch has front-end & editor styles in the iframed editor (create-block-blockjson-styles-no-block-class-style-css and create-block-blockjson-styles-no-block-class-editor-style-css stylesheets, respectively)

I'll look into the other code changes and compare what's the stylesheet footprint (before/after).

@ellatrix I've noticed that we are running the same logic twice for setting __unstableResolvedAssets #51100 Would you mind reviewing that one (and merge it first) so we know that _gutenberg_get_iframed_editor_assets is the only place that resolves the styles?

For the warning, I tested by using this plugin which adds styles with and without .wp-block class using both enqueue_block_assets and enqueue_block_editor_assets. Installing and activating the plugin results in a warning being thrown for the stylesheet enqueue_block_editor_assets_style_wp_block, which is added via the enqueue_block_editor_assets hook and contains the .wp-block class. So, works as expected.

This is the stylesheet footprint for this PR compared to trunk. TLDR: everything seems fine.

Thanks for the review, @oandregal!

I'm getting these warnings.

was added to the iframe incorrectly. Please use block.json or enqueue_block_assets to add styles to the iframe. <style>​…​

Also for core styles:

Also for my theme style.css (enqueued via add_editor_style('style.css')).

@ellatrix

Now that enqueue_block_assets can be used, there should always be a way to add styles correctly.

What about dynamic styles?

My requirements are:

1. Add <style> tag with dynamic content for every block that needs it. Currently using BlockList.__unstableElementContext context to add the tag using createPortal() (this method is used in core so it seems to be the way to go about it). This places them to the end of .editor-styles-wrapper (with latest Gutenberg, when going from non-iframe to iframe mode (by previewing in tablet for example), it duplicates the styles to head as well and prints the warning added in this PR).
2. Add <style> to <head>. For example I query the current template, if that template contains a Container block I limit the .editor-styles-wrapper to appropriate width. If the template is changed, the style needs to change dynamically too. Furthermore the width of the container can be edited in settings, in fact, anything about it can be changed, padding, border, any CSS property, so it can't really be done with static css/variables and body classes.
3. Add <style> tag in the <head> before other styles, it's a reset, it has variables, it has filters, it's dynamic.

1 is very volatile and there can be hundreds of them, so running them through EditorStyles API, causing EditorStyles to re-render and re-transform everything when 1 block has a minor change, it would be very taxing.

Good solution for me would be to add slots to <EditorStyles />.
A slot before and a slot after.

Or the slots could be even more generic, for the start and end of the <head> tag, because one might want to add anything (dynamic) to the head, and currently it's a hassle with editors changing from non-iframe to iframe, and multiple contexts like the block preview/example popovers.

BlockList.__unstableElementContext could also be replaced with a slot, so in the block Edit component, along with InspectorControls and BlockControls you could use something like (Dynamic/Raw)BlockStyles, instead of useContext and createPortal:

Yes, that's a good point. How are you doing these things in a non iframed editor?

Whether the editor is iframed or not, I have a HOC for every block (inefficient but I can't think of other ways) using the editor.BlockEdit filter. The HOC uses useContext(BlockList.__unstableElementContext) to capture the ref to the BlockList node and keep every unique one in a global store.

Then I have registered a @wordpress/plugins plugin, that renders something like:

return <>
    <PluginSidebarMoreMenuItem ... />
    <PluginSidebar ... />
    <MyRenderStyles ... /> // this component renders everywhere
</>

And in MyRenderStyles component I use the nodes from the global store, filter out ones that are not isConnected, find the heads with .ownerDocument.querySelector('head') and use that to portal styles where I need them. So it works for every unique instance, such as the main editor, iframed responsive previews, popover previews, etc.

Would be much more simple if in MyRenderStyles I could use slot fills:

return <>
    <Fill name='blockEditor.head.start'>
        <style id="my-reset">
            {`.editor-styles-wrapper { margin: 0; background: ${defaultBackground}; }`}
        </style>
    </Fill>
    {currentTemplateHasContainer && (
        <Fill name='blockEditor.head.end'>
            <style id="my-styles">
                {`.editor-styles-wrapper { max-width: ${containerWidth}px; margin: 0 auto; }`}
            </style>
        </Fill>
    )}
</>

You add a HoC to every block? And your styles are just some dynamic "global" styles, right? A slot indeed seems to be the best solution. What styles are you resetting? Why do you need to reset styles?

What styles are you resetting?

I have a theme/plugin combo that uses Tailwind, so I use Tailwind preflight CSS reset. Plugin options allow to modify Tailwind config which regenerates the preflight (via a web worker similar to play.tailwindcss.com). There are also configurable Tailwind plugins used that can extend what the preflight contains (forms for example).

For example the prelight includes:

h1,
h2,
h3,
h4,
h5,
h6 {
	font-size: inherit;
	font-weight: inherit;
}

blockquote,
dl,
dd,
h1,
h2,
h3,
h4,
h5,
h6,
hr,
figure,
p,
pre {
	margin: 0;
}

But in my theme style.css I style headings:

h1, h2, h3, h4, h5, h6 {
	margin-top: 2rem;
	margin-bottom: 1rem;
}

and in theme.json I might have:

"styles": {
	"elements": {
		"h1": {
			"typography": {
				"fontSize": "3rem",
				"lineHeight": "1"
			}
		},

so it's paramount that the reset comes first.

I do run that CSS through @wordpress/block-editor transformStyles() of course.

Along with the preflight I also have dynamic CSS variables there. Not entirely sure if they need to be early in the head, but I think so. What if you want to override one in theme style.css, but the original vars are loaded after it?

This is why I think it's important to not only have the ability to add dynamic CSS to head, but also have some control about where it's placed.

Why do you need to reset styles?

I have it on the front end, having it in the back end helps iron out any inconsistencies. If it had to be static I could probably make it work but it being dynamic really simplifies things for me.

While there is wp-includes/css/dist/block-library/reset.css which resets most of the things, such as the styles that wp-admin/css/common.css adds, the preflight also adds things like

border-width: 0;
border-style: solid;
border-color: theme('borderColor.DEFAULT', currentColor);

which is needed for borders to work correctly with Tailwind classes.

But talking about dynamic, the example in my previous post:

{currentTemplateHasContainer && (
	<Fill name='blockEditor.head.end'>
		<style id="my-styles">
			{`.editor-styles-wrapper { max-width: ${containerWidth}px; margin: 0 auto; }`}
		</style>
	</Fill>
)}

is oversimplified, if I'm only setting max-width why don't I just use a CSS variable? In reality I'm applying the CSS generated from Tailwind class names px-4 sm:px-6 md:px-8 mx-auto w-full max-w-wp-content which is editable in the plugin options and by the nature of it being dynamic I can provide a live preview of the results.

For blog posts I apply mx-auto w-full prose prose-base sm:[main>&]:prose-lg prose-a:text-link which is a whole heap of CSS. And if I want to style something, the core/code block for example, I can add prose-pre:shadow-2xl (or [&_.wp-block-code]:shadow-2xl) and observe the change instantly which is very neat.

I do hope I've made a good enough case for the dynamic styles/slots/more control. Thanks for reading!

@ska-dev-1 Thinking more about this, you should be using the block editor settings to add styles dynamically. This already has the flexibility of appending or prepending styles. Unfortunately, it's broken right now, but my PR #52767 should fix it. Could you have look and give you feedback?

Styles look promising, how about fonts?

Currently I also render dynamic fonts the same way I do styles so I can add/remove/swap fonts with live preview

const url = `//fonts.googleapis.com/css2?family=${font.replaceAll(' ', '+')}${variantsQueryString}&display=swap`
return <link id={`ska-font-${id}`} href={url} rel='stylesheet' />

There seems to be a method for svgs:

Something similar could work for <link>s. A slot in the head would also work.

Could you use CSS imports?

Ah, yes, absolutely, didn't even cross my mind. Thank you!