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.

Sign up for GitHub

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

Jump to bottom

Unable to clear badge icon #4182

Open
aleffert opened this issue Jul 6, 2020 · 3 comments
Open

Unable to clear badge icon #4182

aleffert opened this issue Jul 6, 2020 · 3 comments
Labels
a-iOS a-notifications

Comments

@aleffert
Copy link

aleffert commented Jul 6, 2020

Running the latest iOS version (v26.29.52) on the latest iOS (13.5.1).

Steps:
Get @mentioned in a message.
Observe badge with a 1 on the homescreen
View message in the iOS app
Close app
Still observe badge with a 1 on the homescreen

Expected:
No more icon badge since I have nothing unread

I also tried opening up with the desktop app as well as explicitly killing the app on my phone.
@aleffert aleffert mentioned this issue Jul 6, 2020
Closed
@chrisbobbe
Copy link
Contributor

Thanks for the report, @aleffert! This is a known issue, and we're working on a fix; you can follow along here if you like.

@chrisbobbe
Copy link
Contributor

Since this issue has the labels a-IOS and a-notifications, I'm marking it as blocked by #4115, so we only have to implement a solution once. The PR #4163 is currently open for that issue.

@chrisbobbe chrisbobbe added the blocked on other work To come back to after another related PR, or some other task. label Nov 10, 2020
@gnprice gnprice mentioned this issue Nov 10, 2020
Merged
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 2, 2020
@chrisbobbe
ios notifications: Update the badge count if the app is foregrounded.
7a622ac 
This is based on the recommended setup for PushNotificationIOS from
React Native [1], so we might have included it in the next commit,
where we fully set that up. But as Greg points out [2], this code is
purely using upstream iOS APIs, so it doesn't depend on whether
we're using that RN module or something else, so it's fine to do
here on its own.

In a departure from those instructions, we decide to just pass the
thing (`UNNotificationPresentationOptionBadge`) that updates the
badge count when the app is foregrounded. This is a necessary part
of zulip#4182, but one thing we'll still need to do for that is test that
the server is sending the right badge counts to the client.

