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.

Sign up for GitHub

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

Jump to bottom

Fix pattern preview expanding height and scrollbar issues #38175

Merged
merged 9 commits into from
Jan 24, 2022
Merged

Fix pattern preview expanding height and scrollbar issues #38175

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1ec6190
Apply a 99vh max-height to the thumbnail.
jasmussen Jan 24, 2022
a99db21
Polish
jasmussen Jan 24, 2022
94ff198
Apply pixel based max-height.
jasmussen Jan 24, 2022
46950b9
Don't apply overflow auto to all iframes
Mamaduka Jan 24, 2022
e909ec9
Memoize editor styles
Mamaduka Jan 24, 2022
929e572
Remove no need for iframe changes
Mamaduka Jan 24, 2022
86d9384
Unset height
Mamaduka Jan 24, 2022
5c12d18
Apply max-height to container
Mamaduka Jan 24, 2022
0a62238
Update max-height
Mamaduka Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion packages/block-editor/src/components/block-preview/auto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { Disabled } from '@wordpress/components';
import { useResizeObserver, pure, useRefEffect } from '@wordpress/compose';
import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -33,6 +34,18 @@ function AutoBlockPreview( { viewportWidth, __experimentalPadding } ) {
};
}, [] );

// Avoid scrollbars for pattern previews.
const editorStyles = useMemo( () => {
if ( styles ) {
return [
...styles,
{ css: 'body{overflow:hidden;}', __unstableType: 'presets' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment over here, but just wanted to make sure you see it: #37573 (comment)

I think if we need to override a style in the iframe, it'd be good to also do something like height: unset, the reason being that height: 100% is what's causing the scrollbar for patterns.

I'm not sure if you also need overflow:hidden to fix the other issue. It seems ok in conjunction with overriding the height, but just overflow:hidden on its own causes patterns to be cropped unnecessarily.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a comparison to show what I mean:

Just overflow: hidden on the preview body:
Screen Shot 2022-01-24 at 6 13 00 pm

height: unset and optionally overflow: hidden on the preview body:
Screen Shot 2022-01-24 at 6 11 40 pm

It's subtle, but you can see for the second one there's the correct spacing at the bottom of the preview when unsetting the height. My concern is that some patterns may be more affected than others by this effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @talldan.

Added the rule to unset height.

];
}

return styles;
}, [ styles ] );

// Initialize on render instead of module top level, to avoid circular dependency issues.
MemoizedBlockList = MemoizedBlockList || pure( BlockList );

Expand All @@ -49,7 +62,7 @@ function AutoBlockPreview( { viewportWidth, __experimentalPadding } ) {
} }
>
<Iframe
head={ <EditorStyles styles={ styles } /> }
head={ <EditorStyles styles={ editorStyles } /> }
assets={ assets }
contentRef={ useRefEffect( ( bodyElement ) => {
const {
Expand All @@ -70,6 +83,12 @@ function AutoBlockPreview( { viewportWidth, __experimentalPadding } ) {
width: viewportWidth,
height: contentHeight,
pointerEvents: 'none',
// This is a catch-all max-height for patterns.
// VH units are as tall as your current viewport, and when used inside a scaled iframe
// the math to convert an inherited VH unit appears to cause it to keep growing endlessly.
// By applying a max-height, at least it will stop growing.
// A longer term fix would be to disallow the thumbnail from growing after initial load.
maxHeight: '1800px',
} }
>
{ contentResizeListener }
Expand Down