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

layout-v21: Use correct colors for notifications #788

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

luk1337
Copy link

@luk1337 luk1337 commented Dec 29, 2015

Screenshots:
notification: http://i.imgur.com/f9gFp5d.png
notification_big: http://i.imgur.com/7b81w8Q.png

@abarisain
Copy link
Owner

Hi and thanks for the pull request.

I'm not sure it's one we want to merge. Starting with lollipop, music apps seem to use dark notifications. I don't know what the rationale is, but here is a sccreenshot of play music and spotify:

screenshot_20151229-231941

Note that I haven't looked at the Android guidelines for that, so maybe it's explained there.

@luk1337
Copy link
Author

luk1337 commented Dec 29, 2015

Well, some other apps still use white notifications. PowerAmp for example: https://dl2.pushbulletusercontent.com/KliFzWhG4cup5EutgNoIW4cUL03O1p2p/Screenshot_20151230-000603.png

@abarisain
Copy link
Owner

I agree that it looks better (seriously, that screenshot I took looks like shit). I'll try to see if there's any recommendation

@luk1337
Copy link
Author

luk1337 commented Dec 29, 2015

Some music players also use notification background based on cover art color, Eleven from CyanogenMod for example.

@luk1337
Copy link
Author

luk1337 commented Dec 29, 2015

@avuton
Copy link
Contributor

avuton commented Jan 7, 2016

So, I guess the pull request is rejected, but you're going to look into the layout colors?

@abarisain
Copy link
Owner

@avuton I'm pretty neutral about this one. I don't know if we want to switch to a white notification, or keep a dark one.

That said, I don't think the PR should be merged because it hardcodes a lot of colors, which is fine if we hardcode the background, but not if we don't.

@abarisain
Copy link
Owner

Seeing how Android N has white notifications (even GPM uses one) and black notifications stick out like a sore thumb, this should be reworked (by us, possibly), but eventually this will be merged one way or another

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.

3 participants