Skip to content

Conversation

@Brod8362
Copy link

This PR adds icons to the notification screen to show the types of notifications pending, and colors the one currently selected.

The UI is hidden for incoming calls as there isn't enough space.

Normal operation
image
image

Incoming call (hidden)
image

Shown with < 5 notifications
image

Known issues:

  • The voicemail icon is using the "unknown" icon, as the voicemail symbol (codepoint F897) isn't in our copy of FontAwesome, even though it should be.
  • I have not yet tested this on real hardware, though its mostly UI changes so I suspect it should be fine.
  • Will have some minor flash usage increases due to having to include some extra icons

Potential Future Changes:

  • Remove the counter on the top right, as it is now redundant
  • If notification limit is increased such that icons would start going off the top of the screen, we may need to rethink how we show icons. (A possible solution would be only showing 5 at any time, and some kind of ellipsis to show the user "there's more notifications above/below that cannot be seen")

@FintasticMan
Copy link
Member

Nice idea! This will probably have to wait for a little while, as the BLE implementations for notifications currently only support the SimpleAlert and IncomingCall categories. If you want, you could try implementing support for the other categories as well.

@Brod8362
Copy link
Author

I can probably do that (separate PR), is it an issue with Gadgetbridge/companion apps, or is it something that needs to be done from the Pinetime side?

@Brod8362
Copy link
Author

Unless I'm missing something - it seems like doing that is as simple as completing the switch case at AlertNotificationService:71? Or is it more nuanced than that?

Assuming that's the case - can we do it in this PR or does it need one of its own? I know we try to keep PRs to individual features per request, but it feels a bit silly to do so in this instance (though I can see the arguments for both sides)

@Brod8362
Copy link
Author

I've been running this for a few days on my watch - I did implement the switch case in AlertNotificationService, but it appears that the issue is on the companion app side - I'm using Gadgetbridge, and as @FintasticMan said, only standard notifications and calls are supported.

There's also minor clipping that can occur with the icons and text if it happens to not wrap right at the edge.

Either way, this PR will have to wait on support somewhere else, or we can switch to using a "standard" notification icon until that support arrives (I still find the stack quite helpful, even without icon differentiation)

@LinuxinaBit
Copy link

The icons are a little too close to the corner of the grey notification bubble, maybe it's better to have them along the bottom, and shrink the grey notification box vertically so it defines the area available for the icons better visually...
Other than that I think it's a great idea, and I would love to see it in the future!

@LinuxinaBit
Copy link

LinuxinaBit commented Apr 18, 2023

To clarify, I mean something like this (a concept with #835 included, replace with the "1/5" notification counter if not included):

notifications

Tiggilyboo pushed a commit to Tiggilyboo/InfiniTime that referenced this pull request Apr 22, 2023
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