search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.32k stars 4.12k forks source link

BlockPreview: Patterns with VH units scale oddly #50449

Open richtabor opened 1 year ago

richtabor commented 1 year ago

I added a couple new patterns as core patterns, one of which uses the vh unit to set the height of a Cover block. This causes the pattern's BlockPreview to continuously expand in height, resulting in an oddly displayed pattern.

https://user-images.githubusercontent.com/1813435/236935188-0ad15ba8-ad4d-4fd2-9739-9cd2f6e28c49.mp4

Related in part to https://github.com/WordPress/gutenberg/issues/43865. And the same issue occurs in the Site Editor's Zoomed Out mode, as indicated here: https://github.com/WordPress/gutenberg/issues/49299

Thoughts on how we can resolve this?

talldan commented 1 year ago

Thoughts on how we can resolve this?

I guess there's a feedback loop going on here:

When one changes it causes the other to change, which in turn causes the other to change and so on.

It might be possible to limit how much the iframe updates its own height, but that could also cause some legit height changes to not happen. The height update could also be throttled, but that would just slow down the amount it expands 😄

Definitely an interesting one!

kevin940726 commented 1 year ago

Seems related to #38181. I don't think this issue is solvable with iframe, since the iframe height is always 100vh, and we can't render anything visible outside the iframe. Any block that has a minimum height over 100vh will cause the preview to grow infinitely. This comment explains it very well. The only way I can think of for these kinds of patterns to work within iframes is somehow parsing the CSS in the preview and replacing any instance of vh with computed units. Seems like an over-complicated work TBH 😅 .

Another quick fix I can think of is to acknowledge that some patterns can't be shown fully in a preview and instead render a sort of "more below" indicator at the bottom. Something like...

Screenshot

This is obviously not the final design but just a mockup of an idea. I can put up a draft PR and let anyone interested play with the design to find a better balance.

richtabor commented 1 year ago

Any block that has a minimum height over 100vh will cause the preview to grow infinitely.

Would 99vh work?

kevin940726 commented 1 year ago

Would 99vh work?

If it's only a single element with 99vh then it will. But if it's partnered with any non-viewport values like a 30px margin-bottom then it probably won't. 99vh + 30px will be the height of the iframe (100vh), which means 1vh is equal to 30px, and the height will be 3000px. There's currently a max height of the iframe to stop it from growing bigger than 2000px.

This is also the case for the pattern shown in the video of this original issue. IIRC, it has a height of 75vh and some paragraphs below. Adding both of them makes the height of the iframe exceed the 2000px limit, thus making it look weird 😅 .

miminari commented 1 year ago

I think this move is irritating to users. How about removing the transition CSStransition: all 0.3s ease 0s; temporarily until we can come up with a better solution? The interaction is very nice, but I don't feel that it is inevitable for the function.

ellatrix commented 1 year ago

In my opinion, we should set a max height to the preview, perhaps at the same ratio of the viewport height based on the width of the preview. There are previews that are taller, but then the rest of it will appear "under the fold" as it would appear on the front-end and in the editor? That seems reasonable to me.

kevin940726 commented 1 year ago

we should set a max height to the preview

We already set a max height to the preview, currently it's hardcoded to 2000px though 😅 .

perhaps at the same ratio of the viewport height based on the width of the preview

Could you elaborate what you mean here? Do you mean the ratio of the current users' browser viewport?

ellatrix commented 1 year ago

Do you mean the ratio of the current users' browser viewport?

Yes :) I think we should make the ratio the same. The downside is that you cannot preview "under the fold", but that seems reasonable to me. It's just a preview.

richtabor commented 1 year ago

The downside is that you cannot preview "under the fold", but that seems reasonable to me. It's just a preview.

I agree.

kevin940726 commented 1 year ago

The downside is that you cannot preview "under the fold", but that seems reasonable to me. It's just a preview.

Yep, and it's already the case. I don't think the issue is even solvable as per https://github.com/WordPress/gutenberg/issues/50449#issuecomment-1590514366. Any block that has height > 100vh will not be shown with auto-scaling iframe.

kevin940726 commented 1 year ago

I implemented https://github.com/WordPress/gutenberg/issues/50449#issuecomment-1684002018 (IIUC) in #54285 so that we can see it in action. Let's discuss there if that's the approach we want to take at this stage.

richtabor commented 1 year ago

I misunderstood the "under the fold" bit. I wasn't thinking that it was connected to your direct viewport, but a viewport size of the preview itself. If we have a max height on those, and the rest is runoff, that's fine (basically like how patterns in the "Choose a pattern" modals work).

tellthemachines commented 9 months ago

The same bug was reported on Trac here. From the report:

Occurs when any cover block's min. height is set to a vh value in the Site editor settings, and I select the Styles sidebar and from there "Browse styles" to view the theme's style presets - when the template or page "zooms out" to preview style changes, the cover block's height expands vertically perpetually making the preview hard or even impossible (if 100 vh is entered as min. height).

talldan commented 8 months ago

In https://github.com/WordPress/gutenberg/pull/56806 we discovered this bug also happens in the experimental Zoomed Out view in the editor canvas.

t-hamano commented 6 months ago

An issue with the zoomed out view was resolved by #59334. I'm wondering if a similar approach could be applied to block previews as well.

ellatrix commented 6 months ago

The difference with the preview is that it doesn't have a fixed viewport height (expands based on content). For zoom out mode we keep the same viewport height and scale the html element inside the iframe (I removed the expanding). So the only fix would be to set a fixed max height to the previews?

richtabor commented 6 months ago

So the only fix would be to set a fixed max height to the previews?

Do we do this for block previews anywhere already? I recall perhaps within the starter patterns modal, or templates view?