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

Iframe: skip scoping styles #46752

Merged
merged 1 commit into from
Sep 11, 2023
Merged

Iframe: skip scoping styles #46752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion packages/block-editor/src/components/block-canvas/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ export function ExperimentalBlockCanvas( {
if ( ! shouldIframe ) {
return (
<>
<EditorStyles styles={ styles } />
<EditorStyles
styles={ styles }
scope=".editor-styles-wrapper"
/>
<WritingFlow
ref={ contentRef }
className="editor-styles-wrapper"
Expand Down
1 change: 0 additions & 1 deletion 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 @@ -426,7 +426,6 @@
}

.block-editor-iframe__body {
background-color: $white;
Copy link
Member Author

Choose a reason for hiding this comment

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

@ntsekouras This isn't a good solution because now this rule has a higher specificity than a theme just using the body selector. Also, if a theme sets it on html, it won't be visible. We need to set the white background on the iframe element so that it's not transparent. I fixed that below.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this brunch this issue #46291 doesn't work though since we're scaling the body of the iframe and this is where the background is applied from theme.json. Any ideas to work around that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah the scaling... I just talked to Riad about that the other day. I think we should moved the scaling outside the iframe. I'll make a separate PR for that then first.

Copy link
Member

Choose a reason for hiding this comment

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

Would love to see this fix merged! It's breaking a lot of themes in our deployment of Gutenberg, so we've had to remove this line of CSS from our deployment. It seems like load order of CSS (for example, if CSS is concatenated) can break this.

Copy link
Member

Choose a reason for hiding this comment

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

Ah nice, @fullofcaffeine found that this PR fixes the issue: #46732

transition: all 0.3s;
transform-origin: top center;
}
17 changes: 7 additions & 10 deletions packages/block-editor/src/components/editor-styles/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ import { useCallback, useMemo } from '@wordpress/element';
*/
import transformStyles from '../../utils/transform-styles';

const EDITOR_STYLES_SELECTOR = '.editor-styles-wrapper';
extend( [ namesPlugin, a11yPlugin ] );

function useDarkThemeBodyClassName( styles ) {
function useDarkThemeBodyClassName( styles, scope ) {
return useCallback(
( node ) => {
if ( ! node ) {
Expand All @@ -28,9 +27,7 @@ function useDarkThemeBodyClassName( styles ) {

const { ownerDocument } = node;
const { defaultView, body } = ownerDocument;
const canvas = ownerDocument.querySelector(
EDITOR_STYLES_SELECTOR
);
const canvas = scope ? ownerDocument.querySelector( scope ) : body;

let backgroundColor;

Expand Down Expand Up @@ -63,11 +60,11 @@ function useDarkThemeBodyClassName( styles ) {
body.classList.add( 'is-dark-theme' );
}
},
[ styles ]
[ styles, scope ]
);
}

export default function EditorStyles( { styles } ) {
export default function EditorStyles( { styles, scope } ) {
const stylesArray = useMemo(
() => Object.values( styles ?? [] ),
[ styles ]
Expand All @@ -76,9 +73,9 @@ export default function EditorStyles( { styles } ) {
() =>
transformStyles(
stylesArray.filter( ( style ) => style?.css ),
EDITOR_STYLES_SELECTOR
scope
),
[ stylesArray ]
[ stylesArray, scope ]
);

const transformedSvgs = useMemo(
Expand All @@ -94,7 +91,7 @@ export default function EditorStyles( { styles } ) {
<>
{ /* Use an empty style element to have a document reference,
but this could be any element. */ }
<style ref={ useDarkThemeBodyClassName( stylesArray ) } />
<style ref={ useDarkThemeBodyClassName( stylesArray, scope ) } />
{ transformedStyles.map( ( css, index ) => (
<style key={ index }>{ css }</style>
) ) }
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/reset.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// We use :where to keep specificity minimal.
// https://css-tricks.com/almanac/selectors/w/where/
html :where(.editor-styles-wrapper) {
:where(.editor-styles-wrapper) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you think the changes in this "reset" file might impact non-framed editors?

Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this line has created a regression within the iframe when using some classic themes.
#57176

/**
* The following styles revert to the browser defaults overriding the WPAdmin styles.
* This is only needed while the block editor is not being loaded in an iframe.
Expand Down
1 change: 1 addition & 0 deletions packages/edit-site/src/components/block-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
display: block;
width: 100%;
height: 100%;
background: $white;
}

.edit-site-visual-editor__editor-canvas {
Expand Down