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

WebUI not changing theme to Light #3881

Closed
petemill opened this issue Mar 25, 2019 · 8 comments · Fixed by brave/brave-core#2126
Closed

WebUI not changing theme to Light #3881

petemill opened this issue Mar 25, 2019 · 8 comments · Fixed by brave/brave-core#2126

Comments

@petemill
Copy link
Member

If you have a WebUI open, it is in dark mode, and then you change to light mode, the webui will not change theme to light mode until it is closed and re-opened.

However, switching from light to dark seems to be fine.

Assigning to 0.63.x for now since this is a bug with webui themeing which is introduced with 0.63.x

Steps to reproduce:

Explicit theme

  1. Open Settings, switch to 'Dark'
  2. Close Settings
  3. Open Settings, it will be 'Dark'
  4. Change theme to Light
    Actual: Browser chrome changes to Light, but Settings remains as Dark
    Expected: Settings changes to Light too

Same as macOS theme

  1. Change macOS theme to Dark
  2. Open Settings, switch to 'same as macOS'
  3. Close Settings
  4. Open Settings, it will be 'Dark' and keep Settings as active tab
  5. Change macOS theme to Light
    Actual: Browser chrome changes to Light, but Settings remains as Dark
    Expected: Settings changes to Light too
@petemill petemill added this to the 0.63.x - Dev milestone Mar 25, 2019
@simonhong
Copy link
Member

Do we use onBraveThemeTypeChanged to get theme change in settings webui?

@simonhong
Copy link
Member

simonhong commented Mar 26, 2019

Found the cause.
features::kWebUIDarkMode is enabled on MacOS/Win10 in C74 and DarkModeHandler only observes non-dark native theme.

@simonhong
Copy link
Member

simonhong commented Mar 26, 2019

One more thing - we can reuse chrome's api to get current theme and event notification instead of adding our api. WDYT? @petemill
loadTimeData.getBoolean('darkMode')
FireWebUIListener("dark-mode-changed", base::Value(UseDarkMode()));

@petemill
Copy link
Member Author

@simonhong this proble
is happening in brave’s chromium webui too which I assume use that technique. E.g. chrome://settings

@simonhong
Copy link
Member

@petemill I think that problem can be fixed by notifying both theme (native/dark aura).
I mean how about using chrome's variable(darkMode) and event(dark-mode-changed) to get current theme type and theme changing instead of using our own.

@petemill
Copy link
Member Author

@simonhong it's possible we can use that, certainly now whilst there is only 'light' and 'dark' vs more (not sure if there will ever be more choices, probably not!). It would need to be available from extensions panel and I don't believe the chromium variable is?

@simonhong
Copy link
Member

@petemill ah.. they are webui messages. I think it's not available for extension..

@btlechowski
Copy link

btlechowski commented Apr 10, 2019

Verification passed on

Brave 0.63.35 Chromium: 74.0.3729.61 (Official Build) beta (64-bit)
Revision 5df2c8936783bd7575987e45d72a92fcf528496b-refs/branch-heads/3729@{#645}
OS Windows 10 OS Build 17134.523

Used test plan from the description.
Logged #4059

Verification passed on

Brave 0.63.44 Chromium: 74.0.3729.75 (Official Build) beta(64-bit)
Revision fdb7915642fef8cf997beac2554709d148e3c187-refs/branch-heads/3729@{#754}
OS Linux
  • Had to switch from GTK+ to Classic theme on Linux for verification

Verification PASSED on macOS 10.14.4 x64 using the following build:

Brave 0.63.44 Chromium: 74.0.3729.75 (Official Build) beta(64-bit)
Revision fdb7915642fef8cf997beac2554709d148e3c187-refs/branch-heads/3729@{#754}
OS Mac OS X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment