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 #5802

Merged
merged 19 commits into from
Jan 4, 2024

Commits on Jan 4, 2024

  1. NotifTroubleshootingScreen: Revise text for Google Play Services warning

    I suppose the Services could be available on the device, just
    disabled or otherwise made unavailable to Zulip.
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    dea9f17 View commit details
    Browse the repository at this point in the history
  2. notifs [nfc]: Add inheritColor, inheritFontSize to some nested ZulipT…

    …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.
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    c50655b View commit details
    Browse the repository at this point in the history
  3. AlertItem: Make text regular weight, not bold

    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.
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    26c9498 View commit details
    Browse the repository at this point in the history
  4. notifs: Revise "server not set up" message; drop condition on Role.Admin

    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 committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    c889055 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    d0587af View commit details
    Browse the repository at this point in the history
  6. notifs [nfc]: Factor out notifProblemsShortText, from …ShortReactText

    We'll use this soon for a caller that needs a LocalizableText and
    can't use a LocalizableReactText.
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    47377c3 View commit details
    Browse the repository at this point in the history
  7. notifs: Show in more places that the org hasn't enabled push notifica…

    …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 committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    b3e0ad5 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    38882d8 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    f2d056b View commit details
    Browse the repository at this point in the history
  10. ServerPushSetupBanner: Change "Remind me later" button text to "Dismiss"

    And also on ServerCompatBanner, since it seems right to be
    consistent between the two banners.
    
    Fixes-partly: zulip#5785
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    7c3c164 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    80c95e3 View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    5a1e5ce View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    6f70dd7 View commit details
    Browse the repository at this point in the history
  14. NotifTroubleshootingScreen: Use rows in a group, instead of ZulipButtons

    We're about to add a link to the notification troubleshooting guide,
    and it seems cleanest to do these as rows instead of ZulipButtons.
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    944fee6 View commit details
    Browse the repository at this point in the history
  15. Configuration menu
    Copy the full SHA
    ebfb850 View commit details
    Browse the repository at this point in the history
  16. redux types: Remove DismissServerPushSetupNoticeAction from AllAccoun…

    …tsAction
    
    This doesn't belong here. It's already present in the
    PerAccountAction union, so nothing to do after removing it here.
    chrisbobbe committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    ec77fa9 View commit details
    Browse the repository at this point in the history
  17. Configuration menu
    Copy the full SHA
    40c8b50 View commit details
    Browse the repository at this point in the history
  18. Configuration menu
    Copy the full SHA
    2d7e865 View commit details
    Browse the repository at this point in the history
  19. Configuration menu
    Copy the full SHA
    0dbcf8f View commit details
    Browse the repository at this point in the history