From b0d073ae105a1244f69036bb67428dff1deb6dcb Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 13:51:20 -0500 Subject: [PATCH 01/30] Midway commit. Scaling out is broken --- .../src/components/iframe/content.scss | 84 ++++++++------- .../src/components/iframe/index.js | 100 ++++++++++++++---- 2 files changed, 126 insertions(+), 58 deletions(-) diff --git a/packages/block-editor/src/components/iframe/content.scss b/packages/block-editor/src/components/iframe/content.scss index 596c177eab2f32..f8427a25f7dab9 100644 --- a/packages/block-editor/src/components/iframe/content.scss +++ b/packages/block-editor/src/components/iframe/content.scss @@ -3,55 +3,65 @@ } .block-editor-iframe__html { + $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); + $scale: var(--wp-block-editor-iframe-zoom-out-scale, 1); + transform-origin: top center; // We don't want to animate the transform of the translateX because it is used // to "center" the canvas. Leaving it on causes the canvas to slide around in // odd ways. - @include editor-canvas-resize-animation(transform 0s, scale 0s, padding 0s); + @include editor-canvas-resize-animation(transform 0s, scale 0s, padding 0s, translate 0s); &.zoom-out-animation { + position: fixed; + left: 0; + right: 0; + top: calc(-1 * #{$scroll-top}); + bottom: 0; + translate: 0 calc(#{$scroll-top} - #{$scroll-top-next}); + scale: $scale; // we only want to animate the scaling when entering zoom out. When sidebars // are toggled, the resizing of the iframe handles scaling the canvas as well, // and the doubled animations cause very odd animations. - @include editor-canvas-resize-animation(transform 0s); + @include editor-canvas-resize-animation( transform 0s, top 0s, bottom 0s, right 0s, left 0s ); } -} -.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); - // Apply an X translation to center the scaled content within the available space. - transform: translateX(calc((#{$scale-container-width} - #{$container-width}) / 2 / #{$scale})); - scale: #{$scale}; - background-color: $gray-300; - - // Chrome seems to respect that transform scale shouldn't affect the layout size of the element, - // so we need to adjust the height of the content to match the scale by using negative margins. - $extra-content-height: calc(#{$content-height} * (1 - #{$scale})); - $total-frame-height: calc(2 * #{$frame-size} / #{$scale}); - $total-height: calc(#{$extra-content-height} + #{$total-frame-height} + 2px); - margin-bottom: calc(-1 * #{$total-height}); - // Add the top/bottom frame size. We use scaling to account for the left/right, as - // the padding left/right causes the contents to reflow, which breaks the 1:1 scaling - // of the content. - padding-top: calc(#{$frame-size} / #{$scale}); - 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 { + &.is-zoomed-out { + $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); + // Apply an X translation to center the scaled content within the available space. + transform: translateX(calc((#{$scale-container-width} - #{$container-width}) / 2 / #{$scale})); + scale: $scale; + background-color: $gray-300; + + // Chrome seems to respect that transform scale shouldn't affect the layout size of the element, + // so we need to adjust the height of the content to match the scale by using negative margins. + $extra-content-height: calc(#{$content-height} * (1 - #{$scale})); + $total-frame-height: calc(2 * #{$frame-size} / #{$scale}); + $total-height: calc(#{$extra-content-height} + #{$total-frame-height} + 2px); + margin-bottom: calc(-1 * #{$total-height}); + // Add the top/bottom frame size. We use scaling to account for the left/right, as + // the padding left/right causes the contents to reflow, which breaks the 1:1 scaling + // of the content. + padding-top: calc(#{$frame-size} / #{$scale}); + 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; + } } } } diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index b14514f934fb3e..456f69f9a97793 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -128,8 +128,10 @@ function Iframe( { const [ before, writingFlowRef, after ] = useWritingFlow(); const [ contentResizeListener, { height: contentHeight } ] = useResizeObserver(); - const [ containerResizeListener, { width: containerWidth } ] = - useResizeObserver(); + const [ + containerResizeListener, + { width: containerWidth, height: containerHeight }, + ] = useResizeObserver(); const setRef = useRefEffect( ( node ) => { node._load = () => { @@ -252,6 +254,20 @@ function Iframe( { containerWidth ); + const maxWidth = 750; + const scaleValue = + scale === 'default' + ? ( Math.min( containerWidth, maxWidth ) - + parseInt( frameSize ) * 2 ) / + scaleContainerWidth + : scale; + const prevScaleRef = useRef( scaleValue ); + + const frameSizeValue = parseInt( frameSize ); + const prevFrameSizeRef = useRef( frameSizeValue ); + + const prevContainerHeightRef = useRef( containerHeight ); + const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ useBubbleEvents( iframeDocument ), @@ -303,39 +319,86 @@ function Iframe( { useEffect( () => cleanup, [ cleanup ] ); - const zoomOutAnimationClassnameRef = useRef( null ); - + const zoomOutAnimationTimeoutRef = useRef( null ); + const isAnimatingZoomOut = useRef( null ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect // that controls settings the CSS variables, but then we would need to do more work to ensure we're // only toggling these when the zoom out mode changes, as that useEffect is also triggered by a large // number of dependencies. useEffect( () => { - if ( ! iframeDocument || ! isZoomedOut ) { + // If we're animating, don't re-update things. + if ( ! iframeDocument || isAnimatingZoomOut.current ) { return; } - const handleZoomOutAnimationClassname = () => { - clearTimeout( zoomOutAnimationClassnameRef.current ); + const handleZoomOutAnimation = () => { + clearTimeout( zoomOutAnimationTimeoutRef.current ); + isAnimatingZoomOut.current = true; + + const scrollTop = iframeDocument.documentElement.scrollTop; + + // Convert previous values to the zoomed in scale. + // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. + const scrollTopOriginal = Math.round( + ( scrollTop + + prevContainerHeightRef.current / 2 - + prevFrameSizeRef.current ) / + prevScaleRef.current - + prevContainerHeightRef.current / 2 + ); + + // // Convert the zoomed in value to the new scale. + // // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. + const scrollTopNext = Math.round( + ( scrollTopOriginal + containerHeight / 2 ) * scaleValue + + frameSizeValue - + containerHeight / 2 + ); + + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top', + `${ scrollTop }px` + ); + + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top-next', + `${ scrollTopNext }px` + ); iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); - zoomOutAnimationClassnameRef.current = setTimeout( () => { + zoomOutAnimationTimeoutRef.current = setTimeout( () => { iframeDocument.documentElement.classList.remove( 'zoom-out-animation' ); + + prevContainerHeightRef.current = containerHeight; + prevFrameSizeRef.current = frameSizeValue; + prevScaleRef.current = scaleValue; + isAnimatingZoomOut.current = false; + + iframeDocument.documentElement.scrollTop = scrollTopNext; }, 400 ); // 400ms should match the animation speed used in components/iframe/content.scss }; - handleZoomOutAnimationClassname(); - iframeDocument.documentElement.classList.add( 'is-zoomed-out' ); + handleZoomOutAnimation(); - return () => { - handleZoomOutAnimationClassname(); + if ( isZoomedOut ) { + iframeDocument.documentElement.classList.add( 'is-zoomed-out' ); + } else { iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); - }; - }, [ iframeDocument, isZoomedOut ] ); + } + + return () => {}; + }, [ + iframeDocument, + isZoomedOut, + scaleValue, + frameSizeValue, + containerHeight, + ] ); // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { @@ -343,7 +406,6 @@ function Iframe( { return; } - const maxWidth = 750; // Note: When we initialize the zoom out when the canvas is smaller (sidebars open), // initialContainerWidth will be smaller than the full page, and reflow will happen // when the canvas area becomes larger due to sidebars closing. This is a known but @@ -354,11 +416,7 @@ function Iframe( { // but calc( 100px / 2px ) is not. iframeDocument.documentElement.style.setProperty( '--wp-block-editor-iframe-zoom-out-scale', - scale === 'default' - ? ( Math.min( containerWidth, maxWidth ) - - parseInt( frameSize ) * 2 ) / - scaleContainerWidth - : scale + scaleValue ); // frameSize has to be a px value for the scaling and frame size to be computed correctly. @@ -404,7 +462,7 @@ function Iframe( { ); }; }, [ - scale, + scaleValue, frameSize, iframeDocument, iframeWindowInnerHeight, From 122790d794d936736f173247ca4a8973492b0d30 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 13:57:18 -0500 Subject: [PATCH 02/30] Fix zoom in animation --- packages/block-editor/src/components/iframe/content.scss | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/iframe/content.scss b/packages/block-editor/src/components/iframe/content.scss index f8427a25f7dab9..2c2350b6994ad5 100644 --- a/packages/block-editor/src/components/iframe/content.scss +++ b/packages/block-editor/src/components/iframe/content.scss @@ -6,12 +6,12 @@ $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); $scale: var(--wp-block-editor-iframe-zoom-out-scale, 1); - + scale: $scale; transform-origin: top center; // We don't want to animate the transform of the translateX because it is used // to "center" the canvas. Leaving it on causes the canvas to slide around in // odd ways. - @include editor-canvas-resize-animation(transform 0s, scale 0s, padding 0s, translate 0s); + @include editor-canvas-resize-animation( transform 0s, padding 0s, translate 0s); &.zoom-out-animation { position: fixed; @@ -20,7 +20,6 @@ top: calc(-1 * #{$scroll-top}); bottom: 0; translate: 0 calc(#{$scroll-top} - #{$scroll-top-next}); - scale: $scale; // we only want to animate the scaling when entering zoom out. When sidebars // are toggled, the resizing of the iframe handles scaling the canvas as well, // and the doubled animations cause very odd animations. @@ -35,9 +34,7 @@ $container-width: var(--wp-block-editor-iframe-zoom-out-container-width, 100vw); // Apply an X translation to center the scaled content within the available space. transform: translateX(calc((#{$scale-container-width} - #{$container-width}) / 2 / #{$scale})); - scale: $scale; background-color: $gray-300; - // Chrome seems to respect that transform scale shouldn't affect the layout size of the element, // so we need to adjust the height of the content to match the scale by using negative margins. $extra-content-height: calc(#{$content-height} * (1 - #{$scale})); From 1dedf7a327fe4a9f0bacb58450135bbd1bdf4f40 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 14:37:46 -0500 Subject: [PATCH 03/30] Fix scaling by preserving CSS properites --- .../src/components/iframe/content.scss | 2 +- .../src/components/iframe/index.js | 23 +------------------ 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/packages/block-editor/src/components/iframe/content.scss b/packages/block-editor/src/components/iframe/content.scss index 2c2350b6994ad5..c1abf8fa1d39e3 100644 --- a/packages/block-editor/src/components/iframe/content.scss +++ b/packages/block-editor/src/components/iframe/content.scss @@ -11,7 +11,7 @@ // We don't want to animate the transform of the translateX because it is used // to "center" the canvas. Leaving it on causes the canvas to slide around in // odd ways. - @include editor-canvas-resize-animation( transform 0s, padding 0s, translate 0s); + @include editor-canvas-resize-animation( transform 0s, scale 0s, padding 0s, translate 0s); &.zoom-out-animation { position: fixed; diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 456f69f9a97793..1961237f5fdc23 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -402,7 +402,7 @@ function Iframe( { // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { - if ( ! iframeDocument || ! isZoomedOut ) { + if ( ! iframeDocument ) { return; } @@ -440,27 +440,6 @@ function Iframe( { '--wp-block-editor-iframe-zoom-out-scale-container-width', `${ scaleContainerWidth }px` ); - - return () => { - iframeDocument.documentElement.style.removeProperty( - '--wp-block-editor-iframe-zoom-out-scale' - ); - iframeDocument.documentElement.style.removeProperty( - '--wp-block-editor-iframe-zoom-out-frame-size' - ); - 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' - ); - iframeDocument.documentElement.style.removeProperty( - '--wp-block-editor-iframe-zoom-out-scale-container-width' - ); - }; }, [ scaleValue, frameSize, From 114e23b5682a2f49bc970111cf6ef233043540f5 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 14:45:47 -0500 Subject: [PATCH 04/30] Prevent reflow from removing and re adding scrollbar in animation --- packages/block-editor/src/components/iframe/content.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/block-editor/src/components/iframe/content.scss b/packages/block-editor/src/components/iframe/content.scss index c1abf8fa1d39e3..ff5bd3a4052f45 100644 --- a/packages/block-editor/src/components/iframe/content.scss +++ b/packages/block-editor/src/components/iframe/content.scss @@ -20,6 +20,9 @@ top: calc(-1 * #{$scroll-top}); bottom: 0; translate: 0 calc(#{$scroll-top} - #{$scroll-top-next}); + // 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; // we only want to animate the scaling when entering zoom out. When sidebars // are toggled, the resizing of the iframe handles scaling the canvas as well, // and the doubled animations cause very odd animations. From 39eb8dd36f9dfa7a0add51121e7fb030de21b8f1 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 31 Oct 2024 09:27:50 -0500 Subject: [PATCH 05/30] Only rerun zoom out use effects if zoom out has changed. --- .../src/components/iframe/index.js | 110 +++++++++--------- 1 file changed, 52 insertions(+), 58 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 1961237f5fdc23..b88ed31c2ce925 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -13,6 +13,7 @@ import { useMemo, useEffect, useRef, + useCallback, } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { @@ -20,6 +21,7 @@ import { useMergeRefs, useRefEffect, useDisabled, + usePrevious, } from '@wordpress/compose'; import { __experimentalStyleProvider as StyleProvider } from '@wordpress/components'; import { useSelect } from '@wordpress/data'; @@ -242,6 +244,7 @@ function Iframe( { }, [] ); const isZoomedOut = scale !== 1; + const prevIsZoomedOut = usePrevious( isZoomedOut ); useEffect( () => { if ( ! isZoomedOut ) { @@ -320,69 +323,66 @@ function Iframe( { useEffect( () => cleanup, [ cleanup ] ); const zoomOutAnimationTimeoutRef = useRef( null ); - const isAnimatingZoomOut = useRef( null ); - // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect - // that controls settings the CSS variables, but then we would need to do more work to ensure we're - // only toggling these when the zoom out mode changes, as that useEffect is also triggered by a large - // number of dependencies. - useEffect( () => { - // If we're animating, don't re-update things. - if ( ! iframeDocument || isAnimatingZoomOut.current ) { - return; - } - const handleZoomOutAnimation = () => { - clearTimeout( zoomOutAnimationTimeoutRef.current ); - isAnimatingZoomOut.current = true; + const handleZoomOutAnimation = useCallback( () => { + clearTimeout( zoomOutAnimationTimeoutRef.current ); - const scrollTop = iframeDocument.documentElement.scrollTop; + const scrollTop = iframeDocument.documentElement.scrollTop; - // Convert previous values to the zoomed in scale. - // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. - const scrollTopOriginal = Math.round( - ( scrollTop + - prevContainerHeightRef.current / 2 - - prevFrameSizeRef.current ) / - prevScaleRef.current - - prevContainerHeightRef.current / 2 - ); + // Convert previous values to the zoomed in scale. + // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. + const scrollTopOriginal = Math.round( + ( scrollTop + + prevContainerHeightRef.current / 2 - + prevFrameSizeRef.current ) / + prevScaleRef.current - + prevContainerHeightRef.current / 2 + ); - // // Convert the zoomed in value to the new scale. - // // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. - const scrollTopNext = Math.round( - ( scrollTopOriginal + containerHeight / 2 ) * scaleValue + - frameSizeValue - - containerHeight / 2 - ); + // // Convert the zoomed in value to the new scale. + // // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. + const scrollTopNext = Math.round( + ( scrollTopOriginal + containerHeight / 2 ) * scaleValue + + frameSizeValue - + containerHeight / 2 + ); - iframeDocument.documentElement.style.setProperty( - '--wp-block-editor-iframe-zoom-out-scroll-top', - `${ scrollTop }px` - ); + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top', + `${ scrollTop }px` + ); - iframeDocument.documentElement.style.setProperty( - '--wp-block-editor-iframe-zoom-out-scroll-top-next', - `${ scrollTopNext }px` - ); + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top-next', + `${ scrollTopNext }px` + ); + + iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); - iframeDocument.documentElement.classList.add( + zoomOutAnimationTimeoutRef.current = setTimeout( () => { + iframeDocument.documentElement.classList.remove( 'zoom-out-animation' ); - zoomOutAnimationTimeoutRef.current = setTimeout( () => { - iframeDocument.documentElement.classList.remove( - 'zoom-out-animation' - ); + prevContainerHeightRef.current = containerHeight; + prevFrameSizeRef.current = frameSizeValue; + prevScaleRef.current = scaleValue; - prevContainerHeightRef.current = containerHeight; - prevFrameSizeRef.current = frameSizeValue; - prevScaleRef.current = scaleValue; - isAnimatingZoomOut.current = false; + iframeDocument.documentElement.scrollTop = scrollTopNext; + }, 400 ); // 400ms should match the animation speed used in components/iframe/content.scss + }, [ scaleValue, frameSizeValue, containerHeight, iframeDocument ] ); - iframeDocument.documentElement.scrollTop = scrollTopNext; - }, 400 ); // 400ms should match the animation speed used in components/iframe/content.scss - }; + // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect + // that controls settings the CSS variables, but then we would need to do more work to ensure we're + // only toggling these when the zoom out mode changes, as that useEffect is also triggered by a large + // number of dependencies. + useEffect( () => { + // If we're animating, don't re-update things. + if ( ! iframeDocument || prevIsZoomedOut === isZoomedOut ) { + return; + } + // If zoom out mode is toggled, handle the animation handleZoomOutAnimation(); if ( isZoomedOut ) { @@ -392,17 +392,11 @@ function Iframe( { } return () => {}; - }, [ - iframeDocument, - isZoomedOut, - scaleValue, - frameSizeValue, - containerHeight, - ] ); + }, [ iframeDocument, isZoomedOut, handleZoomOutAnimation ] ); // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { - if ( ! iframeDocument ) { + if ( ! iframeDocument || prevIsZoomedOut === isZoomedOut ) { return; } From d08a6f6ae3ecaa6523172f2a1a117ef9f151500b Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 31 Oct 2024 09:35:18 -0500 Subject: [PATCH 06/30] Allow all CSS vars to update when scale changes --- packages/block-editor/src/components/iframe/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index b88ed31c2ce925..5512344e9f12aa 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -396,7 +396,7 @@ function Iframe( { // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { - if ( ! iframeDocument || prevIsZoomedOut === isZoomedOut ) { + if ( ! iframeDocument ) { return; } From 046902a582067923f61fb25fd8de1e9f2c60a01a Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 31 Oct 2024 11:58:41 -0500 Subject: [PATCH 07/30] Midway commit. Math is wrong for addressing top/bottom exceptions --- .../src/components/iframe/index.js | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 5512344e9f12aa..41a8e71ea65085 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -339,14 +339,29 @@ function Iframe( { prevContainerHeightRef.current / 2 ); - // // Convert the zoomed in value to the new scale. - // // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. - const scrollTopNext = Math.round( + // Convert the zoomed in value to the new scale. + // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. + let scrollTopNext = Math.round( ( scrollTopOriginal + containerHeight / 2 ) * scaleValue + frameSizeValue - containerHeight / 2 ); + const edgeThreshold = prevContainerHeightRef.current / 2; + const maxScrollPosition = + contentHeight - prevContainerHeightRef.current - frameSizeValue * 2; + + const scaleToTop = scrollTopOriginal - edgeThreshold <= 0; + const scaleToBottom = + scrollTopOriginal - maxScrollPosition - edgeThreshold <= 0; + + if ( scaleToTop ) { + scrollTopNext = 0; + } else if ( scaleToBottom ) { + // Not sure on this + scrollTopNext = maxScrollPosition * scaleValue; + } + iframeDocument.documentElement.style.setProperty( '--wp-block-editor-iframe-zoom-out-scroll-top', `${ scrollTop }px` @@ -370,7 +385,13 @@ function Iframe( { iframeDocument.documentElement.scrollTop = scrollTopNext; }, 400 ); // 400ms should match the animation speed used in components/iframe/content.scss - }, [ scaleValue, frameSizeValue, containerHeight, iframeDocument ] ); + }, [ + scaleValue, + frameSizeValue, + containerHeight, + iframeDocument, + contentHeight, + ] ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect // that controls settings the CSS variables, but then we would need to do more work to ensure we're From ab88dfac191cfec5310a3e5ed9e5688d854781fb Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 31 Oct 2024 14:37:33 -0500 Subject: [PATCH 08/30] Remove usePrevious usage --- packages/block-editor/src/components/iframe/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 41a8e71ea65085..ee184635146198 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -21,7 +21,6 @@ import { useMergeRefs, useRefEffect, useDisabled, - usePrevious, } from '@wordpress/compose'; import { __experimentalStyleProvider as StyleProvider } from '@wordpress/components'; import { useSelect } from '@wordpress/data'; @@ -244,7 +243,7 @@ function Iframe( { }, [] ); const isZoomedOut = scale !== 1; - const prevIsZoomedOut = usePrevious( isZoomedOut ); + const prevIsZoomedOutRef = useRef( isZoomedOut ); useEffect( () => { if ( ! isZoomedOut ) { @@ -398,6 +397,9 @@ function Iframe( { // only toggling these when the zoom out mode changes, as that useEffect is also triggered by a large // number of dependencies. useEffect( () => { + const prevIsZoomedOut = prevIsZoomedOutRef.current; + prevIsZoomedOutRef.current = isZoomedOut; + // If we're animating, don't re-update things. if ( ! iframeDocument || prevIsZoomedOut === isZoomedOut ) { return; From be9ade948491fac076a226d33f29e04e5f9b1a70 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Tue, 5 Nov 2024 15:14:07 -0600 Subject: [PATCH 09/30] Add prefers-reduced-motion to setTimeout delay --- packages/block-editor/src/components/iframe/index.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index ee184635146198..80f462c4aeed72 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -373,6 +373,14 @@ function Iframe( { iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); + // TODO: See if there's a way to wait for CSS transition to finish. + // 400ms should match the animation speed used in components/iframe/content.scss + // Ignore the delay when reduce motion is enabled. + const reduceMotion = iframeDocument.defaultView.matchMedia( + '(prefers-reduced-motion: reduce)' + ).matches; + const delay = reduceMotion ? 0 : 400; + zoomOutAnimationTimeoutRef.current = setTimeout( () => { iframeDocument.documentElement.classList.remove( 'zoom-out-animation' @@ -383,7 +391,7 @@ function Iframe( { prevScaleRef.current = scaleValue; iframeDocument.documentElement.scrollTop = scrollTopNext; - }, 400 ); // 400ms should match the animation speed used in components/iframe/content.scss + }, delay ); }, [ scaleValue, frameSizeValue, From 9f2d3844f5433f4197f0c1331d0baf3dbfb8bedf Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Tue, 5 Nov 2024 16:31:22 -0600 Subject: [PATCH 10/30] WIP Working zoom without frame size --- .../src/components/iframe/index.js | 62 ++++++++++--------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 80f462c4aeed72..46e836646c1f94 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -122,6 +122,7 @@ function Iframe( { }; }, [] ); const { styles = '', scripts = '' } = resolvedAssets; + /** @type {[Document, any]} */ const [ iframeDocument, setIframeDocument ] = useState(); const initialContainerWidth = useRef( 0 ); const [ bodyClasses, setBodyClasses ] = useState( [] ); @@ -268,7 +269,8 @@ function Iframe( { const frameSizeValue = parseInt( frameSize ); const prevFrameSizeRef = useRef( frameSizeValue ); - const prevContainerHeightRef = useRef( containerHeight ); + const prevClientHeightRef = useRef( containerHeight ); + const prevScrollHeightRef = useRef( contentHeight ); const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ @@ -326,40 +328,48 @@ function Iframe( { const handleZoomOutAnimation = useCallback( () => { clearTimeout( zoomOutAnimationTimeoutRef.current ); + // We can't trust the set value from contentHeight, as it was measured before the zoom out mode was changed. + // After zoom out mode is changed, appenders may appear or disappear, so we need to get the height from the iframe + // at this point when we're about to animate the zoom out. The iframe scrollTop, scrollHeight, and clientHeight will all + // be accurate. The client height also does change when the zoom out mode is toggled, as the bottom bar about selecting + // the template is added/removed when toggling zoom out mode. const scrollTop = iframeDocument.documentElement.scrollTop; + // This is the unscaled height of the iframe content. + const clientHeight = iframeDocument.documentElement.clientHeight; + + // This is the scaled height of the iframe content. + const scrollHeight = iframeDocument.documentElement.scrollHeight; + + const prevClientHeight = prevClientHeightRef.current; + const prevScrollHeight = prevScrollHeightRef.current; + const prevScale = prevScaleRef.current; + const prevFrameSize = prevFrameSizeRef.current; + // Convert previous values to the zoomed in scale. // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. const scrollTopOriginal = Math.round( - ( scrollTop + - prevContainerHeightRef.current / 2 - - prevFrameSizeRef.current ) / - prevScaleRef.current - - prevContainerHeightRef.current / 2 + ( scrollTop + prevClientHeight / 2 - prevFrameSize ) / prevScale - + prevClientHeight / 2 ); // Convert the zoomed in value to the new scale. // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. let scrollTopNext = Math.round( - ( scrollTopOriginal + containerHeight / 2 ) * scaleValue + + ( scrollTopOriginal + clientHeight / 2 ) * scaleValue + frameSizeValue - - containerHeight / 2 + clientHeight / 2 ); - const edgeThreshold = prevContainerHeightRef.current / 2; - const maxScrollPosition = - contentHeight - prevContainerHeightRef.current - frameSizeValue * 2; + const scaleRatio = scaleValue / prevScale; + const maxScrollTop = scrollHeight * scaleRatio - clientHeight; - const scaleToTop = scrollTopOriginal - edgeThreshold <= 0; - const scaleToBottom = - scrollTopOriginal - maxScrollPosition - edgeThreshold <= 0; + // prettier-ignore + scrollTopNext = scrollTopNext - clientHeight / 2 <= 0 ? 0 : scrollTopNext; + // prettier-ignore + scrollTopNext = scrollTopNext + clientHeight / 2 >= maxScrollTop ? maxScrollTop : scrollTopNext; - if ( scaleToTop ) { - scrollTopNext = 0; - } else if ( scaleToBottom ) { - // Not sure on this - scrollTopNext = maxScrollPosition * scaleValue; - } + scrollTopNext = Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ); iframeDocument.documentElement.style.setProperty( '--wp-block-editor-iframe-zoom-out-scroll-top', @@ -386,19 +396,13 @@ function Iframe( { 'zoom-out-animation' ); - prevContainerHeightRef.current = containerHeight; + prevClientHeightRef.current = clientHeight; + prevScrollHeightRef.current = scrollHeight; prevFrameSizeRef.current = frameSizeValue; prevScaleRef.current = scaleValue; - iframeDocument.documentElement.scrollTop = scrollTopNext; }, delay ); - }, [ - scaleValue, - frameSizeValue, - containerHeight, - iframeDocument, - contentHeight, - ] ); + }, [ scaleValue, frameSizeValue, iframeDocument ] ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect // that controls settings the CSS variables, but then we would need to do more work to ensure we're From c4f97ba0717eb99876fdf14e049e62d796b5dda4 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 6 Nov 2024 09:13:01 -0600 Subject: [PATCH 11/30] Account for changes to client height when determining edge threshold --- packages/block-editor/src/components/iframe/index.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 46e836646c1f94..8a243bd810a16c 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -342,7 +342,7 @@ function Iframe( { const scrollHeight = iframeDocument.documentElement.scrollHeight; const prevClientHeight = prevClientHeightRef.current; - const prevScrollHeight = prevScrollHeightRef.current; + // const prevScrollHeight = prevScrollHeightRef.current; const prevScale = prevScaleRef.current; const prevFrameSize = prevFrameSizeRef.current; @@ -364,10 +364,14 @@ function Iframe( { const scaleRatio = scaleValue / prevScale; const maxScrollTop = scrollHeight * scaleRatio - clientHeight; + // Account for differences in client height changes between zoom in and zoom out modes. + const edgeThreshold = + clientHeight / 2 + ( prevClientHeight - clientHeight ); + // prettier-ignore - scrollTopNext = scrollTopNext - clientHeight / 2 <= 0 ? 0 : scrollTopNext; + scrollTopNext = scrollTopNext - edgeThreshold <= 0 ? 0 : scrollTopNext; // prettier-ignore - scrollTopNext = scrollTopNext + clientHeight / 2 >= maxScrollTop ? maxScrollTop : scrollTopNext; + scrollTopNext = scrollTopNext + edgeThreshold >= maxScrollTop ? maxScrollTop : scrollTopNext; scrollTopNext = Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ); From 8bc12b2ff3b06742f98d73637ad69a62c0b6d5f5 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 6 Nov 2024 11:19:37 -0600 Subject: [PATCH 12/30] Zoom to center unless it will reveal top or bottom --- .../src/components/iframe/index.js | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 8a243bd810a16c..3e516079b6de56 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -270,7 +270,6 @@ function Iframe( { const prevFrameSizeRef = useRef( frameSizeValue ); const prevClientHeightRef = useRef( containerHeight ); - const prevScrollHeightRef = useRef( contentHeight ); const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ @@ -342,7 +341,6 @@ function Iframe( { const scrollHeight = iframeDocument.documentElement.scrollHeight; const prevClientHeight = prevClientHeightRef.current; - // const prevScrollHeight = prevScrollHeightRef.current; const prevScale = prevScaleRef.current; const prevFrameSize = prevFrameSizeRef.current; @@ -362,18 +360,14 @@ function Iframe( { ); const scaleRatio = scaleValue / prevScale; - const maxScrollTop = scrollHeight * scaleRatio - clientHeight; + const maxScrollTop = + scrollHeight * scaleRatio - clientHeight + frameSizeValue * 2; - // Account for differences in client height changes between zoom in and zoom out modes. - const edgeThreshold = - clientHeight / 2 + ( prevClientHeight - clientHeight ); - - // prettier-ignore - scrollTopNext = scrollTopNext - edgeThreshold <= 0 ? 0 : scrollTopNext; - // prettier-ignore - scrollTopNext = scrollTopNext + edgeThreshold >= maxScrollTop ? maxScrollTop : scrollTopNext; - - scrollTopNext = Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ); + // scrollTopNext will zoom to the center point unless it would scroll past the top or bottom. + // In that case, it will clamp to the top or bottom. + scrollTopNext = Math.round( + Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ) + ); iframeDocument.documentElement.style.setProperty( '--wp-block-editor-iframe-zoom-out-scroll-top', @@ -401,7 +395,6 @@ function Iframe( { ); prevClientHeightRef.current = clientHeight; - prevScrollHeightRef.current = scrollHeight; prevFrameSizeRef.current = frameSizeValue; prevScaleRef.current = scaleValue; iframeDocument.documentElement.scrollTop = scrollTopNext; From fffc48d73701ede69e5ea477fdc43aed5f75b997 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 6 Nov 2024 11:28:37 -0600 Subject: [PATCH 13/30] Account for a top threshold when zooming in and out --- packages/block-editor/src/components/iframe/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 3e516079b6de56..09ee0ddf09cf67 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -359,6 +359,9 @@ function Iframe( { clientHeight / 2 ); + // If we are near the top of the canvas, set the next scroll top to 0. + scrollTopNext = scrollTop <= prevFrameSize ? 0 : scrollTopNext; + const scaleRatio = scaleValue / prevScale; const maxScrollTop = scrollHeight * scaleRatio - clientHeight + frameSizeValue * 2; From 42dd0308cf52989d1699e1648f1cb6aad921e49b Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 14:59:07 -0600 Subject: [PATCH 14/30] Clean up math and add comments --- .../src/components/iframe/index.js | 80 ++++++++++++------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 09ee0ddf09cf67..d8fa6511d3193a 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -327,47 +327,65 @@ function Iframe( { const handleZoomOutAnimation = useCallback( () => { clearTimeout( zoomOutAnimationTimeoutRef.current ); - // We can't trust the set value from contentHeight, as it was measured before the zoom out mode was changed. - // After zoom out mode is changed, appenders may appear or disappear, so we need to get the height from the iframe - // at this point when we're about to animate the zoom out. The iframe scrollTop, scrollHeight, and clientHeight will all - // be accurate. The client height also does change when the zoom out mode is toggled, as the bottom bar about selecting - // the template is added/removed when toggling zoom out mode. - const scrollTop = iframeDocument.documentElement.scrollTop; - - // This is the unscaled height of the iframe content. - const clientHeight = iframeDocument.documentElement.clientHeight; - - // This is the scaled height of the iframe content. - const scrollHeight = iframeDocument.documentElement.scrollHeight; + // Previous scale value. + const prevScale = prevScaleRef.current; + // Unscaled height of the previous iframe container. const prevClientHeight = prevClientHeightRef.current; - const prevScale = prevScaleRef.current; + + // Unscaled size of the previous padding around the iframe content. const prevFrameSize = prevFrameSizeRef.current; - // Convert previous values to the zoomed in scale. - // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. - const scrollTopOriginal = Math.round( - ( scrollTop + prevClientHeight / 2 - prevFrameSize ) / prevScale - - prevClientHeight / 2 - ); + // Unscaled height of the current iframe container. + const clientHeight = iframeDocument.documentElement.clientHeight; - // Convert the zoomed in value to the new scale. - // Use Math.round to avoid subpixel scrolling which would effectively result in a Math.floor. - let scrollTopNext = Math.round( - ( scrollTopOriginal + clientHeight / 2 ) * scaleValue + - frameSizeValue - - clientHeight / 2 - ); + // Scaled height of the current iframe content. + const scrollHeight = iframeDocument.documentElement.scrollHeight; + + // We can't trust the set value from contentHeight, as it was measured + // before the zoom out mode was changed. After zoom out mode is changed, + // appenders may appear or disappear, so we need to get the height from + // the iframe at this point when we're about to animate the zoom out. + // The iframe scrollTop, scrollHeight, and clientHeight will all be + // accurate. The client height also does change when the zoom out mode + // is toggled, as the bottom bar about selecting the template is + // added/removed when toggling zoom out mode. + const scrollTop = iframeDocument.documentElement.scrollTop; - // If we are near the top of the canvas, set the next scroll top to 0. + // Step 0: Start with the current scrollTop. + let scrollTopNext = scrollTop; + + // Step 1: Undo the effects of the previous scale and frame around the + // midpoint of the visible area. + scrollTopNext = + ( scrollTopNext + prevClientHeight / 2 - prevFrameSize ) / + prevScale - + prevClientHeight / 2; + + // Step 2: Apply the new scale and frame around the midpoint of the + // visible area. + scrollTopNext = + ( scrollTopNext + clientHeight / 2 ) * scaleValue + + frameSizeValue - + clientHeight / 2; + + // Step 3: Handle an edge case so that you scroll to the top of the + // 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; - const scaleRatio = scaleValue / prevScale; + // 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 * scaleRatio - clientHeight + frameSizeValue * 2; + scrollHeight * ( scaleValue / prevScale ) + + frameSizeValue * 2 - + clientHeight; - // scrollTopNext will zoom to the center point unless it would scroll past the top or bottom. - // In that case, it will clamp to the top or bottom. + // Step 4: Clamp the scrollTopNext between the minimum and maximum + // possible scrollTop positions. Round the value to avoid subpixel + // truncation by the browser which sometimes causes a 1px error. scrollTopNext = Math.round( Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ) ); From 5fff8f9d5a4cf54547c3aaff9a004a4cdcf9cefb Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 15:32:11 -0600 Subject: [PATCH 15/30] Add event listener instead of timeout --- .../src/components/iframe/index.js | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index d8fa6511d3193a..96835a8d6eb45f 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -390,6 +390,16 @@ function Iframe( { Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ) ); + const reduceMotion = iframeDocument.defaultView.matchMedia( + '(prefers-reduced-motion: reduce)' + ).matches; + + if ( reduceMotion ) { + // TODO: This seems to be broken somehow. + iframeDocument.documentElement.scrollTop = scrollTopNext; + return; + } + iframeDocument.documentElement.style.setProperty( '--wp-block-editor-iframe-zoom-out-scroll-top', `${ scrollTop }px` @@ -402,24 +412,24 @@ function Iframe( { iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); - // TODO: See if there's a way to wait for CSS transition to finish. - // 400ms should match the animation speed used in components/iframe/content.scss - // Ignore the delay when reduce motion is enabled. - const reduceMotion = iframeDocument.defaultView.matchMedia( - '(prefers-reduced-motion: reduce)' - ).matches; - const delay = reduceMotion ? 0 : 400; + iframeDocument.documentElement.addEventListener( + 'transitionend', + () => { + // Remove the position fixed for the animation. + iframeDocument.documentElement.classList.remove( + 'zoom-out-animation' + ); - zoomOutAnimationTimeoutRef.current = setTimeout( () => { - iframeDocument.documentElement.classList.remove( - 'zoom-out-animation' - ); + // Update previous values. + prevClientHeightRef.current = clientHeight; + prevFrameSizeRef.current = frameSizeValue; + prevScaleRef.current = scaleValue; - prevClientHeightRef.current = clientHeight; - prevFrameSizeRef.current = frameSizeValue; - prevScaleRef.current = scaleValue; - iframeDocument.documentElement.scrollTop = scrollTopNext; - }, delay ); + // Set the final scroll position that was just animated to. + iframeDocument.documentElement.scrollTop = scrollTopNext; + }, + { once: true } + ); }, [ scaleValue, frameSizeValue, iframeDocument ] ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect From c182b70e2f7509e602dcb38115a3bf38128d92c6 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 15:50:11 -0600 Subject: [PATCH 16/30] Fix reduced motion --- .../src/components/iframe/index.js | 68 ++++++++++--------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 96835a8d6eb45f..4ebba65f6951d2 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -32,6 +32,7 @@ import { useBlockSelectionClearer } from '../block-selection-clearer'; import { useWritingFlow } from '../writing-flow'; import { getCompatibilityStyles } from './get-compatibility-styles'; import { store as blockEditorStore } from '../../store'; +import { handle } from '@wordpress/icons'; function bubbleEvent( event, Constructor, frame ) { const init = {}; @@ -390,46 +391,49 @@ function Iframe( { Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ) ); - const reduceMotion = iframeDocument.defaultView.matchMedia( - '(prefers-reduced-motion: reduce)' - ).matches; + function handleZoomOutEnd() { + // Update previous values. + prevClientHeightRef.current = clientHeight; + prevFrameSizeRef.current = frameSizeValue; + prevScaleRef.current = scaleValue; - if ( reduceMotion ) { - // TODO: This seems to be broken somehow. + // Set the final scroll position that was just animated to. iframeDocument.documentElement.scrollTop = scrollTopNext; - return; } - iframeDocument.documentElement.style.setProperty( - '--wp-block-editor-iframe-zoom-out-scroll-top', - `${ scrollTop }px` - ); - - iframeDocument.documentElement.style.setProperty( - '--wp-block-editor-iframe-zoom-out-scroll-top-next', - `${ scrollTopNext }px` - ); + const reduceMotion = iframeDocument.defaultView.matchMedia( + '(prefers-reduced-motion: reduce)' + ).matches; - iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); + if ( reduceMotion ) { + handleZoomOutEnd(); + } else { + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top', + `${ scrollTop }px` + ); - iframeDocument.documentElement.addEventListener( - 'transitionend', - () => { - // Remove the position fixed for the animation. - iframeDocument.documentElement.classList.remove( - 'zoom-out-animation' - ); + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top-next', + `${ scrollTopNext }px` + ); - // Update previous values. - prevClientHeightRef.current = clientHeight; - prevFrameSizeRef.current = frameSizeValue; - prevScaleRef.current = scaleValue; + iframeDocument.documentElement.classList.add( + 'zoom-out-animation' + ); - // Set the final scroll position that was just animated to. - iframeDocument.documentElement.scrollTop = scrollTopNext; - }, - { once: true } - ); + iframeDocument.documentElement.addEventListener( + 'transitionend', + () => { + // Remove the position fixed for the animation. + iframeDocument.documentElement.classList.remove( + 'zoom-out-animation' + ); + handleZoomOutEnd(); + }, + { once: true } + ); + } }, [ scaleValue, frameSizeValue, iframeDocument ] ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect From a9211e72d76c119566af2aaaaff8d71c9ae13679 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 15:55:39 -0600 Subject: [PATCH 17/30] Remove timeout ref --- packages/block-editor/src/components/iframe/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 4ebba65f6951d2..2212d3287cf5b7 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -323,11 +323,7 @@ function Iframe( { useEffect( () => cleanup, [ cleanup ] ); - const zoomOutAnimationTimeoutRef = useRef( null ); - const handleZoomOutAnimation = useCallback( () => { - clearTimeout( zoomOutAnimationTimeoutRef.current ); - // Previous scale value. const prevScale = prevScaleRef.current; From 6c91090b4ec903975eefe83983c05cbbecefbdeb Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 16:16:43 -0600 Subject: [PATCH 18/30] Refactor callback as separate effect --- .../src/components/iframe/index.js | 85 +++++++++++-------- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 2212d3287cf5b7..eeb0e4647e3ece 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -13,7 +13,6 @@ import { useMemo, useEffect, useRef, - useCallback, } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { @@ -32,7 +31,6 @@ import { useBlockSelectionClearer } from '../block-selection-clearer'; import { useWritingFlow } from '../writing-flow'; import { getCompatibilityStyles } from './get-compatibility-styles'; import { store as blockEditorStore } from '../../store'; -import { handle } from '@wordpress/icons'; function bubbleEvent( event, Constructor, frame ) { const init = {}; @@ -245,7 +243,6 @@ function Iframe( { }, [] ); const isZoomedOut = scale !== 1; - const prevIsZoomedOutRef = useRef( isZoomedOut ); useEffect( () => { if ( ! isZoomedOut ) { @@ -323,7 +320,15 @@ function Iframe( { useEffect( () => cleanup, [ cleanup ] ); - const handleZoomOutAnimation = useCallback( () => { + useEffect( () => { + if ( + ! iframeDocument || + // TODO: What should this condition be? + ( scaleValue === 1 ) === ( prevScaleRef.current === 1 ) + ) { + return; + } + // Previous scale value. const prevScale = prevScaleRef.current; @@ -387,7 +392,23 @@ function Iframe( { Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ) ); - function handleZoomOutEnd() { + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top', + `${ scrollTop }px` + ); + + iframeDocument.documentElement.style.setProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top-next', + `${ scrollTopNext }px` + ); + + iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); + + function onZoomOutTransitionEnd() { + // Remove the position fixed for the animation. + iframeDocument.documentElement.classList.remove( + 'zoom-out-animation' + ); // Update previous values. prevClientHeightRef.current = clientHeight; prevFrameSizeRef.current = frameSizeValue; @@ -402,60 +423,52 @@ function Iframe( { ).matches; if ( reduceMotion ) { - handleZoomOutEnd(); + onZoomOutTransitionEnd(); } else { - iframeDocument.documentElement.style.setProperty( - '--wp-block-editor-iframe-zoom-out-scroll-top', - `${ scrollTop }px` + iframeDocument.documentElement.addEventListener( + 'transitionend', + onZoomOutTransitionEnd, + { once: true } ); + } - iframeDocument.documentElement.style.setProperty( - '--wp-block-editor-iframe-zoom-out-scroll-top-next', - `${ scrollTopNext }px` + return () => { + iframeDocument.documentElement.style.removeProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top' ); - - iframeDocument.documentElement.classList.add( + iframeDocument.documentElement.style.removeProperty( + '--wp-block-editor-iframe-zoom-out-scroll-top-next' + ); + iframeDocument.documentElement.classList.remove( 'zoom-out-animation' ); - - iframeDocument.documentElement.addEventListener( + iframeDocument.documentElement.removeEventListener( 'transitionend', - () => { - // Remove the position fixed for the animation. - iframeDocument.documentElement.classList.remove( - 'zoom-out-animation' - ); - handleZoomOutEnd(); - }, - { once: true } + onZoomOutTransitionEnd ); - } - }, [ scaleValue, frameSizeValue, iframeDocument ] ); + }; + }, [ iframeDocument, scaleValue, frameSizeValue ] ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect // that controls settings the CSS variables, but then we would need to do more work to ensure we're // only toggling these when the zoom out mode changes, as that useEffect is also triggered by a large // number of dependencies. useEffect( () => { - const prevIsZoomedOut = prevIsZoomedOutRef.current; - prevIsZoomedOutRef.current = isZoomedOut; - - // If we're animating, don't re-update things. - if ( ! iframeDocument || prevIsZoomedOut === isZoomedOut ) { + if ( ! iframeDocument ) { return; } - // If zoom out mode is toggled, handle the animation - handleZoomOutAnimation(); - if ( isZoomedOut ) { iframeDocument.documentElement.classList.add( 'is-zoomed-out' ); } else { iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); } - return () => {}; - }, [ iframeDocument, isZoomedOut, handleZoomOutAnimation ] ); + return () => { + // TODO: Removing this causes issues with the zoom out animation. + // iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); + }; + }, [ iframeDocument, isZoomedOut ] ); // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { From 615f2b3ed9a62aec3f86cf0e6a926250b3bbde49 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 16:28:34 -0600 Subject: [PATCH 19/30] Fix JSDoc comment --- packages/block-editor/src/components/iframe/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index eeb0e4647e3ece..b1648785b99c5f 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -121,7 +121,7 @@ function Iframe( { }; }, [] ); const { styles = '', scripts = '' } = resolvedAssets; - /** @type {[Document, any]} */ + /** @type {[Document, import('react').Dispatch]} */ const [ iframeDocument, setIframeDocument ] = useState(); const initialContainerWidth = useRef( 0 ); const [ bodyClasses, setBodyClasses ] = useState( [] ); From 907dde4d68a151fe14d3737f35faa645d7ffa7dd Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 16:28:50 -0600 Subject: [PATCH 20/30] Try to add back useEffect cleanups --- .../src/components/iframe/index.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index b1648785b99c5f..5accabc6b9ba06 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -510,6 +510,28 @@ function Iframe( { '--wp-block-editor-iframe-zoom-out-scale-container-width', `${ scaleContainerWidth }px` ); + + return () => { + // TODO: Removing this causes issues with the zoom out animation. + // iframeDocument.documentElement.style.removeProperty( + // '--wp-block-editor-iframe-zoom-out-scale' + // ); + // iframeDocument.documentElement.style.removeProperty( + // '--wp-block-editor-iframe-zoom-out-frame-size' + // ); + // 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' + // ); + // iframeDocument.documentElement.style.removeProperty( + // '--wp-block-editor-iframe-zoom-out-scale-container-width' + // ); + }; }, [ scaleValue, frameSize, From fe8fd4e7ea0a2d16991d991f1a0d3550b0bc374c Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 7 Nov 2024 10:22:03 -0600 Subject: [PATCH 21/30] Initialize prevClientHeight in the useEffect --- .../src/components/iframe/index.js | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 5accabc6b9ba06..2226d32107e4d9 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -129,10 +129,8 @@ function Iframe( { const [ before, writingFlowRef, after ] = useWritingFlow(); const [ contentResizeListener, { height: contentHeight } ] = useResizeObserver(); - const [ - containerResizeListener, - { width: containerWidth, height: containerHeight }, - ] = useResizeObserver(); + const [ containerResizeListener, { width: containerWidth } ] = + useResizeObserver(); const setRef = useRefEffect( ( node ) => { node._load = () => { @@ -267,7 +265,8 @@ function Iframe( { const frameSizeValue = parseInt( frameSize ); const prevFrameSizeRef = useRef( frameSizeValue ); - const prevClientHeightRef = useRef( containerHeight ); + // Initialized in the useEffect. + const prevClientHeightRef = useRef(); const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ @@ -329,20 +328,20 @@ function Iframe( { return; } + // Unscaled height of the current iframe container. + const clientHeight = iframeDocument.documentElement.clientHeight; + + // Scaled height of the current iframe content. + const scrollHeight = iframeDocument.documentElement.scrollHeight; + // Previous scale value. const prevScale = prevScaleRef.current; - // Unscaled height of the previous iframe container. - const prevClientHeight = prevClientHeightRef.current; - // Unscaled size of the previous padding around the iframe content. const prevFrameSize = prevFrameSizeRef.current; - // Unscaled height of the current iframe container. - const clientHeight = iframeDocument.documentElement.clientHeight; - - // Scaled height of the current iframe content. - const scrollHeight = iframeDocument.documentElement.scrollHeight; + // Unscaled height of the previous iframe container. + const prevClientHeight = prevClientHeightRef.current ?? clientHeight; // We can't trust the set value from contentHeight, as it was measured // before the zoom out mode was changed. After zoom out mode is changed, From 624be0154f836c87d0427367b87f944bc66b93ab Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 7 Nov 2024 10:52:03 -0600 Subject: [PATCH 22/30] use useReducedMotion --- packages/block-editor/src/components/iframe/index.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 2226d32107e4d9..20f37e5ee787aa 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -20,6 +20,7 @@ import { useMergeRefs, useRefEffect, useDisabled, + useReducedMotion, } from '@wordpress/compose'; import { __experimentalStyleProvider as StyleProvider } from '@wordpress/components'; import { useSelect } from '@wordpress/data'; @@ -131,6 +132,7 @@ function Iframe( { useResizeObserver(); const [ containerResizeListener, { width: containerWidth } ] = useResizeObserver(); + const prefersReducedMotion = useReducedMotion(); const setRef = useRefEffect( ( node ) => { node._load = () => { @@ -417,11 +419,7 @@ function Iframe( { iframeDocument.documentElement.scrollTop = scrollTopNext; } - const reduceMotion = iframeDocument.defaultView.matchMedia( - '(prefers-reduced-motion: reduce)' - ).matches; - - if ( reduceMotion ) { + if ( prefersReducedMotion ) { onZoomOutTransitionEnd(); } else { iframeDocument.documentElement.addEventListener( @@ -446,7 +444,7 @@ function Iframe( { onZoomOutTransitionEnd ); }; - }, [ iframeDocument, scaleValue, frameSizeValue ] ); + }, [ iframeDocument, scaleValue, frameSizeValue, prefersReducedMotion ] ); // Toggle zoom out CSS Classes only when zoom out mode changes. We could add these into the useEffect // that controls settings the CSS variables, but then we would need to do more work to ensure we're From b9eb0d21e5e270fc020d578b3525268fbd90dae2 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 7 Nov 2024 15:32:43 -0600 Subject: [PATCH 23/30] Add test for zoom in/out location --- test/e2e/specs/site-editor/zoom-out.spec.js | 194 +++++++++++++++++--- 1 file changed, 171 insertions(+), 23 deletions(-) diff --git a/test/e2e/specs/site-editor/zoom-out.spec.js b/test/e2e/specs/site-editor/zoom-out.spec.js index 53b777a2545a34..2a299fd474c737 100644 --- a/test/e2e/specs/site-editor/zoom-out.spec.js +++ b/test/e2e/specs/site-editor/zoom-out.spec.js @@ -3,44 +3,83 @@ */ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); +const EDITOR_ZOOM_OUT_CONTENT = ` + +
+

