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

Fix zoom animation scrollbar #67536

Merged
merged 2 commits into from
Dec 3, 2024
Merged
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
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/iframe/content.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
&.zoom-out-animation {
$scroll-top: var(--wp-block-editor-iframe-zoom-out-scroll-top, 0);
$scroll-top-next: var(--wp-block-editor-iframe-zoom-out-scroll-top-next, 0);
$overflow-behavior: var(--wp-block-editor-iframe-zoom-out-overflow-behavior, scroll);

position: fixed;
left: 0;
Expand All @@ -18,7 +19,7 @@
bottom: 0;
// Force preserving a scrollbar gutter as scrollbar-gutter isn't supported in all browsers yet,
// and removing the scrollbar causes the content to shift.
overflow-y: scroll;
overflow-y: $overflow-behavior;
}

&.is-zoomed-out {
Expand Down
54 changes: 41 additions & 13 deletions packages/block-editor/src/components/iframe/use-scale-canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ function calculateScale( {
);
}

/**
* Compute the next scrollHeight based on the transition states.
*
* @param {TransitionState} transitionFrom Starting point of the transition
* @param {TransitionState} transitionTo Ending state of the transition
* @return {number} Next scrollHeight based on scale and frame value changes.
*/
function computeScrollHeightNext( transitionFrom, transitionTo ) {
const { scaleValue: prevScale, scrollHeight: prevScrollHeight } =
transitionFrom;
const { frameSize, scaleValue } = transitionTo;

return prevScrollHeight * ( scaleValue / prevScale ) + frameSize * 2;
}

/**
* Compute the next scrollTop position after scaling the iframe content.
*
Expand All @@ -47,12 +62,12 @@ function computeScrollTopNext( transitionFrom, transitionTo ) {
containerHeight: prevContainerHeight,
frameSize: prevFrameSize,
scaleValue: prevScale,
scrollTop,
scrollHeight,
scrollTop: prevScrollTop,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to keep naming consistent with all transitionFrom values having a prev prefix for clarity.

} = transitionFrom;
const { containerHeight, frameSize, scaleValue } = transitionTo;
const { containerHeight, frameSize, scaleValue, scrollHeight } =
transitionTo;
// Step 0: Start with the current scrollTop.
let scrollTopNext = scrollTop;
let scrollTopNext = prevScrollTop;
// Step 1: Undo the effects of the previous scale and frame around the
// midpoint of the visible area.
scrollTopNext =
Expand All @@ -71,15 +86,12 @@ function computeScrollTopNext( transitionFrom, transitionTo ) {
// iframe if the top of the iframe content is visible in the container.
// The same edge case for the bottom is skipped because changing content
// makes calculating it impossible.
scrollTopNext = scrollTop <= prevFrameSize ? 0 : scrollTopNext;
scrollTopNext = prevScrollTop <= prevFrameSize ? 0 : scrollTopNext;

// This is the scrollTop value if you are scrolled to the bottom of the
// iframe. We can't just let the browser handle it because we need to
// animate the scaling.
const maxScrollTop =
scrollHeight * ( scaleValue / prevScale ) +
frameSize * 2 -
containerHeight;
const maxScrollTop = scrollHeight - containerHeight;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! This is a lot more understandable after the refactoring!


// Step 4: Clamp the scrollTopNext between the minimum and maximum
// possible scrollTop positions. Round the value to avoid subpixel
Expand Down Expand Up @@ -226,6 +238,15 @@ export function useScaleCanvas( {
`${ scrollTopNext }px`
);

// If the container has a scrolllbar, force a scrollbar to prevent the content from shifting while animating.
iframeDocument.documentElement.style.setProperty(
'--wp-block-editor-iframe-zoom-out-overflow-behavior',
transitionFromRef.current.scrollHeight ===
transitionFromRef.current.containerHeight
? 'auto'
: 'scroll'
);

iframeDocument.documentElement.classList.add( 'zoom-out-animation' );

return iframeDocument.documentElement.animate(
Expand Down Expand Up @@ -278,6 +299,9 @@ export function useScaleCanvas( {
iframeDocument.documentElement.style.removeProperty(
'--wp-block-editor-iframe-zoom-out-scroll-top-next'
);
iframeDocument.documentElement.style.removeProperty(
'--wp-block-editor-iframe-zoom-out-overflow-behavior'
);

// Update previous values.
transitionFromRef.current = transitionToRef.current;
Expand Down Expand Up @@ -409,20 +433,24 @@ export function useScaleCanvas( {
// the iframe at this point when we're about to animate the zoom out.
// The iframe scrollTop, scrollHeight, and clientHeight will all be
// the most accurate.
transitionFromRef.current.containerHeight =
transitionFromRef.current.containerHeight ??
containerHeight; // Use containerHeight, as it's the previous container height value if none was set.
Comment on lines -412 to -414
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the transitionFromRef.current.containerHeight won't be as accurate, as it could have changed from a manual resize between animations. Relying on containerHeight will be more accurate.

transitionFromRef.current.scrollTop =
iframeDocument.documentElement.scrollTop;
transitionFromRef.current.scrollHeight =
iframeDocument.documentElement.scrollHeight;
// Use containerHeight, as it's the previous container height before the zoom out animation starts.
transitionFromRef.current.containerHeight = containerHeight;

transitionToRef.current = {
scaleValue,
frameSize,
containerHeight:
iframeDocument.documentElement.clientHeight, // use clientHeight to get the actual height of the new container, as it will be the most up-to-date.
iframeDocument.documentElement.clientHeight, // use clientHeight to get the actual height of the new container after zoom state changes have rendered, as it will be the most up-to-date.
};

transitionToRef.current.scrollHeight = computeScrollHeightNext(
transitionFromRef.current,
transitionToRef.current
);
Comment on lines +450 to +453
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On first render (previously), transitionToRef did not have a scrollHeight value.

transitionToRef.current.scrollTop = computeScrollTopNext(
transitionFromRef.current,
transitionToRef.current
Expand Down
Loading