Improve messaging when realm push notifications are not enabled #5802

chrisbobbe · 2023-12-14T23:51:42Z

Many screenshots and some discussion coming soon; I'll post in messages after this one. 🙂

chrisbobbe · 2023-12-15T00:37:28Z

This comment is about how this revision relates to the spec in #5785. It shows where the spec is followed, and also where and why it's contradicted in some parts, which we can discuss (maybe on CZO).

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

✅

Before	After

Notice in Settings (Notifications button)

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

✅

Before	After

Notice on login screen

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

For all situations where we warn that notifications may not be working, details button can link to https://zulip.com/help/mobile-notifications#troubleshooting-mobile-notifications.

⚠️

I think you mean the "Pick account" screen, which we discussed in the chat thread. (When you're in the process of logging in / authenticating, with Google or email/password or whatever, we don't have messaging about notifications not working.)
In this revision, I've left the current behavior of the "Details" button: it switches to the account (making it the "active account") and navigates to the account's notification settings screen, titled "Notifications". From there, if it's a vague "may not be working" problem, you can tap "Troubleshooting", and in this revision, the troubleshooting page has a link to the troubleshooting article. (For screenshots of that link, see the comment after this one: Improve messaging when realm push notifications are not enabled #5802 (comment) )
- Is this OK? (Maybe with more explicit text for the "Details" button, like "Go to notifications for this account"?)
- I could follow the spec in the issue and have the "Details" button go straight to the troubleshooting article. But then, if the user needs the "Notifications" or the "Troubleshooting" screen to fix the problem, we're counting on them to switch to the account manually and find those pages themselves. Also, the problem is sometimes an operational issue that can be fixed just by tapping a "Retry" button on the troubleshooting screen, and that fix will be much quicker if you don't read the troubleshooting article first.

Before	After

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."

⚠️

In this revision, I did the first bullet point but not the second; see #5785 (comment). In the "may not be working" cases, I've preserved the link to the in-app troubleshooting screen, which now has a link to the troubleshooting article. The article link is new in this revision; see a screenshot in the comment after this one: #5802 (comment) .

Before	After

Notifications screen

[…]

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.)

✅

Here is the switch in action:

chrisbobbe · 2023-12-15T00:37:44Z

This comment is about additional changes that were easy or seemed natural to do, but not covered in the spec in #5785.

Google Play Services message changed

In the troubleshooting screen, I shortened the message

Notifications require Google Play Services, which is unavailable on this device.

to just

Notifications require Google Play Services, which is unavailable.

because in theory the Services could be available on the device, just disabled or otherwise made unavailable to Zulip.

Bold realm name in user-facing message

Where possible, in the new UI message

Push notifications are not enabled for {realmName}.

