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

Improve messaging when realm push notifications are not enabled #5785

Closed
alya opened this issue Nov 3, 2023 · 8 comments · Fixed by #5802
Closed

Improve messaging when realm push notifications are not enabled #5785

alya opened this issue Nov 3, 2023 · 8 comments · Fixed by #5802

Comments

@alya
Copy link
Collaborator

alya commented Nov 3, 2023

There are several places in the mobile app where we communicate that a user will not receive push notifications when realm_push_notifications_enabled is false. We should improve the messaging and flows for those notifications.

Home screen banner

  • Text: "Push notifications are not enabled for {realmName}." (same for admins and non-admins)
  • Buttons:
    • "Dismiss" to dismiss for 2 weeks (unless we prefer the current "Remind me later")
    • "Learn more" to:
      • Dismiss the banner for 2 weeks
      • Go to Notifications screen

Notice in Settings (Notifications button)

  • Change text to: "Push notifications are not enabled for {realmName}."

Notice on login screen

Notifications screen

Notices

  • Just in the case of realm_push_notifications_enabled = false: "Push notifications are not enabled for {realmName}."

  • For all situations where we warn that notifications may not be working: "View trouble-shooting guide for mobile push notifications."

New setting

A switch labeled: "Silence warnings about disabled mobile push notifications" (off by default)

If you enable the switch, the warning banner on the home screen and the notice on the login screen are turned off. (The info is still available in the settings.)

@gnprice
Copy link
Member

gnprice commented Nov 4, 2023

@alya
Copy link
Collaborator Author

alya commented Dec 4, 2023

@_Alya Abbott|19257 said:

Looking back at this, maybe all the "have not been enabled" strings should actually be "are not enabled"? That way we don't imply that they never worked in the past, which I suppose could be incorrect.

@alya
Copy link
Collaborator Author

alya commented Dec 5, 2023

Updated the issue with that change.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Dec 8, 2023

On the "Notifications" screen, here's what the gray-shaded per-account area currently looks like when the server hasn't enabled notifications (realm_push_notifications_enabled is false), vs. when there's some other problem:

Server hasn't enabled Some other problem
image image

Quoting from the current issue description:

Notifications screen

Notices

  • Just in the case of realm_push_notifications_enabled = false: "Push notifications are not enabled for {realmName}."
  • For all situations where we warn that notifications may not be working: "View trouble-shooting guide for mobile push notifications."

New setting

A switch labeled: "Silence warnings about disabled mobile push notifications" (off by default)

If you enable the switch, the warning banner on the home screen and the notice on the login screen are turned off. (The info is still available in the settings.)

For this part—

  • Just in the case of realm_push_notifications_enabled = false: "Push notifications are not enabled for {realmName}."

—that's an easy change to the "not set up" row's text.

❓ When you tap the row, should it open the troubleshooting article, so you can learn what it means for push notifications to be enabled for your org?

❓ And for the "New setting" part, probably we want that setting to only appear when the server hasn't enabled push notifications, right?

For this other part—

  • For all situations where we warn that notifications may not be working: "View trouble-shooting guide for mobile push notifications."

—I wonder if the change to be made is to add a link to the troubleshooting guide from the "Troubleshooting" page itself. ❓ Does that sound good? If not, I'd like some guidance on what change we want to make to the "Notifications" screen in cases where there's a problem other than realm_push_notifications_enabled being false. Depending on the notification issue, the path to resolving it might be quicker if you visit the "Troubleshooting" page before reading the troubleshooting guide. (For example, in one case, we offer a "Retry" button that could in theory fix the issue within a second of pressing it.)

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
One of the Role.Admin conditions had actually been going the wrong
way, showing the admin message to non-admins and the non-admin
message to admins. Fortunately that was on the notification
troubleshooting screen, which you can't have reached unless the
server had been set up for notifications earlier in the mobile app's
session.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
…tions

In particular, the message

> Push notifications are not enabled for {realmName}.

can now appear in these places:

- on the "Notifications" button in the settings screen
- in the dialog shown when you tap a warning icon on the active
  account in the "Pick account" screen.

Before this, both of those places would just say:

> Notifications for this account may not arrive.

