feat(notification): add notification accent colors - #172
Conversation
| iconDrawable = notification.smallIcon?.loadDrawable(context), | ||
| largeImage = largeImage | ||
| largeImage = largeImage, | ||
| color = notification.color, |
There was a problem hiding this comment.
Do notifications set this field often? If I remember correctly from the NC1 days, most notifications did not have this field?
There was a problem hiding this comment.
Yes! At least in my experience, on stock android the little notification icon adopts this color unless there's an image icon to replace it with. Been running this PR for a few days and so far telegram, gmail, discord, snapchat, it all works.
| * Title font (uint8) | ||
| * Subtitle font (uint8) | ||
| * Body font (uint8) | ||
| * Notification accent color (uint32, ARGB), present only when bucket flag 0x08 is set |
There was a problem hiding this comment.
Any specific reason why this needs to be 32-bit? Pebble's color is 8-bit.
| if (window_notification_data.color != 0) | ||
| { | ||
| const uint32_t color = window_notification_data.color; | ||
| const GColor banner_color = GColorFromRGB( |
There was a problem hiding this comment.
Another reason for using 8-bit instead of 32-bit color, we can create it simply like this:
GColor banner_color = (GColor8) {.argb = window_notification_data.color};
| color & 0xff | ||
| ); | ||
|
|
||
| graphics_context_set_fill_color(ctx, banner_color); |
There was a problem hiding this comment.
This just fills the entire screen with the color? Not sure I like this that much, it's pretty jarring.
Maybe a good alternative would be just coloring the statusbar, like the NC1 did it?
There was a problem hiding this comment.
It may honestly, drawing the color before the notification content? Either way it does create a colored banner title bar, this is what I landed on after messing with it for a while, I'm not very experienced in this level of programming.
There was a problem hiding this comment.
Can you take a picture of a notification with a color?
There was a problem hiding this comment.
Ah that actually looks much better than I expected. No changes needed.
Although a separate request: could you try also making the icon transparent? If not, it's also fine, I can do it after merging.
| * 0x01 - When set, notification was not seen by the user yet | ||
| * 0x02 - When set, notification has a pause enabled | ||
| * 0x04 - When set and this notification is present, app should do periodic vibration | ||
| * 0x08 - When set, the bucket contains a notification accent color |
There was a problem hiding this comment.
I'm not sure I like this flag idea, it seems to add complexity for not much good reason. Maybe if the alpha of the color is set to 0, we can treat it as if the color not present? With the 8-bit color, it would only waste 1 byte on non-colored notifications, which is not that much.
There was a problem hiding this comment.
Very good points, I'll look into the 8 bit colors and get rid of this flag.
Code Coverage
|
ea49328 to
1150ef8
Compare
Lint reportResults
Suppressed ResultsNothing here. |
BREAKING CHANGE: bump the notification sync protocol from version 8 to 9 to transmit notification accent colors.
2201371 to
05aea6b
Compare
|
okay, i switched to the 8 bit color and removed the flag. the color is now 0 if there is no color. the android app converts the color from the 32 bit color that android provides to 8 bit color. I was not able to figure out how to make the icon work right, it would have to be transparent and choose it's (black or white) color based on the banner color. |
|
Thanks! |
2 participants
Closes #8
Adds colored banners to the top of notification windows using Android notification accent colors provided by apps. Visual behavior is only changed for watches with color displays.
Notification.colorfrom androidgcolor_legible_over()from the pebble SDKNotificationParserTest.parseNotificationWithASimpleText()(addedNotification.colorto verify parsing)WatchSyncerImplTest.Sync a notification(added an example color to verify serialization)Tested and working with the android app and watch app together, built from the code in this branch. This addition is pretty simple, basically just pulling the notification color from android and passing it through to the watch. One thing however that is imperfect, is that the app icon has a white background, and I believe that it's generated in the phone this way, so it cannot be fixed without more invasive modifications on the android side, which I am not super sure about, though a fix for this to just make the image transparent if possible would be extremely helpful!
Please let me know if anything needs to be changed, thank you!!