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

[experiment] Send separate notifications when multiple accounts have new messages when app receives push notification. #2534

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Amzd
Copy link
Collaborator

@Amzd Amzd commented Jan 18, 2025

This also fixes notifications for classic email always bunching up with notifications of a chatmail account.

Setup of the screenshot:
The device has two accounts in the same group to simulate receiving multiple notifications at once.

Before After
IMG_0024 IMG_0025

This PR also adds threadIdentifier which means iOS can decide to show each group in its own notification thread, like in the following screenshot.

IMG_0027

This makes it harder to miss notifications for private messages when you are also in a busy group.

Tested on iOS 16 and 17, both work.

@r10s
Copy link
Member

r10s commented Jan 18, 2025

as discussed one-to-one, this is a nice experiment, but it is unclear if that works on all versions. it is also unclear if this is a way that apple "wants" or is more a hack. (otherwise the callback would not be needed at all). at least at some point we were not able to get a UNUserNotificationCenter to emit notifications from the NSE, cmp

so, for now, we aim for stability and do not want to risk to get new bugs in again, in the somehow working notifications.

but interesting, we should keep that in mind, and might pick that up or iterate when we interate over that part again

closing this PR and putting it to resurrection. discussion and pushing can continue, of course

@r10s r10s closed this Jan 18, 2025
@r10s r10s added the notify label Jan 18, 2025
@r10s
Copy link
Member

r10s commented Jan 18, 2025

maybe, things have changed with the "notification center entitlement" (see #2124 )

the "notification center entitlement" allows us to not add a notification. maybe that, as an undocumented side effect, that allows to schedule notifications via UNUserNotificationCenter from NSE (i am pretty sure i read that time posts saying that this is explicitly not possible, but i cannot find a link atm). but even if that's true, i would be hesitant of making use of that hack

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 18, 2025

pushing can continue

hmm I pushed more commits and they don't show up in this PR, so we gotta keep that in mind when picking this up

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 18, 2025

I found this which states it was not possible to queue notifications for the future in NSE in 2019. I can not reproduce this, as I can queue notifications in the future too in DC now, but it has been 6 years since then so maybe it was updated.

@r10s
Copy link
Member

r10s commented Jan 18, 2025

hmm I pushed more commits and they don't show up in this PR, so we gotta keep that in mind when picking this up

oh, did not knew that, we can also reopen for now (maybe it is related to the the PR not being part of deltachat-org? or os that a general thing, that closed PR are frozen and do not follow the branch any longer?)

(usually, we use "close" to make clear that we do not want to have things in just now)

@r10s r10s reopened this Jan 18, 2025
@r10s
Copy link
Member

r10s commented Jan 18, 2025

I found this

here is another pointer: https://developer.apple.com/forums/thread/108340 , but also a bit older

@r10s r10s changed the title Send separate notifications when multiple accounts have new messages when app receives push notification. [experiment] Send separate notifications when multiple accounts have new messages when app receives push notification. Jan 18, 2025
@Amzd
Copy link
Collaborator Author

Amzd commented Jan 18, 2025

here is another pointer: https://developer.apple.com/forums/thread/108340 , but also a bit older

only call the Notification Extension's contentHandler once the UNUserNotificationCenter.current().add() method had fully completed.

I cannot reproduce this, even if I call the contentHandler before add it still works (on iOS 16). Might be a bug that was fixed later.

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 19, 2025

Signal seems to do it this way too (just under many layers of abstraction).

They even just only send a silent notification to update the badge number and all other notifications are through UNUNC

@Amzd Amzd force-pushed the multiple-notifications branch from af78ff5 to 37cab6a Compare January 22, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Closed PRs and Issues
Development

Successfully merging this pull request may close these issues.

2 participants