Problem/Motivation
Tested on Firefox Private Mode (to avoid local storage leftovers), with or without ThemeButton island activated:
| OS: Light | OS: Dark | |
| Default admin theme: Auto | ✅
| ✅
|
| Default admin theme: Light | ✅
| ❌
|
| Default admin theme: Dark | ❌
| ✅
|
| Claro theme | ✅
| ✅![]() |
By design, Shoelace doesn't manage user preferences for color scheme (light/dark), it must be manually managed by the "host" using Shoelace (it is what we do ThemeButton) and injected as sl-theme-dark class ("Light" is the default).
If we disable ThemeButton and tweak a little mechanism to map "Default admin" theme to shoelace, we have good results
| Mapped to shoelace | OS: Light | OS: Dark | |
| Default admin theme: Auto | No value or sl-theme-dark, according to browser preferences
| ✅
| ✅
|
| Default admin theme: Light | No value (Light is default) | ✅
| ✅
|
| Default admin theme: Dark | sl-theme-dark
| ✅
| ✅
|
| Claro: Light | Nothing | ✅
| ✅![]() |
But as soon as we activate ThemeButton island in the profile, we have the initial result again, with color mismatch, even without doing any action in the dropdown.
It is a big deal because ThemeButton has been activated by default since #3630921: Minor fixes post preview/iframe last week, a change neither discussed, nor reviewed. 😔
Proposed resolution
First, we need to disable ThemeButton in the default profile, reducing the hurdle, saving valuable room in the toolbar, and keeping this power feature as an opt-in.
But we need to also address the case when people activate it.
It would be very tempting to simply remove ThemeButton from the codebase, but we will lose the storable user preference (only site admin would decide about the color scheme). So, let's keep it until #3606655: Allow user-configurable UI settings - starting with dark mode is shipped to Core.
Proposal:
- Make Shoelace follows Drupal Default Admin color scheme (not the one stored in config, but the ones in the page markup, the difference is important).
- Make
ThemeButtonswitch Drupal Default Admin color scheme instead of shoelace (so they always match) - Don't display
ThemeButtonwhen theme is not the Default Admin (to avoid switching color scheme in a theme not supporting it like Claro) - Picking "System" option is deleting the local storage (to mitigate the leftovers issue)
- No value means following user preferences from browser, instead of Drupal Default Admin color scheme from config (so Display Builder pages can have a different mode than the rest of the Admin UI, at least until #3606655: Allow user-configurable UI settings - starting with dark mode)
So, the island work like that:
ThemeButton option
| Storage | Admin theme | Shoelace |
| Light | LocalStorageManager.set('theme', 'light') | No class (default) | No class (default) |
| Dark | LocalStorageManager.set('theme', 'dark') | html.dark-mode
| .display-builder.sl-theme-dark
|
| System | LocalStorageManager.remove('theme) | Dynamic class, according to user preferences from browser | |
Test with Default Admin & Claro, and switching between them, with different color values.
| Comment | File | Size | Author |
|---|
Issue fork display_builder-3633745
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
pdureau commentedComment #3
pdureau commentedComment #4
pdureau commentedComment #6
pdureau commentedMR open, ready for review.
Changes:
components/display_builder/js/color_scheme.jsis making Shoelace follows Drupal Default Admin color schemecomponents/theme_menu/theme_menu.jshas been changed to switch Drupal Default Admin color scheme instead of shoelace, and to delete the storage when the "System" option is pickedconfig/optional/display_builder.profile.default.ymlsrc/Plugin/display_builder/Island/ThemeButton.phpis not displayed when the admin theme is not Default AdminAnd the related tests automatically generated by an LLM:
tests/src/Kernel/ThemeButtonTest.php&tests/src/Playwright/Tests/color_scheme.spec.tsFeature review
On a browser Private/Incognito Mode (to avoid local storage leftovers):
ThemeButton: Light.ThemeButton: Dark.ThemeButton: System.ThemeButtonisland disabled. Admin theme: Light.ThemeButtonisland disabled. Admin theme: Dark.ThemeButtonisland disabled. Admin theme: Auto.Testing this was an heavy task, I may have done mistakes. Feel free to do your own tests during the review.
Comment #7
csakiistvanComment #8
csakiistvan@pdureau Your screenshots in #6 cover these cases, so I'm not uploading duplicates.
✅ Tested and works — applied MR !528, rebuilt caches and opened a page layout builder (Theme island disabled) and a Views builder (Theme island enabled, set to Dark) with Default Admin set to Light, Dark, and Auto (the latter under both OS color schemes): the builder always matched the admin UI. On Claro the Theme button is not rendered and the builder stays light. No light/dark mismatch left.
Comment #9
pdureau commentedComment #10
mogtofu33 commentedGin Dark now gets a light builder and no Theme button, and the builder should not write the admin theme's state.
I suggest reading the computed
color-schemeof the body instead, so it works on any theme. An explicit pick wins, otherwise the builder follows the page around it.Also, already on 1.0.x: in Firefox, Shoelace dark reaches
prefers-color-schemeinside our iframes, so a front theme following the system, like DSFR, can render dark there.color-scheme: normalon the Builder and Preview iframes stops it.Comment #11
pdureau commentedGin the contrib theme? It was part of the test cases, but i can add it and address this.
I am not sure I understand. We want the inside of iframe to follow
prefers-color-scheme, right? It is already working like that with DSFR (if the UI skins theme is set to "system").color-scheme: normalstops it, but we are not using this.I will have a look anyway.
Comment #12
pdureau commentedI have added a commt in https://git.drupalcode.org/project/display_builder/-/merge_requests/528/ to support Gin theme from contrib.
UI Suite DSFR seems to follow prefers-color-scheme (when set to "system"). I don' t see what is wrong there. Can you tell me more?
Comment #13
mogtofu33 commentedIMHO Gin contrib support is fine, as long as Default Admin is experimental; we drop it in a follow-up once it is stable.
I would still split the two jobs: Shoelace can follow any theme by reading the computed color-scheme of body, which also covers ui_suite themes through ui_skins and Claro, while the Theme button only switches Default Admin and Gin. I changed my mind when no pick, the builder should follow the page around it, not the browser: an admin theme set to Dark should stay dark in the builder, wdyt?
For DSFR: with a light OS and a dark builder, Firefox gives the iframe prefers-color-scheme: dark, inherited from .sl-theme-dark on the iframe element, so DSFR on "system" renders dark inside the Builder.
Chromium follows the OS, which is why it looks fine there.
Comment #14
mogtofu33 commentedSomething else, I would keep the Theme island in the default profile once the mismatch is fixed.
With no pick the builder follows the page and writes nothing to the host, so an untouched button costs only a toolbar slot, and only on Default Admin and Gin.
A visible switch is what users expect, while an opt-in island in profile config is one editors never find.
Its tooltip should say it applies to the builder only, until #3606655: Allow user-configurable UI settings - starting with dark mode lets it drive the core setting.
Comment #15
pdureau commentedCurrently rebasing (there was some merge conflicts). I will put back in review once it is done.
I honestly don't know if this is what users expects from an out-of-the-box experience. In your extensive survey last summer, we saw some visual building tools with such theme switch, indeed. But there were standalone tools, not parts of a wider Web platform like Drupal is for us.
In the scope of this ticket, I am not personally against putting back the island in default profile.
But I guess it will be removed anyway when we will address UI/UX a last time before shipping 1.0.0, targeting a clean, comforting, securing, first time experience. To convince the newcomers the tool will be easy to use, the less islands in the shipped profiles the better.
Comment #16
pdureau commentedPipeline is green again.
Comment #17
mogtofu33 commentedThanks, I pushed two fixes and put the island back in the default profile, since you are not against it for this ticket.
On Gin, the menu tested
ginDarkMode, but Gin setswindow.ginDarkmode.With nothing picked, the menu also wrote the OS preference over the admin theme's Light or Dark, so builder pages could differ from the other admin pages: it now applies a stored pick only, and System restores the admin theme's mode.
Drupal itself now treats light and dark as basic: Default Admin ships Light, Dark and System, and #3606655: Allow user-configurable UI settings - starting with dark mode makes it a per-user setting.
Until that lands, the switch fills the gap for the builder only, and with no pick it keeps the admin theme's mode and writes nothing, so an untouched switch costs one toolbar slot.
For fewer islands in the shipped profiles before 1.0.0, I suggest a dedicated issue: list every island of the default profile, and judge each one on criteria we agree on first, such as discoverability, cost when unused, and what comparable tools ship.
Profiles already let a site remove any island, so the question is only the default, and I would like to settle it on those criteria rather than on preference, mine included.
Comment #18
pdureau commentedLet's go : RTBC.
Comment #20
mogtofu33 commentedThanks, let's go.
Comment #22
pdureau commentedShipped in beta10 release.