Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#53429 closed defect (bug) (fixed)

Twenty Twenty-One: Dark mode in the new Widgets makes impossible to read

Reported by: oglekler's profile oglekler Owned by:
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.8
Component: Bundled Theme Keywords: has-screenshots
Focuses: Cc:

Description

If on the frontend Dark Mode is active (On), it's also active in the new Widgets (WordPress 5.8) area and makes text and element almost invisible.

Attachments (10)

2021-06-16_17-01-09.png (22.0 KB) - added by oglekler 3 years ago.
Dark Mode: On
2021-06-16_17-02-14.png (24.7 KB) - added by oglekler 3 years ago.
Dark Mode: Off
dark-mode-unreadable.png (120.7 KB) - added by desrosj 3 years ago.
dark-mode-fixed.png (610.9 KB) - added by desrosj 3 years ago.
2021-06-24_23-29-28.png (42.6 KB) - added by oglekler 3 years ago.
5.8-beta3-51224
2021-06-24_23-33-23.png (238.9 KB) - added by oglekler 3 years ago.
Customizer
2021-06-24_23-49-24.png (342.9 KB) - added by oglekler 3 years ago.
Firefox - Dark Mode enabled by the button
2021-06-24_23-47-23.png (53.6 KB) - added by oglekler 3 years ago.
Firefox - Dark Mode enabled by the button - Widgets
widget-screens-53429.png (90.7 KB) - added by poena 3 years ago.
With PR 1471 applied
customizer-no-dark-mode.PNG (391.0 KB) - added by Boniu91 3 years ago.
There's no possibility to enable/disable Dark Mode support. On the frontend Dark Mode is enabled, in customizer it's disabled and no way to enable it

Download all attachments as: .zip

Change History (66)

@oglekler
3 years ago

Dark Mode: On

@oglekler
3 years ago

Dark Mode: Off

#1 @oglekler
3 years ago

And in addition, these Dark Mode buttons all over the place. I suppose they don't need to be there at all (and they are not working).

#2 @desrosj
3 years ago

  • Milestone changed from Awaiting Review to 5.8

#3 @mukesh27
3 years ago

Agree with @oglekler. There should be only one button for widget screen.

#4 @desrosj
3 years ago

  • Keywords has-screenshots needs-patch reporter-feedback added

@oglekler I'm unable to get the Dark Mode button to show in the admin. Can you provide more steps to reproduce?

I have the Twenty Twenty-One theme active, Dark Mode enabled in the theme. On the front end, I see the toggle button, but there is no button on the widget edit screen, even after toggling the button on the front end to each state.

#5 @oglekler
3 years ago

  • Keywords reporter-feedback removed

@desrosj

I've reproduced it on a second testing site with the same result.

Appearance > Customize > Colors & Dark Mode >
check 'Dark Mode support', Publish changes and it's enough.

  • WP Version - 5.8-beta2-51167 and 5.8-beta2-51179
  • Only WordPress Beta Tester plugin enabled
  • Latest theme Twenty Twenty-One version - 1.3
  • No errors in the console.


Last edited 3 years ago by oglekler (previous) (diff)

#6 @isabel_brison
3 years ago

To test, dark mode needs to be enabled as default in system preferences; the theme will follow whatever the system default is.

A fix for this issue has just been merged here: https://github.com/WordPress/gutenberg/pull/32683/files#diff-8446adf7f1ecfdca32179d83664702bd4e3120ce0ba246d874153f222e9dc26eL93 and will be backported to Core, so let's re-test this once Beta 3 is out.

#7 @ryelle
3 years ago

  • Keywords needs-patch removed

Removing needs-patch since it's fixed in Gutenberg. For what it's worth, I still can't replicate the issue. Maybe it needs to be the default sidebar?

#8 @desrosj
3 years ago

  • Keywords needs-testing added

#9 @desrosj
3 years ago

Thanks for giving more details @oglekler.

I am now able to reproduce, but only after applying the PR attached to #53344. I also do not have a Dark Mode option on the version of MacOS I have currently, so I needed to use the button to turn it on before visiting the admin screen. Otherwise, it displayed normally.

Let's test this after the beta 3 changes are all merged, and see if this is resolved.

#10 @desrosj
3 years ago

  • Keywords needs-testing removed
  • Resolution set to fixed
  • Status changed from new to closed

My testing after [51198-51200] shows that this has been resolved. Going to close this out as fixed, but if someone is still seeing this issue, please reopen with test instructions.

#11 @oglekler
3 years ago

