Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zoom out UI scale #61265

Merged
merged 17 commits into from
May 9, 2024
+102 −79
Merged

Fix zoom out UI scale #61265

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9d29a2e
Fix crash when switching to code editor
ajlende Apr 11, 2024
f029d2d
Fix typo
ajlende Apr 11, 2024
a8c290a
Remove unused eslint ignore
ajlende Apr 11, 2024
d2b797f
Add isZoomedOut variable
ajlende Apr 11, 2024
3c3f0db
Remove load callback that isn't needed
ajlende Apr 11, 2024
833f128
Fix zoomed out code editor background
ajlende Apr 11, 2024
c30ca58
Fix UI elements in zoom-out mode
ajlende Apr 26, 2024
add28e3
Move all calculations to CSS
ajlende Apr 26, 2024
8b93816
Move iframe styles into own file
ajlende Apr 26, 2024
ffc1265
Merge branch 'trunk' into fix/iframe-zoom-out-ui-scale
ajlende May 8, 2024
c148197
Revert unrelated changes
ajlende May 8, 2024
3ae1db9
Fix CSS lint
ajlende May 8, 2024
2600547
Remove comment
ajlende May 8, 2024
db88df3
Revert to 100%
ajlende May 8, 2024
04b181a
Recalculate height on rerender
ajlende May 8, 2024
aaea97f
Update variable prefix for package and block
ajlende May 9, 2024
20d21ae
Merge branch 'trunk' into fix/iframe-zoom-out-ui-scale
ajlende May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions packages/block-editor/src/components/block-content-overlay/content.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
pointer-events: none;
}

.block-editor-iframe__body.is-zoomed-out &::before {
// Unfortunately because of the vw unit, this is not always going to be exact
// When the scrollbar is visible, the frame exceeds the canvas by a few pixels.
width: calc(100vw);
left: calc(( 100% - 100vw ) / 2);
.block-editor-block-list__block.has-block-overlay::before {
left: 0;
right: 0;
width: auto;
Copy link
Member

Choose a reason for hiding this comment

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

Why are we changing these rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It provides exact sizing instead of using the hack that is sometimes wrong.

}
}
37 changes: 2 additions & 35 deletions packages/block-editor/src/components/block-list/content.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
left: 0;
outline-color: var(--wp-admin-theme-color);
outline-style: solid;
outline-width: var(--wp-admin-border-width-focus);
outline-offset: calc(-1 * var(--wp-admin-border-width-focus));
outline-width: calc(var(--wp-admin-border-width-focus) / var(--wp-zoom-out-scale, 1));
outline-offset: calc((-1 * var(--wp-admin-border-width-focus)) / var(--wp-zoom-out-scale, 1));
}

// Hide selections on this element, otherwise Safari will include it stacked
Expand Down Expand Up @@ -384,29 +384,6 @@ _::-webkit-full-page-media, _:future, :root .has-multi-selection .block-editor-b
z-index: z-index("{core/image aligned left or right} .wp-block");
}

// Increase width when zoomed out to match visually.
body.is-zoomed-out {
display: flex;
flex-direction: column;

.is-outline-mode .block-editor-block-list__block:not(.remove-outline):not(.rich-text).is-selected::after,
.is-outline-mode .block-editor-block-list__block:not(.remove-outline).is-hovered:not(.is-selected)::after {
outline-width: calc(2 * var(--wp-admin-border-width-focus)); // Adjusted for the zoom scale.
outline-offset: calc(-2 * var(--wp-admin-border-width-focus));
}

> .is-root-container {
flex: 1;
display: flex;
flex-direction: column;
height: 100%;

> main {
flex: 1;
}
}
}

.wp-site-blocks > [data-align="left"] {
float: left;
margin-right: 2em;
Expand Down Expand Up @@ -499,13 +476,3 @@ body.is-zoomed-out {
margin-bottom: auto;
}
}

.block-editor-iframe__html {
transform-origin: top center;
transition: all 0.3s;
@include reduce-motion("transition");
}

.block-editor-iframe__html[style*="scale"] {
background-color: $gray-300;
}
44 changes: 44 additions & 0 deletions packages/block-editor/src/components/iframe/content.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
.block-editor-iframe__html {
transform-origin: top center;
transition: transform 0.3s;
@include reduce-motion("transition");
}

