-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Position scaled html within available container space #66034
Changes from 10 commits
7f4b389
d091621
374477b
3d252da
9f58573
dc49dfa
c5fc1e1
3cea814
ec7a584
581d404
caba47c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,7 +122,7 @@ function Iframe( { | |
}, [] ); | ||
const { styles = '', scripts = '' } = resolvedAssets; | ||
const [ iframeDocument, setIframeDocument ] = useState(); | ||
const prevContainerWidthRef = useRef(); | ||
const initialContainerWidth = useRef(); | ||
const [ bodyClasses, setBodyClasses ] = useState( [] ); | ||
const clearerRef = useBlockSelectionClearer(); | ||
const [ before, writingFlowRef, after ] = useWritingFlow(); | ||
|
@@ -243,7 +243,7 @@ function Iframe( { | |
|
||
useEffect( () => { | ||
if ( ! isZoomedOut ) { | ||
prevContainerWidthRef.current = containerWidth; | ||
initialContainerWidth.current = containerWidth; | ||
} | ||
}, [ containerWidth, isZoomedOut ] ); | ||
|
||
|
@@ -298,14 +298,49 @@ function Iframe( { | |
|
||
useEffect( () => cleanup, [ cleanup ] ); | ||
|
||
const zoomOutAnimationClassnameRef = useRef( null ); | ||
const handleZoomOutAnimationClassname = () => { | ||
clearTimeout( zoomOutAnimationClassnameRef.current ); | ||
|
||
iframeDocument.documentElement.classList.add( 'zoom-out-animation' ); | ||
|
||
zoomOutAnimationClassnameRef.current = setTimeout( () => { | ||
iframeDocument.documentElement.classList.remove( | ||
'zoom-out-animation' | ||
); | ||
}, 400 ); // 400ms should match the animation speed used in components/iframe/content.scss | ||
}; | ||
Comment on lines
+302
to
+312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function should be defined inside the |
||
|
||
// 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 ) { | ||
return; | ||
} | ||
|
||
handleZoomOutAnimationClassname(); | ||
iframeDocument.documentElement.classList.add( 'is-zoomed-out' ); | ||
|
||
return () => { | ||
handleZoomOutAnimationClassname(); | ||
iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); | ||
}; | ||
}, [ iframeDocument, isZoomedOut ] ); | ||
|
||
// Calculate the scaling and CSS variables for the zoom out canvas | ||
useEffect( () => { | ||
if ( ! iframeDocument || ! isZoomedOut ) { | ||
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 | ||
// minor divergence for now. | ||
|
||
// This scaling calculation has to happen within the JS because CSS calc() can | ||
// only divide and multiply by a unitless value. I.e. calc( 100px / 2 ) is valid | ||
// but calc( 100px / 2px ) is not. | ||
|
@@ -314,7 +349,10 @@ function Iframe( { | |
scale === 'default' | ||
? ( Math.min( containerWidth, maxWidth ) - | ||
parseInt( frameSize ) * 2 ) / | ||
prevContainerWidthRef.current | ||
Math.max( | ||
initialContainerWidth.current, | ||
containerWidth | ||
) | ||
: scale | ||
); | ||
|
||
|
@@ -336,13 +374,16 @@ function Iframe( { | |
`${ containerWidth }px` | ||
); | ||
iframeDocument.documentElement.style.setProperty( | ||
'--wp-block-editor-iframe-zoom-out-prev-container-width', | ||
`${ prevContainerWidthRef.current }px` | ||
'--wp-block-editor-iframe-zoom-out-outer-container-width', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would maybe rename this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may not always call it scale container, so I'd be fine leaving this one named as is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, it may be changed, but for the current reader, the connection between the two would be more clear if the variable was named the same as the container that it's setting the width for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
`${ Math.max( initialContainerWidth.current, containerWidth ) }px` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would make this a variable since it's used in three places where they are expected to always be the same. |
||
); | ||
|
||
return () => { | ||
iframeDocument.documentElement.classList.remove( 'is-zoomed-out' ); | ||
// iframeDocument.documentElement.style.setProperty( | ||
// '--wp-block-editor-iframe-zoom-out-outer-container-width', | ||
// `${ Math.max( initialContainerWidth.current, containerWidth ) }px` | ||
// ); | ||
Comment on lines
+381
to
+384
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think these comments were meant to be committed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope! |
||
|
||
return () => { | ||
iframeDocument.documentElement.style.removeProperty( | ||
'--wp-block-editor-iframe-zoom-out-scale' | ||
); | ||
|
@@ -359,7 +400,7 @@ function Iframe( { | |
'--wp-block-editor-iframe-zoom-out-container-width' | ||
); | ||
iframeDocument.documentElement.style.removeProperty( | ||
'--wp-block-editor-iframe-zoom-out-prev-container-width' | ||
'--wp-block-editor-iframe-zoom-out-outer-container-width' | ||
); | ||
}; | ||
}, [ | ||
|
@@ -460,10 +501,12 @@ function Iframe( { | |
isZoomedOut && 'is-zoomed-out' | ||
) } | ||
style={ { | ||
'--wp-block-editor-iframe-zoom-out-container-width': | ||
isZoomedOut && `${ containerWidth }px`, | ||
'--wp-block-editor-iframe-zoom-out-prev-container-width': | ||
isZoomedOut && `${ prevContainerWidthRef.current }px`, | ||
'--wp-block-editor-iframe-zoom-out-outer-container-width': | ||
isZoomedOut && | ||
`${ Math.max( | ||
initialContainerWidth.current, | ||
containerWidth | ||
) }px`, | ||
} } | ||
> | ||
{ iframe } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,10 @@ | |
} | ||
|
||
.block-editor-iframe__scale-container.is-zoomed-out { | ||
$container-width: var(--wp-block-editor-iframe-zoom-out-container-width, 100vw); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be able to remove this from the JS too since it's no longer used outside the iframe. |
||
$prev-container-width: var(--wp-block-editor-iframe-zoom-out-prev-container-width, 100vw); | ||
width: $prev-container-width; | ||
// This is to offset the movement of the iframe when we open sidebars | ||
margin-left: calc(-1 * (#{$prev-container-width} - #{$container-width}) / 2); | ||
Comment on lines
-15
to
-16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was causing the scrollbar to be hidden off canvas. |
||
$outer-container-width: var(--wp-block-editor-iframe-zoom-out-outer-container-width, 100vw); | ||
width: $outer-container-width; | ||
// Position the iframe so that it is always aligned with the right side so that | ||
// the scrollbar is always visible on the right side | ||
position: absolute; | ||
right: 0; | ||
draganescu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't appear to have any effect.