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.

Sign up for GitHub

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

Jump to bottom

Open profile manager causes entire browser UI to go dark #5557

Closed
srirambv opened this issue Aug 6, 2019 · 2 comments · Fixed by brave/brave-core#4210
Closed

Open profile manager causes entire browser UI to go dark #5557

srirambv opened this issue Aug 6, 2019 · 2 comments · Fixed by brave/brave-core#4210
Assignees
@simonhong
Labels
bug feature/themes feature/user-interface All UI related priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/exclude
Milestone
1.4.x - Release

Comments

@srirambv
Copy link
Contributor

srirambv commented Aug 6, 2019 •
edited by bsclifton
Loading

Description

Open profile manager causes entire browser UI to go dark

Test plan / Steps to Reproduce

  1. Upgrade to 0.70.34
  2. Set Brave colour to light, On Linux set theme to use classic
  3. Open Manage people from profile manager, entire webui goes Dark
  4. Close manage people and launch again, no change
    Note: This only happens the first time you try after browser launch and is reproducible each time after the browser is launched.

Actual result:

https://youtu.be/RzhdOvTW2wk

Expected result:

WebUI Colour should not change when manage people window is opened

Reproduces how often:

Easy

Brave version (brave://version info)

Brave 0.70.34 Chromium: 76.0.3809.87 (Official Build) nightly (64-bit)
Revision 111fe1e15d5ced26080a7dc239bcfe70f6c49aad-refs/branch-heads/3809@{#967}
OS Linux

Version/Channel Information:

  • Can you reproduce this issue with the current release? No
  • Can you reproduce this issue with the beta channel? No
  • Can you reproduce this issue with the dev channel? Yes
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

cc: @simonhong
@simonhong
Copy link
Member

Same issue with #5373.

@simonhong simonhong self-assigned this Aug 7, 2019
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Aug 7, 2019
@bsclifton bsclifton mentioned this issue Nov 5, 2019
Closed
@simonhong simonhong added this to the 1.4.x - Nightly milestone Dec 11, 2019
@simonhong simonhong mentioned this issue Dec 11, 2019
Merged
32 tasks
simonhong added a commit to brave/brave-core that referenced this issue Dec 13, 2019
@simonhong
Migrate brave theme prefs from profile to local state
aa59754 
With this, we can fix brave theme related bugs with multi profile
because all profiles use browser-wide prefs.
So far, system theme could be changed whenever new profile is used
because brave theme was profile prefs but system theme is browser
wide setting. And did cleanup and refactoring.

Refactoring point is separation of theme managing and dark mode managing.
So far, theme service has all dark mode handling logic.
But I think theme service handles browser theme with underrlying dark mode.
So, all dark mode logic is in dark_mode namespace.

fix brave/brave-browser#5373
fix brave/brave-browser#5557
simonhong added a commit to brave/brave-core that referenced this issue Dec 18, 2019
@simonhong
Migrate brave theme prefs from profile to local state
24cf9d8 
With this, we can fix brave theme related bugs with multi profile
because all profiles use browser-wide prefs.
So far, system theme could be changed whenever new profile is used
because brave theme was profile prefs but system theme is browser
wide setting. And did cleanup and refactoring.

Refactoring point is separation of theme managing and dark mode managing.
So far, theme service has all dark mode handling logic.
But I think theme service handles browser theme with underrlying dark mode.
So, all dark mode logic is in dark_mode namespace.

fix brave/brave-browser#5373
fix brave/brave-browser#5557
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Feb 13, 2020 •
edited by kjozwiak
Loading

Verification passed on

Brave 1.4.86 Chromium: 80.0.3987.87 (Official Build) beta (64-bit)
Revision 449cb163497b70dbf98d389f54e38e85d4c59b43-refs/branch-heads/3987@{#801}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Light Theme
image

Dark Theme
image

Same as Windows
image

Verification passed on

Brave 1.4.93 Chromium: 80.0.3987.116 (Official Build) (64-bit)
Revision dc00a510e4c2ae25c4d084cc3d946fc782249224-refs/branch-heads/3987@{#917}
OS Linux

Light Theme
image

Dark Theme
image

Verification PASSED on macOS 10.15.3 x64 using the following build:

Brave 1.4.93 Chromium: 80.0.3987.116 (Official Build) (64-bit)
Revision dc00a510e4c2ae25c4d084cc3d946fc782249224-refs/branch-heads/3987@{#917}
OS macOS Version 10.15.3 (Build 19D76)

Screen Shot 2020-02-20 at 8 31 53 PM

Screen Shot 2020-02-20 at 8 31 39 PM

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

@simonhong simonhong

Labels
bug feature/themes feature/user-interface All UI related priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.4.x - Release
Development

Successfully merging a pull request may close this issue.

Migrated brave theme prefs from profile to local state brave/brave-core
6 participants
@kjozwiak @bsclifton @rebron @simonhong @srirambv @GeetaSarvadnya