[1] https://reactnative.dev/docs/0.62/pushnotificationios. These
    instructions are actually missing a required change to the
    AppDelegate.h, which we take here, having seen it in the
    instructions for @react-native-community/push-notification-ios
    (at
    https://github.com/react-native-push-notification-ios/push-notification-ios#update-appdelegateh).

[2] zulip#4163 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Feb 8, 2021
@chrisbobbe
ios notifications: Update the badge count if the app is foregrounded.
15c7984 
This is based on the recommended setup for PushNotificationIOS from
React Native [1], so we might have included it in the next commit,
where we fully set that up. But as Greg points out [2], this code is
purely using upstream iOS APIs, so it doesn't depend on whether
we're using that RN module or something else, so it's fine to do
here on its own.

In a departure from those instructions, we decide to just pass the
thing (`UNNotificationPresentationOptionBadge`) that updates the
badge count when the app is foregrounded. This is a necessary part
of zulip#4182, but one thing we'll still need to do for that is test that
the server is sending the right badge counts to the client.

[1] https://reactnative.dev/docs/0.62/pushnotificationios. These
    instructions are actually missing a required change to the
    AppDelegate.h, which we take here, having seen it in the
    instructions for @react-native-community/push-notification-ios
    (at
    https://github.com/react-native-push-notification-ios/push-notification-ios#update-appdelegateh).

[2] zulip#4163 (comment)
@chrisbobbe chrisbobbe mentioned this issue Mar 8, 2021
Closed
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue May 10, 2021
@chrisbobbe
ios notifications: Update the badge count if the app is foregrounded.
c6383df 
This is based on the recommended setup for PushNotificationIOS from
React Native [1], so we might have included it in the next commit,
where we fully set that up. But as Greg points out [2], this code is
purely using upstream iOS APIs, so it doesn't depend on whether
we're using that RN module or something else, so it's fine to do
here on its own.

In a departure from those instructions, we decide to just pass the
thing (`UNNotificationPresentationOptionBadge`) that updates the
badge count when the app is foregrounded. This is a necessary part
of zulip#4182, but one thing we'll still need to do for that is test that
the server is sending the right badge counts to the client.

[1] https://reactnative.dev/docs/0.62/pushnotificationios. These
    instructions are actually missing a required change to the
    AppDelegate.h, which we take here, having seen it in the
    instructions for @react-native-community/push-notification-ios
    (at
    https://github.com/react-native-push-notification-ios/push-notification-ios#update-appdelegateh).

[2] zulip#4163 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 17, 2021
@chrisbobbe
ios notifications: Update the badge count if the app is foregrounded.
5e6eed9 
This is based on the recommended setup for PushNotificationIOS from
React Native [1], so we might have included it in the next commit,
where we fully set that up. But as Greg points out [2], this code is
purely using upstream iOS APIs, so it doesn't depend on whether
we're using that RN module or something else, so it's fine to do
here on its own.

In a departure from those instructions, we decide to just pass the
thing (`UNNotificationPresentationOptionBadge`) that updates the
badge count when the app is foregrounded, and not ask for other
things like showing the banner or playing a sound. (This new
behavior is necessary for fixing zulip#4182, but I don't think it's
sufficient: we'll still want to test that the server is sending the
right badge counts to the client.)

[1] https://reactnative.dev/docs/0.62/pushnotificationios. These
    instructions are actually missing a required change to the
    AppDelegate.h, which we take here, having seen it in the
    instructions for @react-native-community/push-notification-ios
    (at
    https://github.com/react-native-push-notification-ios/push-notification-ios#update-appdelegateh).

[2] zulip#4163 (comment)
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 18, 2021
@chrisbobbe @gnprice
ios notifications: Update the badge count if the app is foregrounded.
3fc8982 
This is based on the recommended setup for PushNotificationIOS from
React Native [1], so we might have included it in the next commit,
where we fully set that up. But as Greg points out [2], this code is
purely using upstream iOS APIs, so it doesn't depend on whether
we're using that RN module or something else, so it's fine to do
here on its own.

In a departure from those instructions, we decide to just pass the
thing (`UNNotificationPresentationOptionBadge`) that updates the
badge count when the app is foregrounded, and not ask for other
things like showing the banner or playing a sound. (This new
behavior is necessary for fixing zulip#4182, but I don't think it's
sufficient: we'll still want to test that the server is sending the
right badge counts to the client.)

[1] https://reactnative.dev/docs/0.62/pushnotificationios. These
    instructions are actually missing a required change to the
    AppDelegate.h, which we take here, having seen it in the
    instructions for @react-native-community/push-notification-ios
    (at
    https://github.com/react-native-push-notification-ios/push-notification-ios#update-appdelegateh).

[2] zulip#4163 (comment)
@gnprice gnprice removed the blocked on other work To come back to after another related PR, or some other task. label Jun 18, 2021
@gnprice
Copy link
Member

gnprice commented Jun 18, 2021

#4163 is merged! This is now ripe for working on.

In the meantime, back when this issue was new (a few hours after it was filed -- thanks again @aleffert for the report!) we deployed a workaround: the server always sends a badge count of 0. So the current status should be that the app icon is always clear -- that there's never a badge.

Before we deployed that workaround, I reproduced this issue myself:
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20badge.20count/near/926367

So I expect that if we simply reverted the workaround and started sending badge counts, this issue would reappear.

We did fix, as part of #4163, an issue that would have caused the badge count not to update when the app was in the foreground. That could cause symptoms similar to this one; in particular it'd be enough to explain the original report in this thread. But it doesn't explain the repro I described at the link above:

  • The app was never in the foreground.
  • The badge count did successfully update from 3 down to 1 (when it should have been 0); then correctly up to 2; then back down to 1 when it again should have gone to 0.

@gnprice gnprice mentioned this issue Jun 18, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-iOS a-notifications
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gnprice @aleffert @agrawal-d @chrisbobbe