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

Update status color to match toolbar color #3774

Merged
merged 2 commits into from
Jul 1, 2020

Conversation

eames-palmer
Copy link
Contributor

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Remove translucent status bar
  • Change status bar color to match toolbar color across services

Fixes the following issue(s)

Testing apk

app-debug.apk.zip

Agreement

@eames-palmer eames-palmer requested a review from Stypox June 16, 2020 19:25
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Ok, thank you for the clarification. Then this is good.
In the apk you provided PeerTube's status bar color is still darker, though

@eames-palmer
Copy link
Contributor Author

Thank you for the review! After some thorough research/testing, this commit should contain updates for all the status bars present within NewPipe. Feel free to critique any of the changes I have made or leave further suggestions! I did have to update the colorPrimaryDark for the themes, however, this seemed like the best option as directly changing the status bar color with android:statusBarColor is not supported in NewPipe's minimum SDK.

@eames-palmer eames-palmer requested a review from Stypox June 30, 2020 05:39
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Good, thank you!
Have you checked that colorPrimaryDark is only used for toolbars and nothing else? If that's the case, this is ready in my opinion :-D

@eames-palmer
Copy link
Contributor Author

@Stypox Yes! I triple checked and ran some global searches just to make sure. It looks like colorPrimaryDark is only for the contextual app bar's in NewPipe which, from my testing, only impacted the status bar color!

@Stypox Stypox merged commit 5e9dce7 into TeamNewPipe:dev Jul 1, 2020
This was referenced Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants