Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Zoom-out: Move default background to the iframe component #66284

Merged
merged 4 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ iframe[name="editor-canvas"] {
width: 100%;
height: 100%;
display: block;
background-color: transparent;
// Handles transitions between device previews
@include editor-canvas-resize-animation;
background-color: $gray-300;
Copy link
Contributor

@ntsekouras ntsekouras Oct 21, 2024

Choose a reason for hiding this comment

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

I don't remember exactly, but I think we wanted to have transparent background (now you have two background-color rules). Maybe something with the background in theme.json if defined? 🤔 Of course if that was the reason, block editor shouldn't have that dependency..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh it seems this introduced the issue in #62223 back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, the issue seems in trunk too, so there must be something else going on. The regression is not caused by this PR. cc @jasmussen

ellatrix marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
.edit-site-editor-canvas-container {
height: 100%;
background-color: $gray-300;

// Controls height of editor and editor canvas container (style book, global styles revisions previews etc.)
iframe {
Expand Down
11 changes: 11 additions & 0 deletions packages/editor/src/components/visual-editor/style.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
.editor-visual-editor {
position: relative;
display: flex;

// This duplicates the iframe background but it's necessary in some situations
// when the iframe doesn't cover the whole canvas
// like the "focused entities".
background-color: $gray-300;
Copy link
Member

Choose a reason for hiding this comment

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

If we have this rule, why do we still need the background in packages/block-editor/src/components/block-canvas/style.scss?

Copy link
Contributor Author

@youknowriad youknowriad Nov 5, 2024

Choose a reason for hiding this comment

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

For third-party block editor usage. (zoom-out in storybook for example)


// This overrides the iframe background since it's applied again here
// It also prevents some style glitches if `editor-visual-editor`
// like when hovering the preview in the site editor.
iframe[name="editor-canvas"] {
background-color: transparent;
}

// Centralize the editor horizontally (flex-direction is column).
align-items: center;

Expand Down
2 changes: 1 addition & 1 deletion storybook/stories/playground/zoom-out/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function EditorZoomOut( { zoomLevel } ) {
<div
className="editor-zoom-out"
onKeyDown={ ( event ) => event.stopPropagation() }
style={ { background: '#ddd', border: '1px solid gray' } }
style={ { border: '1px solid gray' } }
>
<BlockEditorProvider
value={ blocks }
Expand Down
Loading