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

add support for mark-read-generic and mark-read-messages #4443

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

haileyok
Copy link
Contributor

@haileyok haileyok commented Jun 9, 2024

Why

We've added support for badge incrementing through #4005 and #4233. However, as of right now the only way to "decrement" the badge is to open the app and opening your notifications tab. Instead, we want the badge to decrement in every possible case:

  • You open the notifications tab on the current device
  • You open the notifications tab on another device
  • You read a message on the current device
  • You read a message on another device

How

Right now, we use updateSeen to mark notifications having been "read". This works alright, but it doesn't give us any solid info or granularity. Nonetheless, for now we can just know that our "generic" notification badge count is going to drop to zero any time updateSeen is called.

In the case of messages, we again do not have a good way to "mark all as read". So, we will currently rely on just decrementing the messages badge count by one whenever we receive a mark-read-messages.

In both of these cases though, we should anticipate being able to provide a "count" of messages to decrement by. Therefore, in both cases I've added the ability to pass along a decrementBy field in the notification, which will tell the client to decrement the badge by that number of notifications. the current implementation makes this a little bit sketchy - sometimes we don't actually receive push notifications, I'm not sure why and that needs to get investigated - so this is solely a "looking forward" bit of logic rather than something we're going to do right now.

Test Plan

We'll first need to deploy the backend changes (bluesky-social/atproto#2567), at which point we'll start receiving mark-read-generic reason notifications. The good news is that we can test this at any time, since we do not need to supply a title or body field for the notification, and as such iOS will not display it to the user anyway - even without the NSE "intercepting" the notification.

  • Deploy Send mark-read-generic notification on updateSeen atproto#2567
  • Use this client on device (`yarn intl:build && eas build -p ios --profile testflight --local --output build.ipa --non-interactive && eas submit -p ios --path build.ipa)
  • Send yourself some notifications and observe the badge increment properly
  • On another device or on the web app, open your notifications tab to clear the badge
  • Test that the iOS badge also decrements whenever you do this

Testing that notifications with empty title and body do not show up on iOS

Screen.Recording.2024-06-08.at.6.21.02.PM.mov

Copy link

render bot commented Jun 9, 2024

Copy link

github-actions bot commented Jun 9, 2024

The Pull Request introduced fingerprint changes against the base commit:

Fingerprint diff
[{"type":"dir","filePath":"modules/expo-background-notification-handler/android","reasons":["expoAutolinkingAndroid"],"hash":"64c51c85980a54b168c887d76bcf94fbd1fa1ace"},{"type":"dir","filePath":"modules/expo-background-notification-handler/ios","reasons":["expoAutolinkingIos"],"hash":"0d41246ffadb0207381c45ada5b3b32d31f236ac"}]

Generated by PR labeler 🤖

Copy link

github-actions bot commented Jun 9, 2024

Old size New size Diff
7.92 MB 7.92 MB 0 B (0.00%)

update kotlin stub

update js types

update the correct badge count

clamp single decrements too

clamp decrement

future proof with `decrementBy`

add support for `mark-read-generic` and `mark-read-messages`

do nothing when receiving `mark-read-*` is received

simplify swift

update android with new types

add types to `NotificationReason`

add logic for decrementing the badge

add decrement mutation
@haileyok haileyok force-pushed the hailey/decrement-badge branch from 26c9e1c to 7dad1c2 Compare June 10, 2024 15:33
@haileyok haileyok marked this pull request as draft June 26, 2024 13:46
@haileyok haileyok changed the base branch from main to hailey/shared-preferences June 26, 2024 16:58
@haileyok haileyok force-pushed the hailey/shared-preferences branch from 1a8a154 to 37ff335 Compare June 26, 2024 18:05
@haileyok haileyok force-pushed the hailey/shared-preferences branch from ac0ba3a to 6823d10 Compare June 26, 2024 18:53
@haileyok haileyok force-pushed the hailey/shared-preferences branch 3 times, most recently from fad6f51 to 6f96bb7 Compare July 2, 2024 01:08
@haileyok haileyok force-pushed the hailey/shared-preferences branch from 6f96bb7 to ffe15a9 Compare July 2, 2024 01:11
@haileyok haileyok changed the base branch from hailey/shared-preferences to main July 12, 2024 23:09
@haileyok haileyok marked this pull request as ready for review July 13, 2024 00:18
@haileyok
Copy link
Contributor Author

This should be ready for testing now as soon as we deploy the courier and appview changes.

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.

1 participant