with this change, the connect farcaster prompt appears after all the nux tips have been dismissed, instead of simultaneously
Depends on D14446
Details
Details
Scenarios tested:
- Existing user logging in for first time on new iPhone with latest app version
- NUX is displayed on login
- on next cold start, connect farcaster prompt is displayed
- on subsequent cold start, directory prompt is displayed
- Existing logged-in user after upgrading to latest app version
- cold started the app a bunch of times
- migration runs, clears the alertStore-level coldStartCount
- on next cold start, connect farcaster prompt is displayed
- on subsequent cold start, directory prompt is displayed
- New registration on latest app version
- NUX is displayed when registration completes
- connect farcaster prompt not displayed (user already declined to connect farcaster account during registration)
- on next cold start (cold start count = 2) nothing happens
- on subsequent cold start, directory prompt is displayed (user declined to connect farcaster account during registration. if the user had connected a farcaster account, this prompt would not appear)
- Web migration
- migration succeeds, the alertStore-level coldStartCount is removed
NOTE: directory prompt will not be displayed if user connects farcaster account. this is intentional -- we want to display a different prompt to farcaster users later this month when we have a third tab in the directory modal for foregrounding chats
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
- use coldStartCount to make sure we don't display connect farcaster alert right after NUX
- use connectFarcasterAlertInfo in DisplayCommunityDirectoryPromptHandler to make sure we don't display community directory prompt until a day after connect farcaster prompt
Comment Actions
Your test plan is super incomplete. Can you please update the test plan to go one-by-one through every scenario we've had to contemplate while working on this? Eg. fresh registration, log in of existing user with Farcaster connection, log in of existing user with no Farcaster connection, existing log-in that hasn't logged out since before the new registration flow (never got prompted to connect Farcaster), etc.
| native/components/display-community-directory-prompt.react.js | ||
|---|---|---|
| 64 | Won't this check make it so users that register and get fid set to something other than undefined (and consequently never get the connect Farcaster prompt) will never see the community directory? | |
Comment Actions
make cold start count a per-alert type count instead of a global count, migrations, stagger the connect farcaster and community directory prompts
| native/components/connect-farcaster-alert-handler.react.js | ||
|---|---|---|
| 41 ↗ | (On Diff #47667) | i considered adding another check to ensure that the current route is HomeChatThreadListRouteName like we do in display-community-directory-prompt.react.js, but decided it would just overcomplicate things.
|
| native/components/connect-farcaster-alert-handler.react.js | ||
|---|---|---|
| 41 ↗ | (On Diff #47667) | the scenario described in 2 could only happen if the user opens the app from a notif (cold start count = 2), kills the app, opens from a notif again (cold start count = 3), and navigates to the home screen. then the connect farcaster and community directory prompts will appear back to back. we could also increase the coldStartCount threshold for the DisplayCommunityDirectoryPrompt from 3 to 5 or 6 to reduce the likelihood that both prompts appear on the same cold start |
| native/components/connect-farcaster-alert-handler.react.js | ||
|---|---|---|
| 41 ↗ | (On Diff #47667) |
Are you saying this scenario in the app could occur as-is, or hypothetically if we added a HomeChatThreadListRouteName check here? If it's the latter, I think we can probably just keep the code as-is for the reasons you described in the first comment. |
| native/redux/persist.js | ||
| 1526 ↗ | (On Diff #47667) | Shouldn't we be setting coldStartCount inside of alertStore here? I can see the ?? 0 in the reducer, but you're still doing eg. undefined < 3 comparisons (which would return false, instead of true for 0 < 3) |
Comment Actions
update migrations
| native/components/connect-farcaster-alert-handler.react.js | ||
|---|---|---|
| 41 ↗ | (On Diff #47667) | the latter |
| native/redux/persist.js | ||
| 1526 ↗ | (On Diff #47667) | yeah that's a good point. the ColdStartTracker renders after login but before our prompt handlers so we're not encountering the undefined < 3 comparison, but it's definitely a little sketchy. i've updated the migrations for web and native |
| web/redux/persist.js | ||
|---|---|---|
| 735 ↗ | (On Diff #47668) | I didn't test this again after the most recent update to this diff, but it's identical to the native migration, which i did test, so I think this change is safe |