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

Refactoring brave theme related modules #2905

Merged
merged 2 commits into from
Jul 17, 2019
Merged

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Jul 12, 2019

Enable dark mode of webui/base native ui on platforms that doesn't support system dark mode like win7/win8/linux/mac high sierra.

On linux, gtk theme is considered as a different theme.
So, when user chooses gtk theme instead of classic, it is respected instead of brave theme color for native.

Fix brave/brave-browser#3793
Fix brave/brave-browser#5098
Fix brave/brave-browser#4637
Fix brave/brave-browser#965

Dark at Ubuntu
Screen Shot 2019-07-12 at 16 05 42

gtk theme at Ubuntu with dark
Screen Shot 2019-07-15 at 14 30 02

classic theme at Ubuntu with dark
Screen Shot 2019-07-15 at 14 35 45

Dark at Win8
Screen Shot 2019-07-12 at 23 13 02

Submitter Checklist:

Test Plan:

yarn test brave_browser_tests --filter=BraveThemeServiceTest.NativeThemeObserverTest

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@simonhong simonhong self-assigned this Jul 12, 2019
@simonhong simonhong force-pushed the refactoring_brave_theme branch 5 times, most recently from 5317fe9 to 346db29 Compare July 12, 2019 13:24
@simonhong simonhong marked this pull request as ready for review July 12, 2019 14:14
@simonhong simonhong requested a review from bridiver as a code owner July 12, 2019 14:14
@simonhong simonhong added this to the 0.69.x - Nightly milestone Jul 12, 2019
@simonhong simonhong force-pushed the refactoring_brave_theme branch 3 times, most recently from d4f484b to bea8809 Compare July 12, 2019 14:52
@simonhong simonhong force-pushed the refactoring_brave_theme branch 2 times, most recently from 544decb to 6092379 Compare July 14, 2019 21:39
Enable webui/base native dark mode on platforms that doesn't support
system dark mode like win7/win8/linux/mac high sierra.
gtk theme is considered as a different theme type.
Also, classic theme is set by default insted of gtk theme for using
whole brave theme as a default.
@simonhong simonhong force-pushed the refactoring_brave_theme branch from e561add to 6bc2c3f Compare July 16, 2019 00:46
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified all code works as expected! Changes look good to me. Nice patch cleanup too!

@bridiver can you give this a review too? 😄

Copy link
Member

@petemill petemill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing all the linux issues and making the code more understandable @simonhong.

I noticed in your screenshot that the Windows titlebar seems lighter than I remember seeing. Is that a change from this branch?

image

@simonhong
Copy link
Member Author

simonhong commented Jul 17, 2019

@petemill Our title bar background is not used on Win8. I don't know the reason for now. Needs investigate. Maybe Win8 doesn't allow it? (not sure). I think it has been around for long time.

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