BlockList: Ensure element styles (and svg) are always appended at the end of the document #53859
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
added
[Type] Bug
andrewserong
changed the title
|
Size Change: +7 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Aug 22, 2023
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
This looks good to me but I'd like for @ellatrix to confirm here.
The other thing here is that, there's no a small chance that this extra div impacts CSS that is injected by the theme, things like last-child selectors... Maybe the percentage of themes this impacts is small?
Contributor
Author
|
Thanks for taking a look @youknowriad!
True — I think the likelihood is probably fairly minimal, and on balance I think the behaviour with this PR is probably preferable and keeps the injected styles and svgs in a consistent place at least. My main thinking here is that this extra div is now a sibling of 
Agreed, it'd be good to get another pair of eyes on this if we can. Since this PR is a change that resolves a fairly subtle issue, and in turn could cause other fairly subtle issues, to be on the safe side I'll just give a wider ping to @WordPress/gutenberg-core in case there's anyone who has further thoughts about the markup here 🙂 |
andrewserong
force-pushed
the
fix/ensure-element-styles-are-always-output-at-the-end-of-the-document
branch
from
0e81a9c to
430b991
Compare
Contributor
Author
|
Just gave this a fresh rebase to fix a conflict after #53875 landed, so this should be good to review again now. |
Member
|
There's also the #52888, which proposes a new system for outputting styles and SVGs. |
youknowriad
approved these changes
Aug 29, 2023
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
Approving this cause overall it's seems like an improvement for the current way we do things. We can look at the further improvements separately.
Member
|
Sure this is fine, but outputting extra divs. You all know how much I hate extra divs 😅 |
Contributor
Author
|
Thanks for the reviews and feedback, folks! I'll merge this in now since it fixes the immediate issue for the moment.
Agreed, let's take a closer look at #52888 for a better long-term solution 👍 |
andrewserong
deleted the
fix/ensure-element-styles-are-always-output-at-the-end-of-the-document
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes #36053
Update the
BlockList'sRootcomponent to always output element styles (e.g.styleandsvgelements for layout, element, and duotone block supports) to the end of the document. To do so, add an extradivto the end ofRootto ensure that any changes to theuseInnerBlocksPropselement occur prior to the injectedstyleandsvgtags.Why?
The reason this change is needed is fairly subtle. React's
createPortalappends nodes as the list child of an element. In an already loaded post or page, the inner blocks for the block list will all be rendered at the top, and the injectedstyletags will appear at the bottom of the DOM.Something interesting happens when we go to make changes, though. When hovering over the end of the document and going to insert blocks, the block appender will be rendered after the injects
styletags in the DOM. Then, when we go to insert blocks, inserted blocks will also be rendered after the injected styles. Whereas, what we expect to happen is that the styles would still appear at the end.The reason why, I think, is because after the original injection, when React rerenders elements into the portal, it doesn't attempt to remount into the portal, rather it makes updates in place since the elements have already been rendered into the DOM. This means that subsequent additions are added to the end, but we wind up in an unexpected situation where
styletags wind up "randomly" sitting between other blocks once we go to make changes to blocks within a document.To avoid this collision between an element where lots of mutation is happening due to addition, removal, or moving of blocks, and the injection of styles and svg tags, a solution is to create a separate
divas the entrypoint for portal-based injection of style and svg tags, and have this always sit at the bottom of the document.For further explanation as to why the style and svg tags sitting between blocks is an issue, see the discussion in: #36053 — the TL;DR is that the presence of these tags can break CSS selectors as such sibling, first / last child, etc.
How?
divto the bottom of theRoot.setElementref state function to this node.Testing Instructions
styletags in adivat the end of the document (beneath the block content).styletags that contain thewp-container-123etc rules.hiddenattribute to the containerdiv, because Firefox requires the container to be visible in order for thesvgelements to be accessible for rendering Duotone). As far as I can tell, the current state of this PR doesn't cause any styling issues, but it would be good to confirm via manual testing.Here is some test markup to begin with (you'll need to add your own Image blocks with Duotone for testing):
Testing Instructions for Keyboard
Screenshots or screencast
The below screenshots show the in editor markup for a post just after a paragraph block has been inserted at the end of the document.
Note that in the "before" screenshot, there is a Figure element for the image block, then a series of
styleandsvgtags, then the paragraph block, then the block appender.In the "after" screenshot, in the top area there is the Figure element for thee image block, then the paragraph block, then the block appender. And below all of them is a separate
divto contain all thestyleandsvgtags.