.block-editor-iframe__html.is-zoomed-out {
$scale: var(--wp-zoom-out-scale);
$frame-size: var(--wp-zoom-out-frame-size);
Copy link
Member

Choose a reason for hiding this comment

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

Why did you move everything to CSS? We only need --wp-zoom-out-scale?
This is going to be problematic when I try to animate both scroll and scale in JS at the same time with the same function.

Copy link
Member

Choose a reason for hiding this comment

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

Or I wonder if animating these variables will be the same in terms of performance. But still, I don't get why we make all of these variables if it's not needed.

Copy link
Member

Choose a reason for hiding this comment

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

We might also want to prefix it with wp-block-editor, not sure

Copy link
Contributor Author

@ajlende ajlende May 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Or I wonder if animating these variables will be the same in terms of performance. But still, I don't get why we make all of these variables if it's not needed.

It was easier to read. I would expect the styling to be done in CSS. And it seemed like performance wouldn't be worse.

This is going to be problematic when I try to animate both scroll and scale in JS at the same time with the same function.

Why do you need to animate them in JS? Can't the animations be done with CSS?

$inner-height: var(--wp-zoom-out-inner-height);
$content-height: var(--wp-zoom-out-content-height);

transform: scale(#{$scale});

background-color: $gray-300;

// Firefox and Safari don't render margin-bottom here and margin-bottom is needed for Chrome
// layout, so we use border matching the background instead of margins.
border: calc(#{$frame-size} / #{$scale}) solid $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});
$total-height: calc(#{$extra-content-height} + #{$total-frame-height});
margin-bottom: calc(-1 * #{$total-height});

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

> .is-root-container {
flex: 1;
display: flex;
flex-direction: column;
height: 100%;

> main {
flex: 1;
}
}
}
}
102 changes: 45 additions & 57 deletions packages/block-editor/src/components/iframe/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,70 +207,13 @@ function Iframe( {
};
}, [] );

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

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

const [ iframeWindowInnerHeight, setIframeWindowInnerHeight ] = useState();

const scaleRef = useRefEffect(
( body ) => {
// Hack to get proper margins when scaling the iframe document.
const bottomFrameSize = frameSize - contentHeight * ( 1 - scale );

const { documentElement } = body.ownerDocument;

body.classList.add( 'is-zoomed-out' );

documentElement.style.transform = `scale( ${ scale } )`;
documentElement.style.marginTop = `${ frameSize }px`;
// TODO: `marginBottom` doesn't work in Firefox. We need another way
// to do this.
documentElement.style.marginBottom = `${ bottomFrameSize }px`;
if ( iframeWindowInnerHeight > contentHeight * scale ) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's see in a follow-up if this is still needed

iframeDocument.body.style.minHeight = `${ Math.floor(
( iframeWindowInnerHeight - 2 * frameSize ) / scale
) }px`;
}

return () => {
body.classList.remove( 'is-zoomed-out' );
documentElement.style.transform = '';
documentElement.style.marginTop = '';
documentElement.style.marginBottom = '';
body.style.minHeight = '';
};
},
[
scale,
frameSize,
iframeDocument,
contentHeight,
iframeWindowInnerHeight,
contentWidth,
]
);

const disabledRef = useDisabled( { isDisabled: ! readonly } );
const bodyRef = useMergeRefs( [
useBubbleEvents( iframeDocument ),
contentRef,
clearerRef,
writingFlowRef,
disabledRef,
// Avoid resize listeners when not needed, these will trigger
// unnecessary re-renders when animating the iframe width, or when
// expanding preview iframes.
scale === 1 ? null : windowResizeRef,
scale === 1 ? null : scaleRef,
] );

// Correct doctype is required to enable rendering in standards
Expand Down Expand Up @@ -316,6 +259,51 @@ function Iframe( {
? scale( contentWidth, contentHeight )
: scale;

const isZoomedOut = scale !== 1;

useEffect( () => {
if ( ! iframeDocument || ! isZoomedOut ) {
return;
}

iframeDocument.documentElement.classList.add( 'is-zoomed-out' );

// Needed for calculations in block-list content.scss.
iframeDocument.documentElement.style.setProperty(
'--wp-zoom-out-scale',
`${ scale }`
);
iframeDocument.documentElement.style.setProperty(
'--wp-zoom-out-frame-size',
`${ frameSize }px`
);
iframeDocument.documentElement.style.setProperty(
'--wp-zoom-out-content-height',
`${ contentHeight }px`
);
iframeDocument.documentElement.style.setProperty(
'--wp-zoom-out-inner-height',
`${ iframeDocument.defaultView.innerHeight }px`
);

return () => {
iframeDocument.documentElement.classList.remove( 'is-zoomed-out' );

iframeDocument.documentElement.style.removeProperty(
'--wp-zoom-out-scale'
);
iframeDocument.documentElement.style.removeProperty(
'--wp-zoom-out-frame-size'
);
iframeDocument.documentElement.style.removeProperty(
'--wp-zoom-out-content-height'
);
iframeDocument.documentElement.style.removeProperty(
'--wp-zoom-out-inner-height'
);
};
}, [ scale, frameSize, iframeDocument, contentHeight, isZoomedOut ] );

// Make sure to not render the before and after focusable div elements in view
// mode. They're only needed to capture focus in edit mode.
const shouldRenderFocusCaptureElements = tabIndex >= 0 && ! isPreviewMode;
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/content.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
@import "./components/block-variation-picker/content.scss";
@import "./components/button-block-appender/content.scss";
@import "./components/default-block-appender/content.scss";
@import "./components/iframe/content.scss";
@import "./components/inner-blocks/content.scss";
@import "./components/media-placeholder/content.scss";
@import "./components/plain-text/content.scss";
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/code-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.edit-site-code-editor {
position: relative;
width: 100%;
min-height: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

This keeps the background filllng the screen.
Screenshot 2024-05-08 at 14 15 50

min-height: min-content;
background-color: $white;

&__body {
Expand Down
Loading