Template Replace Flow: Avoid column break within the preview box#56505
Closed
t-hamano wants to merge 1 commit into
Closed
Template Replace Flow: Avoid column break within the preview box#56505t-hamano wants to merge 1 commit into
t-hamano wants to merge 1 commit into
Conversation
t-hamano
added
[Type] Bug
t-hamano
requested review from
Mamaduka,
draganescu,
ntsekouras and
scruffian
|
Size Change: +6 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Mamaduka
approved these changes
Nov 24, 2023
Member
There was a problem hiding this comment.
Thank you, @t-hamano!
I've never used break-inside: avoid-column before, but it definitely fixes the bug.
Update: I just noticed that the 4th column isn't rendered on certain sizes. The issue doesn't exist on the trunk.
Screesnhot
Contributor
Author
|
Thanks for the review, @Mamaduka!
In the screenshot you attached you can see 6 templates, but as far as I tested on TT4 I can only see 4 templates at most. Have you added any templates? |
Member
|
I think I tested this on a clean-ish install with TT4. Modal should display four columns on wide screens, but I can only see three after this change. This works correctly on the trunk. |
Contributor
Author
|
I tried adding templates manually, but could not reproduce the problem. Maybe it has something to do with the OS or browser. 4dfbdb1472c867404b265bc45ab0e028.mp4 |
Member
|
I can re-test again tomorrow or Monday. But let's see if others can reproduce the same issue. |
ntsekouras
reviewed
Nov 27, 2023
| } | ||
|
|
||
| .block-editor-block-patterns-list__list-item { | ||
| break-inside: avoid-column; |
Contributor
There was a problem hiding this comment.
We have the same css rule in swap template modal and while it handles the current use case that it fixes, it does have the problem @Mamaduka describes. I tried a few things but didn't figure out a proper solution. Maybe we can achieve the same result with a different css rule? 🤔 --cc @jameskoster @jasmussen
Contributor
|
I'm also unable to produce the problem of the missing 4th column which makes this a little tricky to debug. Might it make sense to use a more standard row layout here – the 'masonry' effect isn't adding a lot? A rough inspector-hacked mockup: templates.mp4This actually feely a bit easier to browse imo. |
Contributor
Oh.. For me the issue is with three columns. Four display just fine. Just noting that whatever solution we find here, we should apply it to the swap template list too. |
Contributor
Author
I'm starting to lean towards adopting this approach as well. In a full-screen modal, there are two directions in which items can flow. For example, the pattern inserter modal uses a grid layout, so items flow from left to right and top to bottom.
On the other hand, in modals like this PR, column-count is applied, so items flow from top to bottom and from left to right.
Layouts using column-count are used in several places, but I think it might be a good idea to redesign them with a grid layout 🤔 |
Contributor
Author
|
The file this PR modifies has been removed in #61507. Additionally, Template Replace Flow exists in the sidebar.
I would like to close this PR, as this dialog should no longer exist in the latest Gutenberg. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Related to #54609
What?
This PR fixes column wrapping of pattern titles in modal content when replacing template.
Before
After
Why?
These columns are implemented by
column-count. My understanding is that this property affects all internal elements, so the title will wrap unintentionally depending on the browser size.How?
Added
break-inside: avoid-column;. This style is also used in many other places.Testing Instructions