Try: Hide the columns inserter in pattern previews. #36626
Merged
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
kjellr
reviewed
Nov 18, 2021
|
|
||
| // Hide inserter from previews. | ||
| // This selector could be cleaner if we had a CSS class to target inside the preview iframe. | ||
| html[style] body > .is-root-container .wp-block-columns { |
Contributor
There was a problem hiding this comment.
😅 Yeah, that's a weird stack of selectors. It would be way better if there were a more specific class we could tie into for the preview frame.
Contributor
Author
There was a problem hiding this comment.
It didn't feel right, so I just tried to push one that features a classname. But now I'd prefer a developer review of this one as well.
Contributor
There was a problem hiding this comment.
👍 Yeah, this new one feels much better.
|
Size Change: -3.21 kB (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
Comment on lines
6
to
10
|
|
||
| // Hide inserter from previews. | ||
| .block-editor-block-preview__content-iframe .wp-block-columns .block-list-appender { | ||
| display: none; | ||
| } |
Member
There was a problem hiding this comment.
Is there a reason this is being added to the styles of the long-deprecated Text Columns block and not the Columns block?
Contributor
Author
There was a problem hiding this comment.
Yes! The reason is I was moving too fast. Ack. I'll address this and Carolina's feedback below.
Contributor
|
I think the inserter should always be hidden in the preview, is the columns block the only block where it shows? |
Contributor
Author
|
Pushed a fix that removes it from text-columns (🙈) and adds it to the generic preview component styles. What do you think about the iframe class I added? |
Contributor
|
The new CSS is simpler -are there any known third party implementation (custom blocks?) concerns? |
Contributor
Author
Not that I can think of, unless a 3rd party block leverages classname called |
Contributor
Author
|
Is this close enough to land? |
kjellr
approved these changes
Nov 22, 2021
Contributor
kjellr
left a comment
kjellr
left a comment
There was a problem hiding this comment.
This works well, and seems simple enough. Thanks, @jasmussen!
| Before | After |
|---|---|
 |
 |
Contributor
Author
|
Cool, let's try this, and it's always easy to follow up. |
jasmussen
added
the
Backport to WP 6.9 Beta/RC
youknowriad
reviewed
Nov 22, 2021
| } | ||
|
|
||
| // Hide inserter from previews. | ||
| .block-editor-block-preview__content-iframe .block-list-appender { |
Contributor
There was a problem hiding this comment.
We already have a lot of similar styles above, why didn't we just nest this in .block-editor-block-preview__container above? instead of adding this new class?
Contributor
There was a problem hiding this comment.
I guess the fact that we introduced iframes for previews is preventing all these resets from applying, in that case, maybe we should move all these reset styles inside the iframe selector. cc @ellatrix
Contributor
Author
There was a problem hiding this comment.
I'm happy to try a followup here. But from what I can tell, we actually do need both the outer and the inner container. The outer sets things like border, border radius, pointer cursor, all unscaled. The inner is only for content inside the iframe, and as you found, needs to be unscoped.
Member
There was a problem hiding this comment.
Ideally we should be able to render a block list in a disabled mode that doesn't render any appenders. I thought we actually already did that because other appenders didn't show before in the preview.
noisysocks
removed
the
Backport to WP 6.9 Beta/RC
noisysocks
pushed a commit
that referenced
this pull request
Nov 23, 2021
* Try: Hide the columns inserter in pattern previews. * Try a classname instead. * Address feedback.
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.
Description
Fixes #36625. This adds a rule to the columns block editor stylesheet to hide the inserter when seen inside the preview iframe:
For now this is a small PR that fixes it for the columns block, which seems to be the primary offender. The PR could be refactored by adding a small CSS class to the preview component, so the selector to target only the contents of the preview could be simplified. (If you see this and feel lucky, you can push directly to this branch if you like.)
How has this been tested?
Try opening the inserter and going to patterns, you should see no inserter in the Forest pattern as shown.
Checklist:
*.native.jsfiles for terms that need renaming or removal).