, the realm name is bolded. You may have noticed this in screenshots in my previous comment. (This wasn't possible in the native-iOS popup that comes up when you tap a warning icon in the "Pick account" screen.)

The realm name is freeform user-generated content, and I think bolding it is an easy way avoid funny miscommunications by delineating the content from the rest of the message, which is in our voice. (Probably no one would really make a joke realm named "people like you" or something… But in theory some kind of accident could happen, and I'd rather prevent it than think about how likely it is, in English or in any other language we offer the app in. 😛)

The new "silence" setting:

In this revision, if the org goes back and forth between enabling notifications and not, the "silence" setting resets to "off" each time notifications are disabled. I figured the "silence" setting is meant for the situation where you expect notifications to be disabled long-term and you just don't want periodic reminders about it. But if the org is toggling back and forth, there might be an operational issue that you don't want to silence.

~~Also, the setting is hidden when notifications are enabled. Here's a screenshot showing its absence:~~

~~[screenshot]~~

Following Alya's comment #5785 (comment), I've just made the new "silence" setting always be there.

The troubleshooting screen now links to the troubleshooting article

On the troubleshooting screen, you can now tap a row to open this link:

https://zulip.com/help/mobile-notifications#troubleshooting-mobile-notifications

Before	After

Alerts in the troubleshooting screen are un-bolded

See the previous pair of before/after screenshots. Here's the same pair again, but in dark mode:

Before	After

Now, bold text like "support@zulip.com" stands out from surrounding text, as initially intended. It's easy to change the color of that text if it's too light in light mode or too dark in dark mode.

Linking to Help Center instead of RTD:

The home-screen banner's "Learn more" button now links to

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

instead of

https://zulip.readthedocs.io/en/stable/production/mobile-push-notifications.html

. Ditto for the row

Push notifications are not enabled for {realmName}.

when it appears in the notification settings.

Screenshots showing where these links appear:

Banner	Notification settings

Other messages also shown in more places

These two places—

The "Notifications" row in the "Settings" screen
The "Pick account" screen, when you tap a warning icon on an account

—will now also give one of these explicit messages if it applies:

Notifications are disabled in system settings.

Notifications require Google Play Services, which is unavailable.

instead of the vague message

Notifications for this account may not arrive.

Other things

See discussion: Improve messaging when realm push notifications are not enabled #5802 (comment)

chrisbobbe · 2023-12-16T00:07:49Z

Revision pushed, addressing Alya's comments on the issue up to here: #5785 (comment). Here in the PR thread, I've correspondingly updated the screenshots/commentary above.

alya · 2023-12-19T20:46:13Z

All the decisions make sense to me!

alya · 2023-12-19T20:48:14Z

Maybe we should update the warning banner to refer to the troubleshooting guide, not just support?

"... not arrive. Please refer to the troubleshooting guide or contact support@zulip.com with..."

?

chrisbobbe · 2023-12-19T23:30:41Z

Maybe we should update the warning banner to refer to the troubleshooting guide, not just support?

Sure, that sounds good! I think you're referring specifically to this:

… guide As suggested by Alya: zulip#5802 (comment)

chrisbobbe · 2023-12-19T23:41:43Z

Done, and thanks for the review! Revision pushed. @gnprice, over to you when you're feeling better from your sore throat. 🙂

gnprice

Thanks to you both for all the work on this!

I've read through the code, and it all looks good; one small comment below. Now catching up on the discussion thread.

gnprice · 2023-12-21T00:59:48Z

src/account/AccountItem.js

+  // appear in the UI. As of writing, notifProblemShortText doesn't use its
+  // realmName param except in a case where we have server data for the
+  // relevant account. When we have that, `realmName` will be the real realm


Why is this true?

Ah I see, it's because:

notifProblemShortText only uses the realm name for the ServerHasNotEnabled problem;

useNotificationReportsByIdentityKey only returns that problem when we have server data.

Tricky. Maybe worth spelling out a bit more: "in a case where" -> "for a NotificationProblem which we only select when".

gnprice · 2023-12-21T01:32:40Z

OK, all LGTM with that one comment above about clarifying a comment. @chrisbobbe please go ahead and merge when ready after that.

… guide As suggested by Alya: zulip#5802 (comment)

I suppose the Services could be available on the device, just disabled or otherwise made unavailable to Zulip.

…exts These all seem to have gotten the right size and color by accident, from global defaults. But given ZulipText's API, passing both of these is the right pattern for these subparts of UI strings. We never intend to discard a locally chosen color or font size just because we're putting a dynamic value into a translated UI string.

In the message for NotificationProblem.ServerHasNotEnabled, we'd like to make the realm name bold. Now, when we do that, its weight will still be distinct.

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

We'll use this soon for a caller that needs a LocalizableText and can't use a LocalizableReactText.

…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

…tions" Fixes-partly: zulip#5785

And also on ServerCompatBanner, since it seems right to be consistent between the two banners. Fixes-partly: zulip#5785

…avRows

We're about to add a link to the notification troubleshooting guide, and it seems cleanest to do these as rows instead of ZulipButtons.

Fixes-partly: zulip#5785

…tsAction This doesn't belong here. It's already present in the PerAccountAction union, so nothing to do after removing it here.

… built

Fixes: zulip#5785

… guide As suggested by Alya: zulip#5802 (comment)

chrisbobbe · 2024-01-04T01:12:05Z

Thanks for the review!

OK, all LGTM with that one comment above about clarifying a comment. @chrisbobbe please go ahead and merge when ready after that.

Done.

chrisbobbe added the a-notifications label Dec 14, 2023

chrisbobbe requested review from gnprice and alya December 14, 2023 23:51

chrisbobbe force-pushed the pr-messaging-realm-notifs-disabled branch from 1da6c0c to b4151ce Compare December 15, 2023 00:00

chrisbobbe added the P1 high-priority label Dec 15, 2023

chrisbobbe force-pushed the pr-messaging-realm-notifs-disabled branch from b4151ce to b0fc040 Compare December 16, 2023 00:00

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Dec 19, 2023

NotifTroubleshootingScreen: Have genericAlert mention troubleshooting…

977835a

… guide As suggested by Alya: zulip#5802 (comment)

gnprice reviewed Dec 21, 2023

View reviewed changes

alya mentioned this pull request Jan 3, 2024

Disable push-configuration Warning server side. zulip/zulip#28371

Open

chrisbobbe force-pushed the pr-messaging-realm-notifs-disabled branch from 977835a to 05e7ef3 Compare January 3, 2024 22:54

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jan 3, 2024

NotifTroubleshootingScreen: Have genericAlert mention troubleshooting…

05e7ef3

… guide As suggested by Alya: zulip#5802 (comment)

chrisbobbe added 11 commits January 3, 2024 17:09

NotifTroubleshootingScreen: Revise text for Google Play Services warning

dea9f17

I suppose the Services could be available on the device, just disabled or otherwise made unavailable to Zulip.

AlertItem: Make text regular weight, not bold

26c9498

In the message for NotificationProblem.ServerHasNotEnabled, we'd like to make the realm name bold. Now, when we do that, its weight will still be distinct.

notifs [nfc]: Factor out notifProblemShortReactText helper

d0587af

notifs [nfc]: Factor out notifProblemsShortText, from …ShortReactText

47377c3

We'll use this soon for a caller that needs a LocalizableText and can't use a LocalizableReactText.

notifs: Link to Help Center, not RTD, when server hasn't enabled notifs

38882d8

ServerPushSetupBanner: "Learn more" dismisses banner, opens "Notifica…

f2d056b

…tions" Fixes-partly: zulip#5785

ServerPushSetupBanner: Change "Remind me later" button text to "Dismiss"

7c3c164

And also on ServerCompatBanner, since it seems right to be consistent between the two banners. Fixes-partly: zulip#5785

notifs [nfc]: Refactor troubleshooting-screen padding, for upcoming N…

80c95e3

…avRows

chrisbobbe added 8 commits January 3, 2024 17:09

NavRow types: Accept Promise-returning press handler

5a1e5ce

RowGroup [nfc]: Rename, from SettingsGroup; make title optional

6f70dd7

NotifTroubleshootingScreen: Use rows in a group, instead of ZulipButtons

944fee6

We're about to add a link to the notification troubleshooting guide, and it seems cleanest to do these as rows instead of ZulipButtons.

NotifTroubleshootingScreen: Add NavRow linking to troubleshooting guide

ebfb850

Fixes-partly: zulip#5785

redux types: Remove DismissServerPushSetupNoticeAction from AllAccoun…

ec77fa9

…tsAction This doesn't belong here. It's already present in the PerAccountAction union, so nothing to do after removing it here.

PerAccountNotificationSettingsGroup [nfc]: Refactor how children is…

40c8b50

… built

notifs: Add "silence warnings" setting for server-setup banner

2d7e865

Fixes: zulip#5785

NotifTroubleshootingScreen: Have genericAlert mention troubleshooting…

0dbcf8f

… guide As suggested by Alya: zulip#5802 (comment)

chrisbobbe force-pushed the pr-messaging-realm-notifs-disabled branch from 05e7ef3 to 0dbcf8f Compare January 4, 2024 01:10

chrisbobbe merged commit 0dbcf8f into zulip:main Jan 4, 2024
1 check passed

chrisbobbe deleted the pr-messaging-realm-notifs-disabled branch January 4, 2024 01:12

alya mentioned this pull request Jan 25, 2024

Document how to turn off warnings about mobile notifications zulip/zulip#28707

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve messaging when realm push notifications are not enabled #5802

Improve messaging when realm push notifications are not enabled #5802

chrisbobbe commented Dec 14, 2023

chrisbobbe commented Dec 15, 2023 •

edited

Loading

Home screen banner

Notice in Settings (Notifications button)

Notice on login screen

Notifications screen

Notices

Notifications screen

New setting

chrisbobbe commented Dec 15, 2023 •

edited

Loading

chrisbobbe commented Dec 16, 2023 •

edited

Loading

alya commented Dec 19, 2023

alya commented Dec 19, 2023

chrisbobbe commented Dec 19, 2023

chrisbobbe commented Dec 19, 2023 •

edited

Loading

gnprice left a comment

gnprice Dec 21, 2023

gnprice commented Dec 21, 2023

chrisbobbe commented Jan 4, 2024

Improve messaging when realm push notifications are not enabled #5802

Improve messaging when realm push notifications are not enabled #5802

Conversation

chrisbobbe commented Dec 14, 2023

chrisbobbe commented Dec 15, 2023 • edited Loading

Home screen banner

Notice in Settings (Notifications button)

Notice on login screen

Notifications screen

Notices

Notifications screen

New setting

chrisbobbe commented Dec 15, 2023 • edited Loading

Google Play Services message changed

Bold realm name in user-facing message

The new "silence" setting:

The troubleshooting screen now links to the troubleshooting article

Alerts in the troubleshooting screen are un-bolded

Linking to Help Center instead of RTD:

Other messages also shown in more places

Other things

chrisbobbe commented Dec 16, 2023 • edited Loading

alya commented Dec 19, 2023

alya commented Dec 19, 2023

chrisbobbe commented Dec 19, 2023

chrisbobbe commented Dec 19, 2023 • edited Loading

gnprice left a comment

Choose a reason for hiding this comment

gnprice Dec 21, 2023

Choose a reason for hiding this comment

gnprice commented Dec 21, 2023

chrisbobbe commented Jan 4, 2024

chrisbobbe commented Dec 15, 2023 •

edited

Loading

chrisbobbe commented Dec 15, 2023 •

edited

Loading

chrisbobbe commented Dec 16, 2023 •

edited

Loading

chrisbobbe commented Dec 19, 2023 •

edited

Loading