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

Redo zoom out expand empty entry content #66388

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 1 addition & 13 deletions packages/block-editor/src/components/iframe/content.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
.block-editor-iframe__html.is-zoomed-out {
$scale: var(--wp-block-editor-iframe-zoom-out-scale);
$frame-size: var(--wp-block-editor-iframe-zoom-out-frame-size);
$inner-height: var(--wp-block-editor-iframe-zoom-out-inner-height);
$content-height: var(--wp-block-editor-iframe-zoom-out-content-height);
$scale-container-width: var(--wp-block-editor-iframe-zoom-out-scale-container-width);
$container-width: var(--wp-block-editor-iframe-zoom-out-container-width, 100vw);
Expand All @@ -42,17 +41,6 @@
padding-bottom: calc(#{$frame-size} / #{$scale});

body {
min-height: calc((#{$inner-height} - #{$total-frame-height}) / #{$scale});

> .is-root-container:not(.wp-block-post-content) {
flex: 1;
display: flex;
flex-direction: column;
height: 100%;

> main {
flex: 1;
}
}
min-height: 100vh;
}
}
27 changes: 0 additions & 27 deletions packages/block-editor/src/components/iframe/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,21 +209,6 @@ function Iframe( {
};
}, [] );

const [ iframeWindowInnerHeight, setIframeWindowInnerHeight ] = useState();

const iframeResizeRef = useRefEffect( ( node ) => {
const nodeWindow = node.ownerDocument.defaultView;

setIframeWindowInnerHeight( nodeWindow.innerHeight );
const onResize = () => {
setIframeWindowInnerHeight( nodeWindow.innerHeight );
};
nodeWindow.addEventListener( 'resize', onResize );
return () => {
nodeWindow.removeEventListener( 'resize', onResize );
};
}, [] );

const [ windowInnerWidth, setWindowInnerWidth ] = useState();

const windowResizeRef = useRefEffect( ( node ) => {
Expand Down Expand Up @@ -259,10 +244,6 @@ function Iframe( {
clearerRef,
writingFlowRef,
disabledRef,
// Avoid resize listeners when not needed, these will trigger
// unnecessary re-renders when animating the iframe width, or when
// expanding preview iframes.
isZoomedOut ? iframeResizeRef : null,
] );

// Correct doctype is required to enable rendering in standards
Expand Down Expand Up @@ -370,10 +351,6 @@ function Iframe( {
'--wp-block-editor-iframe-zoom-out-content-height',
`${ contentHeight }px`
);
iframeDocument.documentElement.style.setProperty(
'--wp-block-editor-iframe-zoom-out-inner-height',
`${ iframeWindowInnerHeight }px`
);
iframeDocument.documentElement.style.setProperty(
'--wp-block-editor-iframe-zoom-out-container-width',
`${ containerWidth }px`
Expand All @@ -393,9 +370,6 @@ function Iframe( {
iframeDocument.documentElement.style.removeProperty(
'--wp-block-editor-iframe-zoom-out-content-height'
);
iframeDocument.documentElement.style.removeProperty(
'--wp-block-editor-iframe-zoom-out-inner-height'
);
iframeDocument.documentElement.style.removeProperty(
'--wp-block-editor-iframe-zoom-out-container-width'
);
Expand All @@ -407,7 +381,6 @@ function Iframe( {
scale,
frameSize,
iframeDocument,
iframeWindowInnerHeight,
contentHeight,
containerWidth,
windowInnerWidth,
Expand Down
12 changes: 10 additions & 2 deletions packages/editor/src/components/visual-editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ function VisualEditor( {
isDesignPostType,
postType,
isPreview,
isEditedPostEmpty,
} = useSelect( ( select ) => {
const {
getCurrentPostId,
Expand All @@ -127,6 +128,7 @@ function VisualEditor( {
getEditorSettings,
getRenderingMode,
getDeviceType,
isEditedPostEmpty: getIsEditedPostEmpty,
} = select( editorStore );
const { getPostType, getEditedEntityRecord } = select( coreStore );
const postTypeSlug = getCurrentPostType();
Expand Down Expand Up @@ -167,6 +169,7 @@ function VisualEditor( {
isFocusedEntity: !! editorSettings.onNavigateToPreviousEntityRecord,
postType: postTypeSlug,
isPreview: editorSettings.isPreviewMode,
isEditedPostEmpty: getIsEditedPostEmpty(),
};
}, [] );
const { isCleanNewPost } = useSelect( editorStore );
Expand Down Expand Up @@ -363,10 +366,15 @@ function VisualEditor( {
// Some themes will have `min-height: 100vh` for the root container,
// which isn't a requirement in auto resize mode.
enableResizing ? 'min-height:0!important;' : ''
}}`,
}}${
// Vertically expands the top-level post content block when zoomed out on an empty post.
isEditedPostEmpty && isZoomedOut
? '.entry-content {min-height: 33vh;}'
Copy link
Member

Choose a reason for hiding this comment

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

I'm so sorry, but I don't like this. This is pseudo-expanding. :) Either it will push the footer beyond the fold, or it will never reach the bottom. The previous implementation expanded the content exactly to the bottom and pushed the footer down. Let's restore that

Copy link
Contributor Author

@stokesman stokesman Oct 25, 2024

Choose a reason for hiding this comment

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

Either it will push the footer beyond the fold,

In the interest of this being a functional feature providing a larger drop zone, that seems like the right thing to do. If the feature is meant purely as a visual effect I see your point. Though, I think that could be considered a shortcoming of the prior version.

or it will never reach the bottom.

The footer always reaches the bottom of the canvas in this PR, but I guess you are talking about the canvas reaching the bottom of the viewport. There is still expansion so this would seem to boil down to an argument about what looks better. Any expansion creates some layout shift and perhaps less shift is a slight advantage. On the other hand, full expansion makes the canvas look like it does when it has overflow yet I'm not sure that’s actually a good thing.

I'm so sorry

Please don’t be 🤗. Thank you for the feedback. For me, this is an improvement beyond just a restoration for two reasons:

  • There is some expansion even for short viewports.
  • This creates an improved zoom transition for non-empty posts with little to no overflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess a middle ground could be to restore the full canvas and content expansion but only on empty posts.

: ''
}`,
},
];
}, [ styles, enableResizing ] );
}, [ styles, enableResizing, isEditedPostEmpty, isZoomedOut ] );

return (
<div
Expand Down
Loading