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

The CSS class is-dark-theme is not set in the post editor #60299

Open
afercia opened this issue Mar 29, 2024 · 11 comments · Fixed by #60300 · May be fixed by #68481
Open

The CSS class is-dark-theme is not set in the post editor #60299

afercia opened this issue Mar 29, 2024 · 11 comments · Fixed by #60300 · May be fixed by #68481
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Mar 29, 2024

Description

Splitting this out from #60275

To be more accurate, in the Post editor the CSS class is-dark-theme is initially applied but than removed.
In the Site editor, it works as expected.

It seems to be something missed with the refactorings of the editor irame, editor styles scoping and such.

When a theme has a dark background color, useDarkThemeBodyClassName is responsible for adding the is-dark-theme CSS class to the editor iframe body element.

It does that, but then the body gets updated and gets new CSS classes when the iframe is fully loaded, see the Iframe component internal handling of the classes in the onload callback and then the classes are set to the body.

I'm not sure whether that means the body gets entirely replaced. Regardless the new CSS classes replace the initial ones thus removing is-dark-theme it it was set.

The is-dark-theme CSS class is necessaty to make some editor features and styles work correctly.

Step-by-step reproduction instructions

  • Set Twenty Twenty-Four as active theme.
  • Go to the Site editor > Styles, set a theme style variation that has a dark background e.g. 'Onyx' and save. Update: the Site editor UI has changed and now 'Onyx' is shown as a palette variation in the Styles panel > Browse styles
  • In your browser dev tools, inspect the editor iframe body ie. the body with CSS classes block-editor-iframe__body editor-styles-wrapper.
  • Observe the body does have a CSS class is-dark-theme.
  • Go to the Post editor.
  • Observe the editor shows the theme variant with dark background.
  • In your browser dev tools, inspect the editor iframe body ie. the body with CSS classes block-editor-iframe__body editor-styles-wrapper.
  • If you inspect it very quickly, you will notice that initially the body element has these CSS classes: block-editor-iframe__body editor-styles-wrapper is-dark-theme.
  • When the iframe is fully loaded, observe the element body element does not have the is-dark-theme class.
  • Observe the body element CSS classes have been replaced with block-editor-iframe__body editor-styles-wrapper post-type-post admin-color-fresh wp-embed-responsive.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia
Copy link
Contributor Author

afercia commented Apr 10, 2024

The commit to fix this issue was reverted to fix a problem with Safari, see #60616.

I'm reopening this issue as it needs to be solved, but with a different approach.

@afercia afercia reopened this Apr 10, 2024
@ellatrix
Copy link
Member

Not sure if that would break anything, but maybe the best wait to fix the issue is to move is-dark-theme to the html element, which is not managed in React?

@afercia
Copy link
Contributor Author

afercia commented Apr 11, 2024

Based on this comment, looks like the problem with Safari is broader because also the CSS classes admin-color-{$color_scheme} and wp-embed-responsive seems to not be set correctly in Safari. However, I can't reproduce that issue on trunk.

Regarding the bug reported at #60550 I'm not sure the root cause was the change in #60300 as that change followed an already existing pattern used for the admin-color-{$color_scheme} and wp-embed-responsive classes.

Anyways, to illustrate a couple of the issues triggered by the missing is-dark-theme CSS classe, please see the screenshot below. It's just two examples. there are probably more issues.

A block in 'select mode' now shows a blue outline (left) but it is supposed to show a white outline (dark):

Screenshot 2024-04-11 at 15 57 25

In addition to the outline, the Spacer block, when selected, shows a full dark background (left) but it is supposed to show a lighter background (right):

Screenshot 2024-04-11 at 16 58 41

@ellatrix
Copy link
Member

My suggestion would be to move the class to the html element so that it's not overridden later on

@afercia
Copy link
Contributor Author

afercia commented Apr 12, 2024

My suggestion would be to move the class to the html element so that it's not overridden later on

I've been testing pretty exensively. While moving the class to the thml element seems trivial, it doesn't work well anyways with themes that may handle the body background dynamically. For example, Twenty Twenty-One does have its won 'dark mode toggle' feature that changes the body background after the editor useDarkThemeBodyClassName checks the background color luminance.

That's a pre-existing issue and I suspect it never worked well. However, it seems there's a need to a better mechanism to get the actual body background color even when it gets changed by themes or plugins features.

@afercia
Copy link
Contributor Author

afercia commented Sep 17, 2024

A first attempt to fix this issue was reverted in #60616. I can still reproduce the incorrect behavior in the Post editor. @glendaviesnz @ellatrix any change to explore an alternative approach to fix this issue>
RIght now, the implementation of the is-dark-theme class in the Post editor is just broken.

@afercia
Copy link
Contributor Author

afercia commented Dec 17, 2024

This issue is still a blocker for #60275
I would appreciate some focus on it to find a better, more solid, solution than the one tried in the first attempt.
Cc @WordPress/gutenberg-core

To recap: In the post editor the CSS classes of the iframe body are initially:

block-editor-iframe__body editor-styles-wrapper is-dark-theme

and after the page is fully loaded, a re-render replaces the classes with:

block-editor-iframe__body editor-styles-wrapper post-type-post admin-color-fresh wp-embed-responsive

As such, is-dark-theme is lost and there's no way to distinguish a light theme from a dark theme. This doesn't allow to address the accessibility issue in #60275

@afercia afercia removed their assignment Dec 18, 2024
@afercia afercia self-assigned this Jan 3, 2025
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jan 3, 2025
@afercia afercia linked a pull request Jan 3, 2025 that will close this issue
@afercia
Copy link
Contributor Author

afercia commented Jan 3, 2025

Previously related: #64548 / #64549

@richtabor
Copy link
Member

I'm against changing the admin selected UI color as a result of the theme's background color (the block outlines).

It doesn't scale as-is, and actually makes the experience more confusing as what was once familiar is no longer.

If we were to work on this, the resulting solution needs to be complete, accounting for:

  • theme background contrast comparable to the admin selected color.
  • a parent block's background color comparable to the selected color of blocks nested within.

Otherwise, you can have a black site with white sections and you no longer see any selected blocks.

It cannot be a site-wide condition.

@afercia
Copy link
Contributor Author

afercia commented Jan 13, 2025

Thanks for your feedback. I'm not sure it's directly related to this issue. It may be a follow-up consideration to address separately.
The fact the is-dark-theme CSS class works in the Site editor and doesn't in the Post editor is a bug that must be fixed.
This bug is a blocker for #60275 which is an accessibility issue that needs to be fixed.

@colinduwe
Copy link
Contributor

I propose adding a bodyClasses property to settings in the 'core/block-editor' store and refactoring the iframe component to use that state to maintain its body classes. useDarkThemeBodyClassName could update the store and not be overwritten by the iframe component's onLoad behavior.
Here's a rough proposal related to some other open issues caused by the current way bodyClasses are applied to the iframe component: #68538

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
4 participants