@desrosj I have not seen any changes…

@oglekler
3 years ago

5.8-beta3-51224

@oglekler
3 years ago

Customizer

#12 @desrosj
3 years ago

@oglekler can you provide some more details?

  • which OS do you have?
  • Is Dark Mode enabled at the OS leave?
  • which browser(s) do you see this in?
  • Can you confirm the version you’re now testing on?

#13 @oglekler
3 years ago

@desrosj

  • Windows 8.1 (I can test on Windows 10 and Windows 7 but not today)
  • I don't this that I have that dark mode in the OS
  • Chrome 91.0.4472.106
  • WordPress 5.8-beta3-51224
  • Twenty-Twenty One theme (the last Version: 1.3)
  • only WordPress Beta Tester active

I've checked on Firefox 89.0.2 - the same result:

@oglekler
3 years ago

Firefox - Dark Mode enabled by the button

@oglekler
3 years ago

Firefox - Dark Mode enabled by the button - Widgets

#14 @Clorith
3 years ago

I've managed to replicate this reliably, the reasoning appears to be by setting the Background Color in the Customizer to a light color, the darkmode also sets font colors to be a light color. In the frontend, the darkmode stylesheet overrides the globally defined bright color, but this stylesheet is never loaded in the backend, allowing the configured light background color to be used for each widget block.

#15 @isabel_brison
3 years ago

I can also reproduce this with a custom set light background colour. Looking into it now.

#16 @isabel_brison
3 years ago

This is a thorny one. All the correct stylesheets are being loaded, but the legacy widget preview is missing a conditional classname on its <html> element that TT1 styles depend on to override the custom background colour set by the customizer. The missing classname is 'respect_user_color_preference' and it's added to the theme header template: https://github.com/WordPress/wordpress-develop/blob/master/src/wp-content/themes/twentytwentyone/classes/class-twenty-twenty-one-dark-mode.php#L239

Because the widget preview doesn't render a whole page but only the single widget, we're missing that crucial bit of markup to hook the styles to.

Not sure what's the best way forward here. If we had a Core API defining classnames for themes to provide dark mode support we could fix this globally, but as it is each theme provides its own. I'm reluctant to hardcode background and text colors in the preview as that kind of defeats the purpose of having a preview at all :\

#17 @Clorith
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening (forgot to do that previously), as this issue is still present.

#18 @Clorith
3 years ago

Quick update, as there's one step that isn't super obvious in testing here.

The color issue only represents it self in the Legacy Widget preview, proper blocks are all looking good here, this limits the impact to any existing WP users, since block-based widgets are set up by default in a new install starting with WordPress 5.8

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 years ago

#20 @desrosj
3 years ago

  • Keywords needs-patch added

This one still needs a patch. Even though RC1 is happening in a few hours, I'm OK with letting this one sit a bit longer because it only affects Twenty Twenty-One and not Core itself.

If someone is able to fix this before RC2, I think it's fine to include.

#21 follow-up: @desrosj
3 years ago

  • Milestone changed from 5.8 to 5.8.1

Sorry for the noise, but I'm actually going to punt this to 5.8.1 for now. If a fix is created, we can consider including it.

#22 @aristath
3 years ago

PR https://github.com/WordPress/wordpress-develop/pull/1471 fixes the issue by tweaking the TwentyTwenty-One theme, and exiting early if it detects the request is for a widget-preview.

Since this is a commit in the twentytwenty-one theme, it can be published independently from WP 5.8.

@poena
3 years ago

With PR 1471 applied

#23 @poena
3 years ago

With PR 1471 the dark mode toggle button no longer overlaps content in the widget screens.

The original body background color is showing in the widget screen in the admin area, but the widgets are visible and that's an improvement.

#24 in reply to: ↑ 21 @SergeyBiryukov
3 years ago

  • Milestone changed from 5.8.1 to 5.8

Replying to desrosj:

If a fix is created, we can consider including it.

Moving back to 5.8 as a fix is now available.

#25 @SergeyBiryukov
3 years ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in PR #1486 on WordPress/wordpress-develop by desrosj.


3 years ago
#26

This allows for a later setup, which allows get_current_screen() to be used to check that the user is on the widget screen.

Trac ticket: https://core.trac.wordpress.org/ticket/53429.

#27 @desrosj
3 years ago

  • Keywords needs-testing added

I've added an alternate solution to the first PR that refactors how the action and filter hooks are registered a bit to allow the code checking for the widgets screen to make use of get_current_screen().

