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

Fixing issue where toolbar and status bar icons would be black on dar… #6424

Closed

Conversation

JonathanZopf
Copy link
Contributor

Fixing issue #6274. Mostly only modifications in ToolbarIconTintManager. However, I needed to upgrade the AndroidX Core dependency due to a bug in older versions where the status bar color would not change.(see google/accompanist#949 (comment) for reference).

@ByteHamster ByteHamster requested a review from keunes April 12, 2023 17:12
@ByteHamster
Copy link
Member

@keunes is this PR what you had in mind for the issue?

@keunes
Copy link
Member

keunes commented Apr 16, 2023

It's better, but not ideal yet:
Imagepipe_0.jpg

The notification bar (in the light theme) changes colour in time. But the top app bar (hamburger menu, search button, etc) overlap still with the podcast info (cover image + title). Only when the other icons (settings, info, filter) touch the app bar, the app bar gets a background:
Imagepipe_0.jpg

Imagepipe_0.jpg

I think to make the whole look smooth, we should make the background image go all the way to the top, show the app top bar background colour as soon as the cover image & title hit its area, and only at that point change the colour of Android's notification bar (from white to grey, in the light theme).

@JonathanZopf
Copy link
Contributor Author

JonathanZopf commented Apr 17, 2023

So you mean, we should change the background color of the status bar and the action bar rather than the tint of the icons? @keunes

@JonathanZopf
Copy link
Contributor Author

I will take a look and check if thats the better alternative. But of course that would help to avoid the white-over-white issue as in the images you've shown above.

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.

None yet

3 participants