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

Styles > Browse Styles: query loop causing ever expanding view when zoomed out #52297

Closed
annezazu opened this issue Jul 4, 2023 · 13 comments
Closed
Labels
[Block] Query Loop Affects the Query Loop Block Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

annezazu commented Jul 4, 2023

Description

When using 6.3 beta 3, I noticed that when browsing style variations when there's a query loop in the template, the zoomed out view causes an infinitely scrolling view, stretching far beyond the bounds. This doesn't happen when browsing styles in Site View nor if you are browsing styles in a template without the query loop block which is why I think the two are related. This reminds me of the problem with pattern previews: #34729

Step-by-step reproduction instructions

  1. Open Site Editor
  2. Edit home template with a query loop
  3. Open Styles > Browse variations
  4. Notice that when zoomed out, the preview expands.

Screenshots, screen recording, code snippet

Here's a video of the problem:

expanding.mov

Here's the functionality working properly when in Site View > Styles or when editing a template without a Query Loop:

style.variation.switching.working.mov

Environment info

  • WP 6.3 Beta 3
  • TT3
  • No GB

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

@annezazu annezazu added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Block] Query Loop Affects the Query Loop Block labels Jul 4, 2023
@ramonjd
Copy link
Member

ramonjd commented Jul 5, 2023

I can reproduce this.

This looks to be a phenomenon of 2023 only.

Here's 2022

2023-07-05 16 21 14

My suspicion is the max(15vw, 30vh) added to the post featured image block style.

See: https://github.com/search?q=repo%3AWordPress%2Ftwentytwentythree%2015vw&type=code

The max() is recalculating the height until it hits the largest of either 30vh or 15vw. As the height of the zoomed out iframe grows, the height of the featured images will also grow up to a max of, for example, 30% of the viewport height or 15% of the viewport width, whichever is greater. I think that's how it works 😄

Anyway, here's the 2023 home page template (/templates/home.html) without max(15vw, 30vh) in the post featured image block attributes.

2023-07-05 16 24 45

My vote would be to update them in the theme unless it's huge deal. According to WordPress/twentytwentythree#165, they might be there to ensure the featured images are aligned.

cc @mikachan

@ramonjd ramonjd moved this from 📥 Todo to 🗣️ In discussion, needs decision in WordPress 6.3.x Editor Tasks Jul 5, 2023
@annezazu
Copy link
Contributor Author

annezazu commented Jul 5, 2023

Excellent sleuthing! Thank you. ccing @jffng too just to be safe.

@mikachan
Copy link
Member

mikachan commented Jul 5, 2023

According to WordPress/twentytwentythree#165, they might be there to ensure the featured images are aligned.

Yes, that's right, they were added to ensure all the images were the same height when displayed in a grid. I'm happy to take a look at addressing this directly in the theme (unless anyone beats me to it, then I'm happy to review!)

@t-hamano
Copy link
Contributor

t-hamano commented Jul 6, 2023

As @ramonjd says, I think the fundamental problem is that the height recalculation loops. Similar problems have been reported in the past.

@annezazu
Copy link
Contributor Author

annezazu commented Jul 6, 2023

Sweet. @mikachan can I lean on you to see this through for TT3? Otherwise, I'm going to close this out for now. It feels important to address either way since TT3 is obviously still the default theme and experience for folks starting out.

@annezazu annezazu closed this as completed Jul 6, 2023
@github-project-automation github-project-automation bot moved this from 🗣️ In discussion, needs decision to ✅ Done in WordPress 6.3.x Editor Tasks Jul 6, 2023
@mikachan
Copy link
Member

mikachan commented Jul 7, 2023

@mikachan can I lean on you to see this through for TT3?

Yes! Will drop an update here when I have a fix.

@mikachan
Copy link
Member

I've opened a potential fix here: WordPress/wordpress-develop#4821

This swaps the current height of the featured images from max(15vw, 30vh) to clamp(15vw, 30vh, 400px), which means they have a maximum fixed-height of 400px.

Before After
Screenshot 2023-07-10 at 12 27 02 Screenshot 2023-07-10 at 12 37 16

@benharri
Copy link
Contributor

benharri commented Oct 6, 2023

still seeing this happen on 6.3.1 with TT3. Gutenberg plugin not installed so i have whatever is in trunk.

https://wordpress.slack.com/files/U05RF0P3E31/F05VDHUGV0S/firefox_ok1tmoqlth.mp4

@benharri
Copy link
Contributor

benharri commented Oct 6, 2023

I did recently change the Post Featured Image block to use 4:3 aspect ratio, which might've re-triggered this behavior with vh/vw

@yonkov
Copy link

yonkov commented Nov 7, 2023

Is anyone working on this? I am currently making a block theme from scratch and I can confirm the issue persists in WordPress 6.3 and WordPress 6.4 Beta 3, when no additional plugins are installed. I tried using the Cover block with 100vh and styles preview in the editor keeps expanding, creating awful user experience. I tested the same code in WordPress 6.1 and the style preview there works flowlessly. It is clearly something that has been introduced in WordPress 6.2 or WordPress 6.3 and for blocks with vh.

I recorded a short video to see what I am talking about. This bug basically ruins the whole advantage of having style variations. If i remove the 100vh, the problem is gone.

I think one reason for slow block theme adoption are bugs like this, so it is very important to address that in a reasonable timeframe. Please let us know on the progress and when a fix for this will be merged into core.

2023-11-07_17-47-10.mp4

@ramonjd
Copy link
Member

ramonjd commented Nov 8, 2023

This is a tricky one. I reverted 2023's changes locally to test again how it behaved.

From what I can tell, the contentHeight calculation in the iframe component from useResizeObserver() is always trying to return the current content document height, but as the images grow in accordance with CSS (30vh) they're pushing the document higher and so contentHeight is always trying to catch up.

For every step in height, the iframe rerenders too, so it's not ideal.

I'm not sure what the fix could be. Maybe try to calculate the initial content height as a factor of the scale value before the zoom kicks in, e.g., const maxHeight = contentRefProp.current?.offsetHeight / scale; In the the block canvas component perhaps.

And then use maxHeight as a ceiling somehow. 🤷🏻

@yonkov
Copy link

yonkov commented Nov 8, 2023

@ramonjd , thank you for your reply. Is there any workaround that I can currently use? For example, disable this behavior on style preview with some css or js?

@ramonjd
Copy link
Member

ramonjd commented Nov 8, 2023

Is there any workaround that I can currently use? For example, disable this behavior on style preview with some css or js?

Nothing in JS that I know, and the iframe doesn't receive a is-zoomed-out class so there's no hook on the element either.

Out of interest I tried debouncing the callbacks in useResizeObserver() as well, but that just slows down the inevitable: the vh units are expanding with the iframe height.

Removing the transition style does make the window eventually stop. Here, I think, is the source of the race condition.

2023-11-09.10.37.40.mp4

The result isn't ideal, but better.

So I suppose you could do this:

.edit-site-visual-editor__editor-canvas {
	transition: none !important;
}

I'm not sure what the right approach would be, but I'd expect the iframe down to resize itself the original body height * scale and the content to be at the same proportion so that it all fits.

Even declaring a MAX_HEIGHT, which, when reached, would stop the iframe resizing would work. We'd have to deal with scroll, but it would be a better experience in such cases.

I think we could open an issue to discuss and explore because I believe it's not feasible to leave it up to the themes to avoid vh 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

No branches or pull requests

6 participants