Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patterns using VH units or fixed background vertically expanding their previews endlessly #34729

Closed
Robertght opened this issue Sep 10, 2021 · 16 comments · Fixed by #38175 · May be fixed by #38181
Closed

Patterns using VH units or fixed background vertically expanding their previews endlessly #34729

Robertght opened this issue Sep 10, 2021 · 16 comments · Fixed by #38175 · May be fixed by #38181
Assignees
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@Robertght
Copy link

Robertght commented Sep 10, 2021

Description

This is a replica of this issue: Automattic/wp-calypso#56077

Step-by-step reproduction instructions

Steps to Reproduce

  1. Start by opening a page to edit
  2. Click the blue plus sign to open the blocks & pattern search sidebar
  3. Go to Patterns → Headers (in the Patterns drop down)
  4. Start scrolling. The second pattern, "Large header with text and a button" does not load correctly and breaks the entire pattern selection box.

I tested this on Chrome and did not see the same error/issue.

In Safari, I also got an error that "This webpage is using significant memory. Closing it may improve the responsiveness of your Mac."

I'm currently running Safari Version 14.1.2 (16611.3.10.1.6) on macOS Big Sur, version 11.5.2. 16" 2019 MacBook Pro.

Video of the error:

Screen.Recording.2021-09-07.at.2.27.37.PM.mov

Screenshots, screen recording, code snippet

No response

Environment info

Operating System

macOS

OS Version

11.5.2

Browser

Safari

Browser Version(s)

14.1.2

Is this specific to applied theme? If so, what is the theme name?

I tested using Blank Canvas and also Bradford. Issue happened on both themes.

Console and/or error logs

No response

Available Workarounds

You can use a different web browser - I was able to select a header pattern in Chrome.

Reproducibility

No response

Other information

No response

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

@jasmussen
Copy link
Contributor

Thank you for the ticket. I can reproduce this in trunk. The issue is with the "Large header with text and a button" pattern under "Headers".

The pattern in question uses both the cover block's fixed-position property and the has a height of 100vh. This is the element that expands:

<div class="block-editor-block-preview__content components-disabled css-idxg31-StyledWrapper e1ac3xxk0" style="transform: scale(0.250833); height: 10028.1px;"><iframe aria-hidden="true" tabindex="-1" title="Editor canvas" style="position: absolute; width: 1200px; pointer-events: none; height: 39979px;"></iframe></div>

bug

It appears to be the iframe auto-resize function that keeps firing, possibly because 100vh keeps increasing as the iframe grows taller, causing a loop.

@Robertght
Copy link
Author

@jasmussen It looks like there are other browsers and OS's(& versions) affected by this: Automattic/wp-calypso#56182

@jasmussen jasmussen changed the title Some patterns(headers/about) expanding in Safari Patterns using VH units or fixed background vertically expanding their previews endlessly Sep 17, 2021
@jasmussen
Copy link
Contributor

I took the liberty of updating the title a bit.

@annezazu annezazu added [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Sep 17, 2021
@zdenys
Copy link

zdenys commented Sep 22, 2021

I was able to reproduce it with the Coming Soon patterns too.

bug-block-pattern-height-expanding-indefinitely-2021-09-21-at-09.mp4

@yansern
Copy link
Contributor

yansern commented Oct 13, 2021

Investigation note:

  1. 100vh units causes the resize observer to go into a firing loop.
  2. The component tree looks like this BlockPatternsList > BlockPattern > BlockPreview > AutoHeightBlockPreview.
  3. The file to look at is packages/block-editor/src/components/block-preview/auto.js.
  4. Quick fix: As there is no DOM event that could determine when a page is fully rendered, I think the quickest way is to just have a hard limit on how many times ResizeObserver listener can be triggered.

@gwwar
Copy link
Contributor

gwwar commented Oct 15, 2021

I saw this with Skatepark too. BlockPreview already has a viewportWidth prop. Can we provide a similar default for viewportHeight? Patterns that use vh should probably explicitly set this when registering the pattern. In addition to this we can set a max height (so we don't get silly results).

CleanShot.2021-10-15.at.11.36.41.mp4

alshakero added a commit to alshakero/gutenberg that referenced this issue Oct 21, 2021
@alshakero
Copy link
Contributor

I'm working on this and I think I've find a good solution. I'll send a PR shortly.

alshakero added a commit to alshakero/gutenberg that referenced this issue Oct 21, 2021
Use iframe content height as the iframe height, but cap it to the arbitrary but reasonable aspect ratio of 1:1.

This prevents vh units from making the iframe grow forever.

Fixes: WordPress#34729
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 21, 2021
@alshakero
Copy link
Contributor

PR ready #35841

@kjellr
Copy link
Contributor

kjellr commented Oct 28, 2021

Just a heads up that I've temporarily removed Header area with hatched pattern from the list of featured patterns, so that this doesn't happen whenever folks scroll down the "Featured" list.

@ndiego
Copy link
Member

ndiego commented Jan 19, 2022

I wanted to circle back on this issue as it's a pretty important one, especially as patterns become a more integral part of the building experience.

I have been unable to diagnose the root cause, but I did want to highlight an incredibly easy way to reproduce this using native block controls.

  1. Make sure you have the latest version of Gutenberg active.
  2. Create a Columns block with two columns.
  3. In the right column, add a Cover block, set a background image/color and apply the "Toggle full height" setting.
  4. In the left column add some text.
  5. Register a pattern with this block code.
  6. Preview the pattern in the Inserter.

vh-issue

What strange is while I am able to reproduce this in the latest version of Gutenberg, this issue does not appear to exist In WordPress 5.9 RC3! The same pattern in RC3 without Gutenberg active looks like this:

image

Very curious 🤔

@richtabor
Copy link
Member

richtabor commented Jan 21, 2022

I've been able to replicate this, but only with vh units set to 100. If I set a pattern to 99vh it seems to be working fine.. probably 😅

@courtneyr-dev
Copy link
Contributor

I have reproduced this issue as well when activating Gutenberg 12.5.x. I found it especially noticeable in Twenty Twenty One theme, and less apparent in Twenty Twenty Two.

large.header.vh.mp4

Mac Monterey, Chrome Version 97.0.4692.71, running site in LocalWP, 5.9RC3

@noisysocks
Copy link
Member

Can the folks here who are able to reliably reproduce the issue please go and test whether #35841 fixes it?

@ixkaito
Copy link
Contributor

ixkaito commented Jan 24, 2022

I share my video from WordPress Core's Trac. This bug exists with all bundled themes except Twenty Twenty-Two.

Kapture.2022-01-23.at.19.09.59.mp4

#35841 seems to provide a minimum fix. Blocks still expand vertically but stop at the aspect ratio 1:1.

Kapture.2022-01-24.at.14.00.51.mp4

WordPress: trunk, 5.9-RC3
OS: macOS
Browsers: Chrome, Firefox

@ixkaito
Copy link
Contributor

ixkaito commented Jan 24, 2022

I noticed that this bug occurs with non-block themes. The bug is caused by the vertical margin applied to previews of 100vh blocks. The vertical margin is set in the following file.

margin-top: $default-block-margin;
margin-bottom: $default-block-margin;

@hellofromtonya
Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet