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

Revert "Make sure the CSS class id-dark-theme is added to the editor iframe body. (#60300) #60616

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Revert "Make sure the CSS class id-dark-theme is added to the editor iframe body. (#60300) #60616

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
22 changes: 6 additions & 16 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 @@ -141,13 +141,6 @@ function Iframe( {
function onLoad() {
const { contentDocument, ownerDocument } = node;
const { documentElement } = contentDocument;
// Get any CSS classes the iframe document body may initially have
// to re-apply them later together with the ones of the main document
// body. This is necessary for some CSS classes for example the
// `is-dark-theme` class added by useDarkThemeBodyClassName.
const initialIframeBodyClasses = Array.from(
contentDocument.body.classList
);
Copy link
Member

Choose a reason for hiding this comment

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

Why not just revert this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because .concat( initialIframeBodyClasses ) is not relevant without the above, and is-dark-theme check at this point may also not be relevant, but it can be added back if needed in a follow up PR that gets is-dark-theme working in the post editor, otherwise it seemed like there was potential for it to be left there as dead code.

Copy link
Contributor Author

@glendaviesnz glendaviesnz Apr 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If I took your suggestion to "remove all this logic introduced last week" too literally 😄 I can put up a PR to add back in the `|| name === 'is-dark-theme' tomorrow if you think it is worth it.

Copy link
Member

Choose a reason for hiding this comment

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

18.1 has already been released with the revert. 18.2 will be cut in 2 weeks if it can wait. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, I don't believe it will be an urgent change if we do add it back, 18.2 will be fine I think.

iFrameDocument = contentDocument;

documentElement.classList.add( 'block-editor-iframe__html' );
Expand All @@ -158,15 +151,12 @@ function Iframe( {
// be added in the editor too, which we'll somehow have to get from
// the server in the future (which will run the PHP filters).
setBodyClasses(
Array.from( ownerDocument.body.classList )
.concat( initialIframeBodyClasses )
.filter(
( name ) =>
name.startsWith( 'admin-color-' ) ||
name.startsWith( 'post-type-' ) ||
name === 'wp-embed-responsive' ||
name === 'is-dark-theme'
)
Array.from( ownerDocument.body.classList ).filter(
( name ) =>
name.startsWith( 'admin-color-' ) ||
name.startsWith( 'post-type-' ) ||
name === 'wp-embed-responsive'
)
);

contentDocument.dir = ownerDocument.dir;
Expand Down
Loading