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

Default theme should be dark or light depending on Windows 10 preference #3804

Closed
btlechowski opened this issue Mar 20, 2019 · 7 comments · Fixed by brave/brave-core#2352
Closed

Comments

@btlechowski
Copy link

Follow up to #1189

This feature has been implemented on MacOS, but Windows version had some problems and is hidden behind a flag. brave/brave-core#1805 (review)

Steps to Reproduce

  1. Clean install
  2. Launch Browser
  3. Check theme settings have three options (Same as Windows, Dark, Light)
  4. Check Same as Windows applies Windows theme to brave theme
  5. Check Dark/Light works independently with Windows theme.

Actual result:

There is no Same as Windows in Brave colors
image

Expected result:

Same as Windows is available in Brave colors
Same as Windows is the default option on Windows 10

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 0.62.32 Chromium: 73.0.3683.75 (Official Build) beta (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Windows 10 OS Build 17134.523
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

cc @brave/legacy_qa @petemill @simonhong

@simonhong
Copy link
Member

This is expected behavior for now because upstream disabled dark theme support on Win10.
When they enable it, we can consider this option again.

@srirambv
Copy link
Contributor

@btlechowski have you checked with setting apps colour?
image
image

By default the app is set to light always.

@simonhong
Copy link
Member

simonhong commented Mar 20, 2019

@srirambv @btlechowski We don't need to test theme on Windows for now.
Chromium didn't enable platform theme support on Win10 yet.

@bsclifton bsclifton added the Chromium/waiting upstream Issue is in Chromium; we'll likely wait for the fix label Mar 21, 2019
@rebron rebron added priority/P4 Planned work. We expect to get to it "soon". priority/P5 Not scheduled. Don't anticipate work on this any time soon. and removed priority/P4 Planned work. We expect to get to it "soon". labels Mar 22, 2019
@simonhong simonhong self-assigned this Apr 14, 2019
@simonhong
Copy link
Member

@btlechowski We can test Same as Windows feature since 0.63 (currently beta)

@simonhong simonhong removed the Chromium/waiting upstream Issue is in Chromium; we'll likely wait for the fix label Apr 15, 2019
@kjozwiak
Copy link
Member

kjozwiak commented May 2, 2019

@simonhong was this upstreamed in one of the C74 bumps?

@simonhong
Copy link
Member

@kjozwiak beta(C74) has Same as Windows option and it's default.
However, needs more fixes to make theme works properly on Windows like MacOS.
Fix for #4059 is merged and waiting for uplifting.
Fix for #4056 is waiting for review.
When all above fixes are uplifted to C74, brave theme will work properly in Windows.

@btlechowski
Copy link
Author

btlechowski commented Jun 12, 2019

Verification passed on

Brave 0.66.87 Chromium: 75.0.3770.80 (Official Build) beta (64-bit)
Revision 9a9aa15057b6b2cc0909bdcf638c0b65ecd516f2-refs/branch-heads/3770@{#948}
OS Windows 10 OS Version 1809 (Build 17763.379)

Verified test plan from brave/brave-core#2352 and from the description. Checked changing Same as Windows(Dark)->Same as Windows(Light) and Same as Windows(Light)->Same as Windows(Dark). Checked settings dropdown, permissions alert box (notifications, geolocation, translations, alerts) and downloads bar.

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

Successfully merging a pull request may close this issue.

6 participants