Individual Patterns have scrollbars in Inserter making it difficult to scroll through

annezazu commented 2 years ago

Description

With WordPress beta 4, after opening the Inserter, I found that the various patterns show a scroll bar and are randomly very condensed. This makes it hard to quickly navigate between patterns while scrolling as sometimes the focus gets lost in scrolling in the pattern itself.

Step-by-step reproduction instructions

Open Inserter.
Switch to patterns tab.
Try scrolling down through the patterns.

Screenshots, screen recording, code snippet

https://user-images.githubusercontent.com/26996883/147023921-676f4c23-ed30-494f-bf33-3d6e1cb955b7.mov

Environment info

WordPress 5.9 Beta 4
TT2
Chrome
No Gutenberg

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

Mamaduka commented 2 years ago

Hi, @annezazu

Since this isn't a significant issue, I think we can punt it to the next minor/major release.

I'm tentatively removing this from the board, but feel free to add it back.

WhiteHeatherClub commented 2 years ago

Hi, I'm finding it a bit significant having convinced my organisation with 300 editors to move from SharePoint to WordPress because of the page building ability and built 40+ block patterns which are currently being used in 5.8, but now the inserter in RC1 is in an unusable state with randomly compressed, wrong or inconsistent aspect ratios and scrollbars (unless I install the latest Gutenberg plugin on top of RC1).

If you are interested and have three and a half minutes to spare I can demonstrate here: https://youtu.be/XoXepDW0PbM

annezazu commented 2 years ago

I would agree that this isn't necessarily a minor issue, especially since patterns are a growing part of the WordPress ecosystem and a big part of block themes. At the least, I believe this needs to make it into a point release for 5.9.

With that said, I just tested this with Gutenberg 12.3 and found the issue isn't visible so there is some fix in place. I tried to look through the 12.3 release changelog but nothing necessarily stood out. Ideally, whatever was done, can be backported cc @noisysocks

chthonic-ds commented 2 years ago

Came to report this after testing a theme with 5.9-RC1. I've noticed this doesn't happen consistently:

sometimes a pattern loads correctly
sometimes scrolling is required to scan a pattern
sometimes a scrollbar is present and the full pattern is visible

The scrolling-required version happens most of the time, and is visible in both the sidebar and "Explore" modal views.

pattern-explorer-scrollbars-5 9RC1

noisysocks commented 2 years ago

With that said, I just tested this with Gutenberg 12.3 and found the issue isn't visible so there is some fix in place. I tried to look through the 12.3 release changelog but nothing necessarily stood out. Ideally, whatever was done, can be backported cc @noisysocks

Maybe we can figure out what fixed it using git bisect. It would have assumedly landed after Gutenberg 11.9.

jasmussen commented 2 years ago

I can consistently reproduce this with the latest nightly, with the Gutenberg plugin deactivated. It's possible there's a loading order or race condition issue attached.

noisysocks commented 2 years ago

I can't personally reproduce the issue which makes it difficult for me to find which PR needs to be backported.

https://user-images.githubusercontent.com/612155/149068005-461f6da6-7f6b-49c5-9581-e24a1b617650.mp4

Am I doing something wrong when attempting to reproduce?

I searched for PRs merged after Gutenberg 11.9 (the last release merged wholesale into Core) with the word "inserter" in it but couldn't find anything that looks like a fix for the issue.

If someone is able to reproduce the issue locally then they could potentially find where this was fixed using git bisect in the Gutenberg repo with v11.9.0 as the bad commit and trunk as the good commit.

@WordPress/gutenberg-core: Does anyone recall merging something that would have fixed this?

ntsekouras commented 2 years ago

I can't reproduce either..

I can consistently reproduce this with the latest nightly, with the Gutenberg plugin deactivated. It's possible there's a loading order or race condition issue attached.

@jasmussen which theme do you use? Or is there any difference between post and site editors?

jasmussen commented 2 years ago

I'm using the TwentyTwentyTwo theme that comes with the latest nightly of WordPress, and I'm seeing the pattern scrollbars in both the post and site editors.

Here's a video showing how it's working fine with the plugin enabled on latest nightly, and breaks as soon as I deactivate the plugin:

