From 4192156ca474558c908d92eb60dbeccc464d73b6 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 13:51:20 -0500 Subject: [PATCH 01/32] 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 76d2e09dfb7a30..bbca6372a51c43 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 = () => { @@ -268,6 +270,20 @@ function Iframe( { containerWidth ); + const maxWidth = 750; + const scaleValue = + scale === 'auto-scaled' + ? ( 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 ), @@ -320,39 +336,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( () => { @@ -360,7 +423,6 @@ function Iframe( { return; } - const maxWidth = 750; // Note: When we initialize the zoom out when the canvas is smaller (sidebars open), // initialContainerWidthRef 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 @@ -371,11 +433,7 @@ function Iframe( { // but calc( 100px / 2px ) is not. iframeDocument.documentElement.style.setProperty( '--wp-block-editor-iframe-zoom-out-scale', - scale === 'auto-scaled' - ? ( 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. @@ -421,7 +479,7 @@ function Iframe( { ); }; }, [ - scale, + scaleValue, frameSize, iframeDocument, iframeWindowInnerHeight, From 5bb30a1af1912d146b22e8766209c5832a7d9129 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 13:57:18 -0500 Subject: [PATCH 02/32] 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 22186f27038bb6517d11d518ffb802ed9dda5520 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 14:37:46 -0500 Subject: [PATCH 03/32] 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 bbca6372a51c43..cc2ed079133f65 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -419,7 +419,7 @@ function Iframe( { // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { - if ( ! iframeDocument || ! isZoomedOut ) { + if ( ! iframeDocument ) { return; } @@ -457,27 +457,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 afd8ca37704fce18d9ff9e55223547122696cb32 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 30 Oct 2024 14:45:47 -0500 Subject: [PATCH 04/32] 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 123d6479e0d9383ed4d468a92ebaf7ada1adb92d Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 31 Oct 2024 09:27:50 -0500 Subject: [PATCH 05/32] 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 cc2ed079133f65..b810bf0ba54e40 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'; @@ -258,6 +260,7 @@ function Iframe( { }, [] ); const isZoomedOut = scale !== 1; + const prevIsZoomedOut = usePrevious( isZoomedOut ); useEffect( () => { if ( ! isZoomedOut ) { @@ -337,69 +340,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 ) { @@ -409,17 +409,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 fd159129a723884d423699a6dbdb9fd2410df058 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 31 Oct 2024 09:35:18 -0500 Subject: [PATCH 06/32] 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 b810bf0ba54e40..59bf492b15be13 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -413,7 +413,7 @@ function Iframe( { // Calculate the scaling and CSS variables for the zoom out canvas useEffect( () => { - if ( ! iframeDocument || prevIsZoomedOut === isZoomedOut ) { + if ( ! iframeDocument ) { return; } From 3a3e1ad7302e0e35e5a49ce99bc26426bf49e450 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 31 Oct 2024 11:58:41 -0500 Subject: [PATCH 07/32] 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 59bf492b15be13..7d2f2dc85f65b0 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -356,14 +356,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` @@ -387,7 +402,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 e16a2a8542bb3ff2ca15d2a51874002f5ffc77ab Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 31 Oct 2024 14:37:33 -0500 Subject: [PATCH 08/32] 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 7d2f2dc85f65b0..13df9df8f0e583 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'; @@ -260,7 +259,7 @@ function Iframe( { }, [] ); const isZoomedOut = scale !== 1; - const prevIsZoomedOut = usePrevious( isZoomedOut ); + const prevIsZoomedOutRef = useRef( isZoomedOut ); useEffect( () => { if ( ! isZoomedOut ) { @@ -415,6 +414,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 e4bf2dd9363a6b78ddd5aca66f825a5b6bccfccd Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Tue, 5 Nov 2024 15:14:07 -0600 Subject: [PATCH 09/32] 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 13df9df8f0e583..c1a87bbc3f6209 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -390,6 +390,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' @@ -400,7 +408,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 8d767b4e4b1955630d6c781cb08ca31e6a5e2054 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Tue, 5 Nov 2024 16:31:22 -0600 Subject: [PATCH 10/32] 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 c1a87bbc3f6209..f1f2dc3dd3ab6c 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 initialContainerWidthRef = useRef( 0 ); const [ bodyClasses, setBodyClasses ] = useState( [] ); @@ -284,7 +285,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( [ @@ -343,40 +345,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', @@ -403,19 +413,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 f37547d12aa59ad5c846775c686ad23f85c97b47 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 6 Nov 2024 09:13:01 -0600 Subject: [PATCH 11/32] 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 f1f2dc3dd3ab6c..879023f0e11054 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -359,7 +359,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; @@ -381,10 +381,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 5cce3b269a4008904210092fe21cdf0e13d26981 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 6 Nov 2024 11:19:37 -0600 Subject: [PATCH 12/32] 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 879023f0e11054..00feec845dc0ba 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -286,7 +286,6 @@ function Iframe( { const prevFrameSizeRef = useRef( frameSizeValue ); const prevClientHeightRef = useRef( containerHeight ); - const prevScrollHeightRef = useRef( contentHeight ); const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ @@ -359,7 +358,6 @@ function Iframe( { const scrollHeight = iframeDocument.documentElement.scrollHeight; const prevClientHeight = prevClientHeightRef.current; - // const prevScrollHeight = prevScrollHeightRef.current; const prevScale = prevScaleRef.current; const prevFrameSize = prevFrameSizeRef.current; @@ -379,18 +377,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', @@ -418,7 +412,6 @@ function Iframe( { ); prevClientHeightRef.current = clientHeight; - prevScrollHeightRef.current = scrollHeight; prevFrameSizeRef.current = frameSizeValue; prevScaleRef.current = scaleValue; iframeDocument.documentElement.scrollTop = scrollTopNext; From 8b9c69366cabf15ee9cc29ba107e5460703e1033 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 6 Nov 2024 11:28:37 -0600 Subject: [PATCH 13/32] 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 00feec845dc0ba..4035a4d98d5772 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -376,6 +376,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 adea71ca01f77f76f902e500cc2198f6bd42b49d Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 14:59:07 -0600 Subject: [PATCH 14/32] 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 4035a4d98d5772..fdec2d817d61c6 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -344,47 +344,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 fcc49f4df50480d1a8b98daaa276d4debdbfa4d5 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 15:32:11 -0600 Subject: [PATCH 15/32] 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 fdec2d817d61c6..a4f23b79e770a6 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -407,6 +407,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` @@ -419,24 +429,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 08247f0e69483e925b0d9f64be54d16ca1ed1812 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 15:50:11 -0600 Subject: [PATCH 16/32] 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 a4f23b79e770a6..cdf7eb32f8cce6 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 = {}; @@ -407,46 +408,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 ec73741a54d5999ef8c9f2ffaaca8a45fb010165 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 15:55:39 -0600 Subject: [PATCH 17/32] 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 cdf7eb32f8cce6..063d12a4540f3b 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -340,11 +340,7 @@ function Iframe( { useEffect( () => cleanup, [ cleanup ] ); - const zoomOutAnimationTimeoutRef = useRef( null ); - const handleZoomOutAnimation = useCallback( () => { - clearTimeout( zoomOutAnimationTimeoutRef.current ); - // Previous scale value. const prevScale = prevScaleRef.current; From 6996c1bea64a24ffb38f7c3cac21e33bca07f78e Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 16:16:43 -0600 Subject: [PATCH 18/32] 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 063d12a4540f3b..15191e1761053e 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 = {}; @@ -261,7 +259,6 @@ function Iframe( { }, [] ); const isZoomedOut = scale !== 1; - const prevIsZoomedOutRef = useRef( isZoomedOut ); useEffect( () => { if ( ! isZoomedOut ) { @@ -340,7 +337,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; @@ -404,7 +409,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; @@ -419,60 +440,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 c79718e8b7fc56350218d49e9c688b8c9ee69ff5 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 16:28:34 -0600 Subject: [PATCH 19/32] 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 15191e1761053e..8e8bb30809a6b1 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 initialContainerWidthRef = useRef( 0 ); const [ bodyClasses, setBodyClasses ] = useState( [] ); From 77559a8c29229ff7c7401a68ad2287462174da78 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 6 Nov 2024 16:28:50 -0600 Subject: [PATCH 20/32] 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 8e8bb30809a6b1..7ac7ea30b6e4ed 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -527,6 +527,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 9fa40e200ecd0572c5db3bfa886a8c54fa85d6b2 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 7 Nov 2024 10:22:03 -0600 Subject: [PATCH 21/32] 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 7ac7ea30b6e4ed..223adfda6e988f 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 = () => { @@ -283,7 +281,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( [ @@ -346,20 +345,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 f0fe6abd71f316792d8186ac94e40d3b4586cbd9 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 7 Nov 2024 10:52:03 -0600 Subject: [PATCH 22/32] 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 223adfda6e988f..175d2fa458c702 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 = () => { @@ -434,11 +436,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( @@ -463,7 +461,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 0ccd9e517cd2856618ef7cc5f0a15413cf35e32e Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 7 Nov 2024 15:32:43 -0600 Subject: [PATCH 23/32] Add test for zoom in/out location --- test/e2e/specs/site-editor/zoom-out.spec.js | 166 ++++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/test/e2e/specs/site-editor/zoom-out.spec.js b/test/e2e/specs/site-editor/zoom-out.spec.js index 464bd4a4a4efad..0a89ec28865036 100644 --- a/test/e2e/specs/site-editor/zoom-out.spec.js +++ b/test/e2e/specs/site-editor/zoom-out.spec.js @@ -3,6 +3,63 @@ */ 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

+
+`; + test.describe( 'Zoom Out', () => { test.beforeAll( async ( { requestUtils } ) => { await requestUtils.activateTheme( 'twentytwentyfour' ); @@ -47,4 +104,113 @@ test.describe( 'Zoom Out', () => { expect( htmlRect.y + paddingTop ).toBeGreaterThan( iframeRect.y ); expect( htmlRect.x ).toBeGreaterThan( iframeRect.x ); } ); + + 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 e88ee8314f06a9a5d118100ee13eb7ebce7ad86f Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 11:13:08 -0600 Subject: [PATCH 24/32] 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 175d2fa458c702..5523f3f5ffcf2e 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -437,7 +437,10 @@ function Iframe( { } if ( prefersReducedMotion ) { - onZoomOutTransitionEnd(); + // Hack: Wait for the window values to recalculate. + iframeDocument.defaultView.requestAnimationFrame( + onZoomOutTransitionEnd + ); } else { iframeDocument.documentElement.addEventListener( 'transitionend', From 14b9f63335f9aa587368cd6bbfcbfc29805aa2ca Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:24:22 -0600 Subject: [PATCH 25/32] 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 5523f3f5ffcf2e..ab42953b07662f 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -271,20 +271,18 @@ function Iframe( { containerWidth ); + const frameSizeValue = parseInt( frameSize ); + const maxWidth = 750; const scaleValue = scale === 'auto-scaled' - ? ( 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 f9ab5191a766eb812e77c40567f034d744887ab7 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:24:43 -0600 Subject: [PATCH 26/32] 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 ab42953b07662f..0a75c81822fd9c 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -339,7 +339,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; @@ -480,7 +481,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 ] ); @@ -527,7 +529,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 c5d344562b54bdc770545f06afd0e0609b37b073 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:32:32 -0600 Subject: [PATCH 27/32] 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 0a75c81822fd9c..f9a1c2350b2e8d 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -426,6 +426,7 @@ function Iframe( { iframeDocument.documentElement.classList.remove( 'zoom-out-animation' ); + // Update previous values. prevClientHeightRef.current = clientHeight; prevFrameSizeRef.current = frameSizeValue; @@ -435,9 +436,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 { @@ -458,10 +460,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 534485a78f2fc3d6a71ca6aa07ac067d6448e2c4 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:43:47 -0600 Subject: [PATCH 28/32] 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 8b912e49dc6f59ad0122e274a629a14e68db0fad Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Fri, 8 Nov 2024 15:49:43 -0600 Subject: [PATCH 29/32] 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 f9a1c2350b2e8d..a8425f4c435936 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -483,6 +483,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 32ae73611ab2f613af7ae1995163f4bec67cb325 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Mon, 11 Nov 2024 12:25:34 -0600 Subject: [PATCH 30/32] 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 a8425f4c435936..bf4fa55df8efc0 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -406,7 +406,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( From 73e032e76aca3269dd294c184bed11fe22360210 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Tue, 19 Nov 2024 12:23:08 -0600 Subject: [PATCH 31/32] Fix zoom-out.spec.js --- test/e2e/specs/site-editor/zoom-out.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/specs/site-editor/zoom-out.spec.js b/test/e2e/specs/site-editor/zoom-out.spec.js index 0a89ec28865036..e698a94b7cf0dc 100644 --- a/test/e2e/specs/site-editor/zoom-out.spec.js +++ b/test/e2e/specs/site-editor/zoom-out.spec.js @@ -109,6 +109,8 @@ test.describe( 'Zoom Out', () => { page, editor, } ) => { + // Add some patterns into the page. + await editor.setContent( EDITOR_ZOOM_OUT_CONTENT ); // Find the scroll container element await page.evaluate( () => { const { activeElement } = From 857ca64ae813ae75e5f3a3c5a7d4f930d7bd47d9 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Mon, 25 Nov 2024 10:44:10 -0600 Subject: [PATCH 32/32] Disable errant react-compiler eslint rule --- 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 bf4fa55df8efc0..f8b7c25084e38d 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -436,6 +436,9 @@ function Iframe( { prevScaleRef.current = scaleValue; // Set the final scroll position that was just animated to. + // Disable reason: Eslint isn't smart enough to know that this is a + // DOM element. https://github.com/facebook/react/issues/31483 + // eslint-disable-next-line react-compiler/react-compiler iframeDocument.documentElement.scrollTop = scrollTopNext; }