Default theme should be dark or light depending on OS preference

[petemill]

macOS Mojave and Windows 10 support dark vs light appearance themes. Brave should respond to those preferences and default to that OS-level preference, unless the user has explicitly chosen dark or light theme.

[simonhong]

Hmm. it's weird.
Below code prints NSAppearanceNameAqua in brave-core and chromium.
I think it should NSAppearanceNameDarkAuqa in dark mode.

    NSAppearance* appearance = NSAppearance.currentAppearance;
    LOG(ERROR) << [appearance.name UTF8String];

However, sample macos console app prints NSAppearanceNameDarkAqua with same api.

int main(int argc, const char * argv[]) {
    @autoreleasepool {
        NSAppearance* appearance = NSAppearance.currentAppearance;
        NSLog(@"%@", appearance.name);
    }
    return 0;
}

Why chromium and sample app have different result?

[simonhong]

When NSRequiresAquaSystemAppearance is set to true in Info.plist, it overrides system's theme.
Because of this, I got NSAppearanceNameAqua always.
https://developer.apple.com/documentation/appkit/nsappearancecustomization/choosing_a_specific_appearance_for_your_app?language=objc

[simonhong]

Menu text isn't visible in dark mode.

[petemill]

@simonhong the implementation referred to in https://bugs.chromium.org/p/chromium/issues/detail?id=850098 also makes the changes to the code to tell the OS that it supports light / dark appearance modes and also changes the menu background color appropriately. Of course, ours will need to look at ActiveTheme, in case the user changes it.

[petemill]

@bradleyrichter @rossmoody @bbondy @simonhong @rebron we need to re-think the preferences for light / dark mode again when we land OS appearance support, since there would now be multiple places that this configuration value can come from:

• Operating System (macOS Mojave / Win 10)
• User (Preferences / Welcome screen)
• Channel (Default to Dark for Dev, Light for Beta / Release)

Once the user changes the Brave setting to Light / Dark (away from default) there is no UI for changing it back to track the OS value (as we removed 'Default' in brave/brave-core#774).

So I propose either of the following:

Option 1 - remove app-level user choice

• If OS supports OS-level light v dark mode:
  • Brave does not present an configuration option for light or dark mode
  • Brave always listens to the OS value for dark or light
• If OS does not support OS-level light v dark mode:
  • Present option in settings with the values: Light, or Dark
  • Brave continues to provide a default based on channel

Option 2 - more (confusing?) user-choice

• If OS supports OS-level light v dark mode

  • Present option in settings with the values: Light, Dark, or Same as macOS (or Windows)
  • Default to Same as xyzOS (stored as Default under the hood)

• If OS does not support OS-level light v dark mode (same as current master)

  • Present option in settings with the values: Light, Dark
  • Default to current channel-based logic

Option 1 is simplest and seems to fit in with what other apps are doing (including Chromium), but since Settings is advanced anyway then I don't really mind which we go for.

I would also, either way, suggest we consider removing the Channel-based defaults as I question the value: it is no longer a method of determining which channel a user is on since there are and will be many ways to change that.

One downside to Option 2 is that to complicate matters, if we advertise to macOS that we support light / dark mode, but then allow the user to differ the window's dark / ligth appearance separate from the OS then macOS will apply incorrect native UI adornations, e.g. window border tint, context menu color. Brave has this issue right now - we get the light window highlight even when we change our colors to dark, since the OS thinks the Brave window is a regular 'light' window:

[simonhong]

+1 to Option 1.
About channel based default, I think it is the only way to determine active theme type if user doesn't choose their preference and os-level theme doesn't exist.

[simonhong]

I think we should pending this implementation before upstream CLs are merged into C71.
Or should we apply them in advance?

[cndouglas]

@simonhong

One downside to Option 2 is that to complicate matters, if we advertise to macOS that we support light / dark mode, but then allow the user to differ the window's dark / ligth appearance separate from the OS then macOS will apply incorrect native UI adornations, e.g. window border tint, context menu color. Brave has this issue right now - we get the light window highlight even when we change our colors to dark, since the OS thinks the Brave window is a regular 'light' window:

As I mentioned in #1289, you can override the system theme on macOS by using the API NSApplication.appearance.

I just ran a quick test and confirmed that this works (my system setting is dark). Each time one of the checkboxes is toggled, NSApplication.appearance is set to a different value, and the window's appearance changes dynamically.

[simonhong]

@liunkae Yep, I already tested and I found #1189 (comment).
I initially implemented this at chrome layer like this - brave/brave-core#897.
But, it turns out that platform type detector should be in /ui layer because fundamental controls like menu also needs platform theme type.
In upstream, they already added it to ui layer and they are implementing dark theme in chrome ui layer now. So, I'm thinking whether to copy it to brave in advance or wait next version(maybe C72?).
Also, we need to fix our policy to respect os theme type or not.

Thanks for the testing ;)

[btlechowski]

The Windows 10 version has not been fully implemented to the point that it is hidden behind a flag.
Logged #3804 as a follow up to this issue.

Tested on

Brave	0.63.14 Chromium: 73.0.3683.75 (Official Build) dev (64-bit)
Revision	909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS	Windows 10 OS Build 17134.523

[LaurenWags]

@kjozwiak not sure if this one can be marked as passed due to your testing on #1289 (comment) ?

[kjozwiak]

@kjozwiak not sure if this one can be marked as passed due to your testing on #1289 (comment) ?

Yup, checked this case as well while going through #1289. Checked it one more time for this issue 👍

Verification PASSED on macOS 10.14.3 x64 using the following build:

Brave	0.63.14 Chromium: 73.0.3683.75 (Official Build) dev(64-bit)
Revision	909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS	Mac OS X

• ensured that Brave colors: Same as macOS
• switched themes back and forth under General settings on the macOS machine and made sure that Brave adapted as expected.

[ubuntudroid]

Strangely this has stopped working since it has been implemented. Whenever the system changes its appearance (e.g. based on sunrise/sunset) Brave will just ignore this and stick with the previous theme. You have to manually switch to either dark or light theme in Brave settings and then reselect "Same as macOS" which will then finally make Brave pick up the correct theme.

[ubuntudroid]

Back with some more info on this: seems that it only does not work when the system changes its theme while the device is sleeping (or locked?). Otherwise it works as expected.