https://user-images.githubusercontent.com/1204802/149126825-6f9af8b2-28a1-464f-a4ee-03a8a1a61cc2.mov

Note in the video it shows there's an updated version nightly version, but after recording the video I tried updating and am still seeing the same issue.

Usually that fact means there's a fix in the plugin not yet ported to the core version. But I'm fairly sure we've had issues like these in the past which was related to the loading order of CSS or JS changing between the plugin that enqueues it, and the core version that has it integrated. I can't say whether it's one or the other.

jasmussen commented 2 years ago

The only other thing I can think of is that there was a Safari specific issue where a pattern would keep growing vertically if it used vh units perhaps misbehaving. I can't find the PR, but that was merged a while ago.

Mamaduka commented 2 years ago

@jasmussen, I think this is issue #34729.

WhiteHeatherClub commented 2 years ago

Getting a new twist on this issue now with RC2. Again, works fine (albeit with scrollbars) with Gutenberg plugin activated.

https://user-images.githubusercontent.com/97255524/149234573-350e105f-907c-48eb-8f47-cf602ce9a556.mov

walbo commented 2 years ago

I can consistently reproduce this on RC2 when SCRIPT_DEBUG is false.

When script debug is disabled all the styles are concatenated into load-styles.php. Because of this all admin styles are added to the iframe preview and not only the required ones.

Ex. When the styles are not concatenated the wp-reset-editor-styles are not included in the iframe.

https://github.com/WordPress/gutenberg/blob/f2161e246b9fdd9a2a56e7552b0b28050f1a5302/packages/block-editor/src/components/iframe/index.js#L63-L68

But when wp-reset-editor-styles is in load-styles.php there is no way to not include it.

I suspect that there is style in load-styles.php that are effecting the previews.

I also think https://core.trac.wordpress.org/ticket/54752 is related and the PoC patch which don't concateneted the styles used in the iframe seems so solve the issue for me.

ndiego commented 2 years ago

Just wanted to add my notes here. In doing a little testing last night, I disabled Gutenberg and tested just 5.9 Beta RC2. The pattern previews were all over the place, both in Chrome and Safari. I have tried identifying the root cause with little success.

This first image is TT2 in 5.9 RC2. Notice how tall the container with bird image is and the pattern with Query block is not rendering properly.

image (1)

Second image is TT2 in 5.9 RC2 with Gutenberg 12.3.2 active and all looks as it should.

image (2)

noisysocks commented 2 years ago

I believe since devs are struggling to reproduce this issue and RC 4 is tomorrow that we may need to fix this in WordPress 5.9.1.

I also think https://core.trac.wordpress.org/ticket/54752 is related and the PoC patch which don't concateneted the styles used in the iframe seems so solve the issue for me.

Oh that's interesting. @ellatrix @youknowriad: Do you know what's going on?

tellthemachines commented 2 years ago

I can only reproduce this issue on 5.9 RC2 in a production environment without the plugin installed. What seems to be happening is the following:

The height of each preview is calculated from a resize event listener attached to the contents of its iframe, and then added dynamically to block-editor-block-preview__content.
In a dev environment this works fine, but in production the event listener is being removed for some reason.
Because the event listener is removed before the iframe contents are fully loaded, the height isn't calculated properly, resulting in either scrollbars or a super tall box.

I haven't found out why this is happening yet; if anyone else has ideas feel free to jump in! Seems unlikely it'll be fixed for RC tomorrow though.

youknowriad commented 2 years ago

Just wanted to share that I have been failing to reproduce this so far (so difficult for me to help) 😬 I tried several things (unsetting the debug constants, with and without Gutenberg...

What comes to my mind here is that during the 4.9 lifecycle, we switched the previews to be in "iframes" and my random guess is that it's a side effect of that change that for some reason didn't show up in the Gutenberg plugin (so we didn't catch it early enough).

sc0ttkclark commented 2 years ago

Just to be super clear, I've seen issues like this only in FSE but not in the normal post editor screen. FSE seems to be the factor some people trying to reproduce have potentially missed here.

EDIT: Ignore this, I was wrong

tellthemachines commented 2 years ago

Just to be super clear, I've seen issues like this only in FSE but not in the normal post editor screen. FSE seems to be the factor some people trying to reproduce have potentially missed here.

That's not the case with this issue; I can reproduce it in the post editor with a classic theme such as TT1. The main factor here seems to be script concatenation: the issue only happens in production environments, not development, and it can't be reproduced with Gutenberg enabled, because the plugin doesn't concatenate its scripts.

ndiego commented 2 years ago

Yes, I can second the comment above. I am able to replicate in both the page/post Editor as well as the Site Editor when Gutenberg is not active.

sc0ttkclark commented 2 years ago

Sorry I conflated the issue with a separate issue, my bad here

annezazu commented 2 years ago

Noting that this still happens with 5.9 RC3 (not surprising; just noting).

jasmussen commented 2 years ago

Just cross-posting @ndiego's good comment from the VH units issue, possibly relevant to this discussion as well: https://github.com/WordPress/gutenberg/issues/34729#issuecomment-1016453489

ellenbauer commented 2 years ago

I can also confirm the issue, consistently both in the post/page editor and in the site editor (in the sidebar as well as in the Explore patterns view).

It's either a scrollbar or the patterns heights are either stretched or squeezed in which case the patterns preview are not visible. Still occurring in 5.9-RC3 without the Gutenberg plugin active.

I don't think it's a minor issue at all. It makes the experience to explore patterns a pretty broken one 😟

ndiego commented 2 years ago

I don't think it's a minor issue at all. It makes the experience to explore patterns a pretty broken one 😟

Agreed, when a fix is found I personally think it's important enough to be included in the 5.9.1.

WhiteHeatherClub commented 2 years ago

I don't think it's a minor issue at all. It makes the experience to explore patterns a pretty broken one 😟

Agreed, when a fix is found I personally think it's important enough to be included in the 5.9.1.

Agreed. Makes 5.9 a non-starter here. Already using Patterns extensively in production sites, there's no way I could roll this version out. Having welcomed Gutenberg on it's arrival, then embraced Patterns, I can't help but feel disappointment with this release.

noisysocks commented 2 years ago

Thanks for the reports everyone!

This issue is in the the WP 5.9.1 milestone along with other bugs that did not have a patch in time for the WP 5.9 code freeze.

It might be that this is caused by https://github.com/WordPress/gutenberg/issues/34729,. @jasmussen and @Mamaduka are currently investigating whether that one can be fixed in time for the unscheduled WP 5.9 RC 4 release today.

talldan commented 2 years ago

I finally managed to reproduce this, but only with a few patterns. It happen in my normal development environment with this pattern and a couple of other 'Pages' patterns (with Gutenberg active): Screen Shot 2022-01-24 at 4 24 34 pm

edit: It seems like all the patterns that have this issue (for me) contain the Site Logo block. I don't have a logo uploaded, so the block is just showing the placeholder, but something unusual with those placeholder styles is making it break out of its container. It looks like the pattern preview doesn't resize to the correct size whenever any element breaks out of a container, so the scrollbar is shown.

I'm not sure that this is the same issue reported though. It seems like that happened on more patterns than I'm seeing.

talldan commented 2 years ago

I'm not sure that this is the same issue reported though. It seems like that happened on more patterns than I'm seeing.

I've managed to reproduce this using the SCRIPT_DEBUG method now and that results in more patterns being affected. So it seems like the bug I observed before is a different but related problem.

When script debug is disabled all the styles are concatenated into load-styles.php. Because of this all admin styles are added to the iframe preview and not only the required ones.

This is a good point. The main difference in one of the patterns I checked seems to be that a and div elements have outline: 0. iframe elements have border: 0. These are both things that don't seem to have any effect. There may be other styles that also cause an issue, but none that I noticed.

Mamaduka commented 2 years ago

@talldan pushed #38175 as a proposed fix.

talldan commented 2 years ago

@talldan pushed #38175 as a proposed fix.

@Mamaduka I just spotted the issue. I don't know whether your fix is quite right, as it will result in patterns being cut off.

The offending style is this height: 100% on the body of patterns: Screen Shot 2022-01-24 at 6 02 36 pm

Kudos to @walbo for spotting the problem.

ndiego commented 2 years ago

@Mamaduka and @talldan I do want to note that this issue appears to only be in WordPress 5.9. (The vh expanding issue is separate and is in both 5.9 and Gutenberg). As soon as you activate Gutenberg, the bulk of the issues go away. So it appears to be something specific in 5.9. Perhaps @tellthemachines comment above sheds some light on this?

That's not the case with this issue; I can reproduce it in the post editor with a classic theme such as TT1. The main factor here seems to be script concatenation: the issue only happens in production environments, not development, and it can't be reproduced with Gutenberg enabled, because the plugin doesn't concatenate its scripts.

youknowriad commented 2 years ago

Here's my wild guess:

The iFrame (which is now used in previews similar to the site editor) works by looping through the stylesheets loaded in the current page and trying to detect which ones target the blocks (.wp-block or .editor-styles-wrapper selectors). Depending on whether the styles are concatenated, the result is probably different: with style concatenation we probably end up including more than what's necessary causing style issues and differences because the actual stylesheets that need to be loaded are concatenated with unrelated styles. cc @ellatrix if my theory makes sense :)

Mamaduka commented 2 years ago

@ndiego, @youknowriad

When the plugin is active, we're overriding style load paths. Unfortunately, the plugin assets aren't concatenated, so we get different results.

It also makes debugging hard 😞

It would be great to fix the source of the issue in future releases. For WP 5.9, here's a proposed bandaid fix - #38175.

ndiego commented 2 years ago

Thanks for all your work on this @Mamaduka ❤️

hellofromtonya commented 2 years ago

Hello all 👋 PR #38175 has been merged and will be backported to Core to include in 5.9 RC4 and final release. Thank you everyone for contributing!

ellenbauer commented 2 years ago

Thanks so much for all the last minute work that has been done to fix this issue. Unfortunately I just tested RC4 with the Gutenberg plugin disabled and the issue persists, see screenshot:

Now the scrollbars are gone, but the height is not displayed correctly, which makes it impossible to view the pattern.

annezazu commented 2 years ago

Re-opening this having tested with 5.9. Right now, I'm seeing some odd behavior. At first, the pattern doesn't show the right height but, after adding it to a post or page, it then appears in the Inserter correctly. Here's a quick video:

https://user-images.githubusercontent.com/26996883/151056304-822258a4-8588-4d9e-a9d4-f033ce330549.mov

I think this is good to dig into for 5.9.1 cc @Mamaduka @noisysocks

WhiteHeatherClub commented 2 years ago

It appears correctly on the page, but stays at the wrong height all the time for me in the inserter

webmandesign commented 2 years ago

Hi guys, I'd like to chime in as I think this may be related:

I experience continual height change in block pattern preview for blocks with 100vh height such as Cover block and also Media & Text block in my case (where I use custom helper CSS class to set height).
Also, blocks with height of a different value, such as 62vh, are not correctly sized in block pattern preview and seem to be lower/narrower.

Doesn't matter if the height is applied with inline styles like it is with Cover block or with a custom CSS class like it is in Media & Text block.

Here are video examples with WP 5.8.3 behavior for comparison: Cover block:

https://user-images.githubusercontent.com/2456735/151227174-d1fafacc-21b0-48d1-b744-0efe94cf97dd.mp4

Media & Text block:

https://user-images.githubusercontent.com/2456735/151227218-f82266df-ae02-4a94-b6a6-d6da4b4c45ea.mp4

I'm on WP 5.9 and using classic (non-FSE) theme Michelle 1.3.0.

I was able to resolve this issue temporarily via theme by forcing absolute units in my block editor stylesheet, such as:

body.editor-styles-wrapper [style*="min-height: 100vh"] {
    min-height: 900px !important;
}

talldan commented 2 years ago

I think it might be best to open a new issue for this. This issue has the wrong title and description for the bug that's now happening. Anyone looking to fix it would have to scroll all the way to the bottom to find out the problem.

annezazu commented 2 years ago

@talldan Okay! Done and closing: https://github.com/WordPress/gutenberg/issues/38501

WordPress / gutenberg