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

Move request for notifications permissions to HomeReadyScreen #3977

Merged
merged 15 commits into from
May 13, 2024

Conversation

haileyok
Copy link
Contributor

@haileyok haileyok commented May 12, 2024

Why

Asking for permissions randomly at the start of onboarding can be pretty jarring and might result in more denials of permission. Instead, we can ask for permission after onboarding is complete.

We also want to be able to track this, so I have:

  1. Added a feature gate on whether to show this at the start of onboarding or afterward
  • Display it on the interests screen of onboarding if the reduced onboarding gate is false
  • Display it on the profile screen of onboarding if the reduced onboarding gate is true
  1. Added a new log type - notifications:request
  • Accepts a status with values granted, denied, or undetermined - the default values for Expo Notifications

On top of these changes, I am fixing/cleaning up some other logic. On main right now, we can see that we are registering the push token multiple times on app launch:

Screenshot 2024-05-11 at 11 11 46 PM

As noted below, that is because we are

  1. Getting the device push token and registering it
  2. Creating a listener that calls register token whenever the token changes. Whenever it changes, we register the new token (the one we just retrieved in step one!)

Instead, what I have done is call getDevicePushToken() on DID change without registering it, then let the change listener handle the registration. This is the result:

Screenshot 2024-05-11 at 11 14 54 PM

Test Plan

Different gate configurations should be tested:

Reduced Onboarding: False, Request After Onboarding: False

RocketSim_Recording_iPhone_15_Pro_6.1_2024-05-11_22.50.05.mp4

Reduced Onboarding: True, Request After Onboarding: False

RocketSim_Recording_iPhone_15_Pro_6.1_2024-05-11_22.56.02.mp4

Reduced Onboarding: True/False, Request After Onboarding: True

RocketSim_Recording_iPhone_15_Pro_6.1_2024-05-11_22.58.20.mp4

Copy link

render bot commented May 12, 2024

Copy link

github-actions bot commented May 12, 2024

Old size New size Diff
7.14 MB 7.15 MB 14.79 KB (0.20%)

@haileyok haileyok requested a review from gaearon May 12, 2024 06:20
Copy link
Member

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@haileyok haileyok merged commit d3406c8 into main May 13, 2024
6 checks passed
@haileyok haileyok deleted the hailey/move-push-notification-request branch May 13, 2024 16:19
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.

3 participants