First Section Start

+ + + +

First Section Center

+ + + +

First Section End

+
+ + + +
+

Second Section Start

+ + + +

Second Section Center

+ + + +

Second Section End

+
+ + + +
+

Third Section Start

+ + + +

Third Section Center

+ + + +

Third Section End

+
+ + + +
+

Fourth Section Start

+ + + +

Fourth Section Center

+ + + +

Fourth Section End

+
+`; + // The test is flaky and fails almost consistently. // See: https://github.com/WordPress/gutenberg/issues/61806. // eslint-disable-next-line playwright/no-skipped-test -test.describe.skip( 'Zoom Out', () => { +test.describe( 'Zoom Out', () => { test.beforeAll( async ( { requestUtils } ) => { await requestUtils.activateTheme( 'emptytheme' ); } ); - test.beforeEach( async ( { admin, editor, page } ) => { - await admin.visitAdminPage( 'admin.php', 'page=gutenberg-experiments' ); - - const zoomedOutCheckbox = page.getByLabel( - 'Enable zoomed out view when selecting a pattern category in the main inserter.' - ); - - await zoomedOutCheckbox.setChecked( true ); - await expect( zoomedOutCheckbox ).toBeChecked(); - await page.getByRole( 'button', { name: 'Save Changes' } ).click(); - + test.beforeEach( async ( { admin, editor } ) => { await admin.visitSiteEditor(); await editor.canvas.locator( 'body' ).click(); - } ); - - test.afterEach( async ( { admin, page } ) => { - await admin.visitAdminPage( 'admin.php', 'page=gutenberg-experiments' ); - const zoomedOutCheckbox = page.getByLabel( - 'Enable zoomed out view when selecting a pattern category in the main inserter.' - ); - await zoomedOutCheckbox.setChecked( false ); - await expect( zoomedOutCheckbox ).not.toBeChecked(); - await page.getByRole( 'button', { name: 'Save Changes' } ).click(); + // Add some patterns into the page. + await editor.setContent( EDITOR_ZOOM_OUT_CONTENT ); } ); test.afterAll( async ( { requestUtils } ) => { await requestUtils.activateTheme( 'twentytwentyone' ); } ); - test( 'Clicking on inserter while on zoom-out should open the patterns tab on the inserter', async ( { + test.skip( 'Clicking on inserter while on zoom-out should open the patterns tab on the inserter', async ( { page, } ) => { // Trigger zoom out on Global Styles because there the inserter is not open. @@ -64,4 +103,113 @@ test.describe.skip( 'Zoom Out', () => { .nth( 1 ) ).toBeVisible(); } ); + + test( 'Toggling zoom state should keep content centered', async ( { + page, + editor, + } ) => { + // Find the scroll container element + await page.evaluate( () => { + const { activeElement } = + document.activeElement?.contentDocument ?? document; + window.scrollContainer = + window.wp.dom.getScrollContainer( activeElement ); + return window.scrollContainer; + } ); + + // Test: Test from top of page (scrollTop 0) + // Enter Zoom Out + await page.getByRole( 'button', { name: 'Zoom Out' } ).click(); + + const scrollTopZoomed = await page.evaluate( () => { + return window.scrollContainer.scrollTop; + } ); + + expect( scrollTopZoomed ).toBe( 0 ); + + // Exit Zoom Out + await page.getByRole( 'button', { name: 'Zoom Out' } ).click(); + + const scrollTopNoZoom = await page.evaluate( () => { + return window.scrollContainer.scrollTop; + } ); + + expect( scrollTopNoZoom ).toBe( 0 ); + + // Test: Should center the scroll position when zooming out/in + const firstSectionEnd = editor.canvas.locator( + 'text=First Section End' + ); + const secondSectionStart = editor.canvas.locator( + 'text=Second Section Start' + ); + const secondSectionCenter = editor.canvas.locator( + 'text=Second Section Center' + ); + const secondSectionEnd = editor.canvas.locator( + 'text=Second Section End' + ); + const thirdSectionStart = editor.canvas.locator( + 'text=Third Section Start' + ); + const thirdSectionCenter = editor.canvas.locator( + 'text=Third Section Center' + ); + const thirdSectionEnd = editor.canvas.locator( + 'text=Third Section End' + ); + const fourthSectionStart = editor.canvas.locator( + 'text=Fourth Section Start' + ); + + // Test for second section + // Playwright scrolls it to the center of the viewport, so this is what we scroll to. + await secondSectionCenter.scrollIntoViewIfNeeded(); + + // Because the text is spread with a group height of 100vh, they should both be visible. + await expect( firstSectionEnd ).not.toBeInViewport(); + await expect( secondSectionStart ).toBeInViewport(); + await expect( secondSectionEnd ).toBeInViewport(); + await expect( thirdSectionStart ).not.toBeInViewport(); + + // After zooming, if we zoomed out with the correct central point, they should both still be visible when toggling zoom out state + // Enter Zoom Out + await page.getByRole( 'button', { name: 'Zoom Out' } ).click(); + await expect( firstSectionEnd ).toBeInViewport(); + await expect( secondSectionStart ).toBeInViewport(); + await expect( secondSectionEnd ).toBeInViewport(); + await expect( thirdSectionStart ).toBeInViewport(); + + // Exit Zoom Out + await page.getByRole( 'button', { name: 'Zoom Out' } ).click(); + await expect( firstSectionEnd ).not.toBeInViewport(); + await expect( secondSectionStart ).toBeInViewport(); + await expect( secondSectionEnd ).toBeInViewport(); + await expect( thirdSectionStart ).not.toBeInViewport(); + + // Test for third section + // Playwright scrolls it to the center of the viewport, so this is what we scroll to. + await thirdSectionCenter.scrollIntoViewIfNeeded(); + + // Because the text is spread with a group height of 100vh, they should both be visible. + await expect( secondSectionEnd ).not.toBeInViewport(); + await expect( thirdSectionStart ).toBeInViewport(); + await expect( thirdSectionEnd ).toBeInViewport(); + await expect( fourthSectionStart ).not.toBeInViewport(); + + // After zooming, if we zoomed out with the correct central point, they should both still be visible when toggling zoom out state + // Enter Zoom Out + await page.getByRole( 'button', { name: 'Zoom Out' } ).click(); + await expect( secondSectionEnd ).toBeInViewport(); + await expect( thirdSectionStart ).toBeInViewport(); + await expect( thirdSectionEnd ).toBeInViewport(); + await expect( fourthSectionStart ).toBeInViewport(); + + // Exit Zoom Out + await page.getByRole( 'button', { name: 'Zoom Out' } ).click(); + await expect( secondSectionEnd ).not.toBeInViewport(); + await expect( thirdSectionStart ).toBeInViewport(); + await expect( thirdSectionEnd ).toBeInViewport(); + await expect( fourthSectionStart ).not.toBeInViewport(); + } ); } ); From d51db3854817c53b447124b84e6688721f628a89 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 11:13:08 -0600 Subject: [PATCH 24/30] Hack to fix reduced motion --- packages/block-editor/src/components/iframe/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 20f37e5ee787aa..f7158eda1e3f20 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -420,7 +420,10 @@ function Iframe( { } if ( prefersReducedMotion ) { - onZoomOutTransitionEnd(); + // Hack: Wait for the window values to recalculate. + iframeDocument.defaultView.requestAnimationFrame( + onZoomOutTransitionEnd + ); } else { iframeDocument.documentElement.addEventListener( 'transitionend', From 1e7f1eed5a54dc138b5f7f776689a6e1afe25329 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:24:22 -0600 Subject: [PATCH 25/30] Clean up the frameSizeValue and scaleValue calculations --- packages/block-editor/src/components/iframe/index.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index f7158eda1e3f20..94e5dfb149fd2a 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -255,20 +255,18 @@ function Iframe( { containerWidth ); + const frameSizeValue = parseInt( frameSize ); + const maxWidth = 750; const scaleValue = scale === 'default' - ? ( Math.min( containerWidth, maxWidth ) - - parseInt( frameSize ) * 2 ) / + ? ( Math.min( containerWidth, maxWidth ) - frameSizeValue * 2 ) / scaleContainerWidth : scale; - const prevScaleRef = useRef( scaleValue ); - const frameSizeValue = parseInt( frameSize ); + const prevScaleRef = useRef( scaleValue ); const prevFrameSizeRef = useRef( frameSizeValue ); - - // Initialized in the useEffect. - const prevClientHeightRef = useRef(); + const prevClientHeightRef = useRef( /* Initialized in the useEffect. */ ); const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ From 740cd9a485e6d4bde157621e5a45229b9ed79f1f Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:24:43 -0600 Subject: [PATCH 26/30] Replace TODO comments with HACK comments --- packages/block-editor/src/components/iframe/index.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 94e5dfb149fd2a..d4ba05aedd29bf 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -322,7 +322,8 @@ function Iframe( { useEffect( () => { if ( ! iframeDocument || - // TODO: What should this condition be? + // HACK: Checking if isZoomedOut differs from prevIsZoomedOut here + // instead of the dependency array to appease the linter. ( scaleValue === 1 ) === ( prevScaleRef.current === 1 ) ) { return; @@ -463,7 +464,8 @@ function Iframe( { } return () => { - // TODO: Removing this causes issues with the zoom out animation. + // HACK: Skipping cleanup because it causes issues with the zoom out + // animation. More refactoring is needed to fix this properly. // iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); }; }, [ iframeDocument, isZoomedOut ] ); @@ -510,7 +512,8 @@ function Iframe( { ); return () => { - // TODO: Removing this causes issues with the zoom out animation. + // HACK: Skipping cleanup because it causes issues with the zoom out + // animation. More refactoring is needed to fix this properly. // iframeDocument.documentElement.style.removeProperty( // '--wp-block-editor-iframe-zoom-out-scale' // ); From 37111fae1b2c22f71224b521388957752fa65dc8 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:32:32 -0600 Subject: [PATCH 27/30] Add cleanup for raf --- .../block-editor/src/components/iframe/index.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index d4ba05aedd29bf..054f27418eed61 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -409,6 +409,7 @@ function Iframe( { iframeDocument.documentElement.classList.remove( 'zoom-out-animation' ); + // Update previous values. prevClientHeightRef.current = clientHeight; prevFrameSizeRef.current = frameSizeValue; @@ -418,9 +419,10 @@ function Iframe( { iframeDocument.documentElement.scrollTop = scrollTopNext; } + let raf; if ( prefersReducedMotion ) { // Hack: Wait for the window values to recalculate. - iframeDocument.defaultView.requestAnimationFrame( + raf = iframeDocument.defaultView.requestAnimationFrame( onZoomOutTransitionEnd ); } else { @@ -441,10 +443,14 @@ function Iframe( { iframeDocument.documentElement.classList.remove( 'zoom-out-animation' ); - iframeDocument.documentElement.removeEventListener( - 'transitionend', - onZoomOutTransitionEnd - ); + if ( prefersReducedMotion ) { + iframeDocument.defaultView.cancelAnimationFrame( raf ); + } else { + iframeDocument.documentElement.removeEventListener( + 'transitionend', + onZoomOutTransitionEnd + ); + } }; }, [ iframeDocument, scaleValue, frameSizeValue, prefersReducedMotion ] ); From ce8665d8ba045221d56ca7208d94f616b5d5fc04 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:43:47 -0600 Subject: [PATCH 28/30] Simplify CSS diff for 6.7 review --- .../src/components/iframe/content.scss | 73 ++++++++++--------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/packages/block-editor/src/components/iframe/content.scss b/packages/block-editor/src/components/iframe/content.scss index ff5bd3a4052f45..5e390800719949 100644 --- a/packages/block-editor/src/components/iframe/content.scss +++ b/packages/block-editor/src/components/iframe/content.scss @@ -3,10 +3,6 @@ } .block-editor-iframe__html { - $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); - $scale: var(--wp-block-editor-iframe-zoom-out-scale, 1); - scale: $scale; transform-origin: top center; // We don't want to animate the transform of the translateX because it is used // to "center" the canvas. Leaving it on causes the canvas to slide around in @@ -14,6 +10,9 @@ @include editor-canvas-resize-animation( transform 0s, scale 0s, padding 0s, translate 0s); &.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); + position: fixed; left: 0; right: 0; @@ -23,45 +22,49 @@ // 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; - // we only want to animate the scaling when entering zoom out. When sidebars + + // We only want to animate the scaling when entering zoom out. When sidebars // are toggled, the resizing of the iframe handles scaling the canvas as well, // and the doubled animations cause very odd animations. @include editor-canvas-resize-animation( transform 0s, top 0s, bottom 0s, right 0s, left 0s ); } +} - &.is-zoomed-out { - $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); - // Apply an X translation to center the scaled content within the available space. - transform: translateX(calc((#{$scale-container-width} - #{$container-width}) / 2 / #{$scale})); - background-color: $gray-300; - // Chrome seems to respect that transform scale shouldn't affect the layout size of the element, - // so we need to adjust the height of the content to match the scale by using negative margins. - $extra-content-height: calc(#{$content-height} * (1 - #{$scale})); - $total-frame-height: calc(2 * #{$frame-size} / #{$scale}); - $total-height: calc(#{$extra-content-height} + #{$total-frame-height} + 2px); - margin-bottom: calc(-1 * #{$total-height}); - // Add the top/bottom frame size. We use scaling to account for the left/right, as - // the padding left/right causes the contents to reflow, which breaks the 1:1 scaling - // of the content. - padding-top: calc(#{$frame-size} / #{$scale}); - padding-bottom: calc(#{$frame-size} / #{$scale}); +.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); + // Apply an X translation to center the scaled content within the available space. + transform: translateX(calc((#{$scale-container-width} - #{$container-width}) / 2 / #{$scale})); + scale: #{$scale}; + background-color: $gray-300; - body { - min-height: calc((#{$inner-height} - #{$total-frame-height}) / #{$scale}); + // Chrome seems to respect that transform scale shouldn't affect the layout size of the element, + // so we need to adjust the height of the content to match the scale by using negative margins. + $extra-content-height: calc(#{$content-height} * (1 - #{$scale})); + $total-frame-height: calc(2 * #{$frame-size} / #{$scale}); + $total-height: calc(#{$extra-content-height} + #{$total-frame-height} + 2px); + margin-bottom: calc(-1 * #{$total-height}); + // Add the top/bottom frame size. We use scaling to account for the left/right, as + // the padding left/right causes the contents to reflow, which breaks the 1:1 scaling + // of the content. + padding-top: calc(#{$frame-size} / #{$scale}); + padding-bottom: calc(#{$frame-size} / #{$scale}); - > .is-root-container:not(.wp-block-post-content) { - flex: 1; - display: flex; - flex-direction: column; - height: 100%; + 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; - } + > main { + flex: 1; } } } From ab31e4472720baa1ed8943cb6d54c26ae61cdf72 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:49:43 -0600 Subject: [PATCH 29/30] Add one more HACK comment --- packages/block-editor/src/components/iframe/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 054f27418eed61..536fc6f254b563 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -466,6 +466,7 @@ function Iframe( { if ( isZoomedOut ) { iframeDocument.documentElement.classList.add( 'is-zoomed-out' ); } else { + // HACK: Since we can't remove this in the cleanup, we need to do it here. iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); } From 0b4d86d6805f6c318539f222907d7bbe595af905 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Mon, 11 Nov 2024 12:25:34 -0600 Subject: [PATCH 30/30] Do not allow scrollTopNext to be smaller than 0 --- packages/block-editor/src/components/iframe/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 536fc6f254b563..0bb7e30a942046 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -389,7 +389,10 @@ function Iframe( { // possible scrollTop positions. Round the value to avoid subpixel // truncation by the browser which sometimes causes a 1px error. scrollTopNext = Math.round( - Math.min( Math.max( 0, scrollTopNext ), maxScrollTop ) + Math.min( + Math.max( 0, scrollTopNext ), + Math.max( 0, maxScrollTop ) + ) ); iframeDocument.documentElement.style.setProperty(