Could everyone give that a look and some testing when you're able?

#28 @isabel_brison
3 years ago

I tested PR 1486 and it's working well; same as 1471. Thanks for fixing this!

#29 @aristath
3 years ago

I tested https://github.com/WordPress/wordpress-develop/pull/1486 and it works brilliantly.
The code looks good and properly fixes the issue by changing when the hooks get loaded (and skipping them in the widgets screen).
However, it only fixes the issue in the widgets screen and not the customizer so it would be nice if we account for that screen as well.

#30 @desrosj
3 years ago

I've updated the PR, and it should now fix the issue in the Customizer. I have noticed something strange though. The Customizer shows the Color & Dark Mode section in the sidebar, but the actual Dark Mode toggle is set to display:none, so you can't actually enable or disable Dark Mode support. I have not yet been able to figure out why that is.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#32 @Boniu91
3 years ago

I can confirm that it fixes:

  1. Appearance > Widgets section
  2. Customize > Widgets sidebar section

However, with the fix we're not able to:

  1. Enable/disable the Dark Mode support anymore from the Dark Mode customizer section.
  2. Switch between the dark/light in customizer, it's always light, even if it's set to dark on the frontend.

@Boniu91
3 years ago

There's no possibility to enable/disable Dark Mode support. On the frontend Dark Mode is enabled, in customizer it's disabled and no way to enable it

This ticket was mentioned in Slack in #core by desrosj. View the logs.


3 years ago

#34 @desrosj
3 years ago

  • Milestone changed from 5.8 to 5.8.1

Unfortunately, I have not been able to wrap up the attached PR. I'm punting this to 5.8.1, but since themes can be released independently of WordPress Core, the fix could be released whenever it's ready.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#38 @desrosj
3 years ago

  • Milestone changed from 5.8.1 to 5.8.2

5.8.1 RC is due out in less than 24 hours. Since this has not been able to receive attention the last few weeks, I am punting to 5.8.2.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by fabiankaegy. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by justinahinon. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by justinahinon. View the logs.


3 years ago

#44 @justinahinon
3 years ago

Test Report

Env

  • WordPress: 5.8.1
  • Browser: Firefox Developer Edition 93.0b8
  • OS: macOS Monterey 12.0 Beta
  • Localhost: wp-env
  • Theme: Twenty Twenty-One
  • Plugins: none
  • Scripts: none

Steps to Test

  • On a fresh site activate Twenty Twenty One theme
  • Go to Appearance > Customizer > Colors & Dark Mode and enable the Dark Mode checkbox
  • Enable the Dark Mode with the bottom of the screen bottom right
  • Go to Appearance > Widgets. You should see that the widgets shows correctly in Dark Mode
  • Apply the PR https://github.com/WordPress/wordpress-develop/pull/1486
  • Go to Appearance > Customizer > Colors & Dark Mode, you will notice that the Dark Mode checkbox is not present, and it is not possible to enable dark mode

Test Results

This ticket was mentioned in Slack in #core-test by justinahinon. View the logs.


3 years ago

#46 @hellofromTonya
3 years ago

  • Keywords needs-testing removed

#47 @poena
3 years ago

  • Milestone changed from 5.8.2 to 5.9

#48 @poena
3 years ago

  • Keywords needs-patch added; has-patch removed

This ticket was mentioned in Slack in #themereview by kafleg. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 years ago

#51 @chaion07
3 years ago

Hi @oglekler! Thank you so much for reporting this. We discussed this ticket during a recent bug-scrub and based on the discussion, we feel the need for some attention from a developer if possible.Currently our efforts and priorities elsewhere for 5.9 and other areas. Cheers!

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#53 @costdev
3 years ago

GB 36759 (#53801 on Trac) will remove the "Dark Mode" toggle buttons from legacy widget previews.

The "Dark Mode" toggle button isn't part of the legacy widget's content and isn't supposed to display in the preview. It's loaded because all scripts and styles are loaded in the previews, rather than the specific scripts and styles that the legacy widget requires.

Just posting this as a note for those working on the ticket.

#54 @audrasjb
3 years ago

  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Closing this ticket as it seems it was fixed in #53801 and [52277].

hellofromtonya commented on PR #1471:


3 years ago
#55

Closing as the ticket is closed from a fix in https://core.trac.wordpress.org/changeset/52277.

hellofromtonya commented on PR #1486:


3 years ago
#56

Closing as the ticket is closed from a fix in https://core.trac.wordpress.org/changeset/52277.

Note: See TracTickets for help on using tickets.