Pattern Assembler - Fast preview component max-height: 2000px cuts off patterns in tablet viewport

[miksansegundo]

Proposed Changes

• Allow overwriting the default block renderer maxHeight
• Disable maxHeight in the large preview
• Limit the height of 100vh patterns to 500px instead of 2000px or use the viewport height if it's passed

This solution fixes the pattern cut-off issue in the large preview while still supporting viewportHeight for patterns with 100vh and sets a height of 500px for their thumbnail preview, which looks better than 2000px.

Thumbnail for 100vh patterns

Before	After

Large preview for patterns that create viewport scroll

Before	After

Testing Instructions

• Create a new site, choose a domain, and a plan...
• Click continue until the Design picker, and scroll down to click Start designing
• Add the pattern with four images in a grid 2x2
  
• In the large preview, use the tablet viewport, and the scroll should appear because the pattern height is taller
• Confirm that the whole pattern is visible when you scroll the viewport down and it's not cut off

Pre-merge Checklist

• Has the general commit checklist been followed? (PCYsg-hS-p2)
• Have you written new tests for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
• For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

Related to #72313 #70063

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:commit-f4ffbc66784923c1279612b71261324a761f9eb0

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:commit-f4ffbc66784923c1279612b71261324a761f9eb0&env=jetpack

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Async-loaded Components (~208 bytes added 📈 [gzipped])

name                              parsed_size           gzip_size
async-load-pattern-list-renderer       +233 B  (+0.0%)     +107 B  (+0.0%)
async-load-pattern-large-preview       +228 B  (+0.0%)     +101 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[miksansegundo]

@autumnfjeld I'm going to merge this PR because the issue exists in production now.

[autumnfjeld]

I think your changes are definitely good. I tested in production and it is much better. :)
I followed your testing instructions ✅
Thank you for your attention to detail on this!

I do wonder if we should allow the thumbnail to be a height greater than 500px. It’s not really possible to convey that “this pattern will fill your screen”. But maybe it should be taller than 500px? The reason I say that is to give a better “impression” that the pattern is “very tall” 🙂.

I’m fine with leaving it how it is however. Just wanted to say my two thoughts. I mentioned that in a comment.

[noahtallen]

Hey @miksansegundo, I noticed this while working on #74540. What are the units for maxHeight here? One issue I noticed is that maxHeight can technically be a string (e.g. "200px"), which probably wouldn't work with this logic. (But it doesn't seem like we use it like that anywhere.)

But the main issue is that the types are technically wrong, since we now allow maxHeight to be a string or a number. If we only want to allow "none", could we change it so that the prop type is "none" | number?

[miksansegundo]

Thanks for the ping @noahtallen, the units for maxHeight are pixels and the type is number. I added the type string only to support disabling the default value using none .

I'll change the type to use 'none' | number for now and follow up with a better way to disable the default value via a new prop. See #74843

[noahtallen]

Cool!