which was less definitive than we would like. (If the server hasn't
enabled push notifications, then they definitely won't arrive.)

In this commit, we also start using those same two places to give
other actionable feedback as it applies:

> Notifications are disabled in system settings.

> Notifications require Google Play Services, which is unavailable.

For the "Google Play Services" message, this commit also causes it
to appear on the "Troubleshooting" button in the notification
settings screen. (When the "not enabled for {realmName}" case or the
"disabled in system settings" case applies, the notification
settings screen already has appropriate messaging, since before this
commit, that doesn't involve the "Troubleshooting" button.)

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
And also on ServerCompatBanner, since it seems right to be
consistent between the two banners.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 14, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 15, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 15, 2023
And also on ServerCompatBanner, since it seems right to be
consistent between the two banners.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 15, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 15, 2023
@alya
Copy link
Collaborator Author

alya commented Dec 15, 2023

When you tap the row, should it open the troubleshooting article, so you can learn what it means for push notifications to be enabled for your org?

I think this heading would be most appropriate when notifications are not enabled on the server:

https://zulip.com/help/mobile-notifications#enabling-push-notifications-for-self-hosted-servers

@alya
Copy link
Collaborator Author

alya commented Dec 15, 2023

And for the "New setting" part, probably we want that setting to only appear when the server hasn't enabled push notifications, right?

I think the setting can always be there. There may be other reasons why users get nagged about push notifications, and they can just ignore it if they've never seen a banner.

@alya
Copy link
Collaborator Author

alya commented Dec 15, 2023

I wonder if the change to be made is to add a link to the troubleshooting guide from the "Troubleshooting" page itself.

Sure, let's try that.

@chrisbobbe
Copy link
Contributor

When you tap the row, should it open the troubleshooting article, so you can learn what it means for push notifications to be enabled for your org?

I think this heading would be most appropriate when notifications are not enabled on the server:

https://zulip.com/help/mobile-notifications#enabling-push-notifications-for-self-hosted-servers

Oh indeed, yeah. I just checked and that's actually the heading I ended up using in the PR :)

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 16, 2023
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 3, 2024
One of the Role.Admin conditions had actually been going the wrong
way, showing the admin message to non-admins and the non-admin
message to admins. Fortunately that was on the notification
troubleshooting screen, which you can't have reached unless the
server had been set up for notifications earlier in the mobile app's
session.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 3, 2024
…tions

In particular, the message

> Push notifications are not enabled for {realmName}.

can now appear in these places:

- on the "Notifications" button in the settings screen
- in the dialog shown when you tap a warning icon on the active
  account in the "Pick account" screen.

Before this, both of those places would just say:

> Notifications for this account may not arrive.

which was less definitive than we would like. (If the server hasn't
enabled push notifications, then they definitely won't arrive.)

In this commit, we also start using those same two places to give
other actionable feedback as it applies:

> Notifications are disabled in system settings.

> Notifications require Google Play Services, which is unavailable.

For the "Google Play Services" message, this commit also causes it
to appear on the "Troubleshooting" button in the notification
settings screen. (When the "not enabled for {realmName}" case or the
"disabled in system settings" case applies, the notification
settings screen already has appropriate messaging, since before this
commit, that doesn't involve the "Troubleshooting" button.)

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 3, 2024
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 3, 2024
And also on ServerCompatBanner, since it seems right to be
consistent between the two banners.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 3, 2024
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 3, 2024
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 4, 2024
One of the Role.Admin conditions had actually been going the wrong
way, showing the admin message to non-admins and the non-admin
message to admins. Fortunately that was on the notification
troubleshooting screen, which you can't have reached unless the
server had been set up for notifications earlier in the mobile app's
session.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 4, 2024
…tions

In particular, the message

> Push notifications are not enabled for {realmName}.

can now appear in these places:

- on the "Notifications" button in the settings screen
- in the dialog shown when you tap a warning icon on the active
  account in the "Pick account" screen.

Before this, both of those places would just say:

> Notifications for this account may not arrive.

which was less definitive than we would like. (If the server hasn't
enabled push notifications, then they definitely won't arrive.)

In this commit, we also start using those same two places to give
other actionable feedback as it applies:

> Notifications are disabled in system settings.

> Notifications require Google Play Services, which is unavailable.

For the "Google Play Services" message, this commit also causes it
to appear on the "Troubleshooting" button in the notification
settings screen. (When the "not enabled for {realmName}" case or the
"disabled in system settings" case applies, the notification
settings screen already has appropriate messaging, since before this
commit, that doesn't involve the "Troubleshooting" button.)

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 4, 2024
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 4, 2024
And also on ServerCompatBanner, since it seems right to be
consistent between the two banners.

Fixes-partly: zulip#5785
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 4, 2024
chrisbobbe added a commit that referenced this issue Jan 5, 2024
The changes in #5785 and #5805 will be user-visible, but probably
not worthy of highlighting explicitly in the user notes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants