[native] DisplayCommunityDirectoryPrompt
Needs RevisionPublic
Actions

Authored by varun on Nov 26 2024, 10:14 PM.

Details

Reviewers

Summary

the second time a logged in user foregrounds the app, they will see the bottom sheet asking if they want to check out the community directory. if the user is not on the home screen, they won't get the prompt

Depends on D13997

Test Plan

confirmed that prompt only displays once per installation. it doesn't display if user is not logged in or not on the home screen

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Nov 26 2024, 10:14 PM

varun held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptNov 26 2024, 10:14 PM

varun added a child revision: D14054: [native] navigate to community directory nux tip if user declines prompt or closes bottom sheet.Nov 26 2024, 10:19 PM

varun added inline comments.Nov 26 2024, 10:20 PM

native/components/display-community-directory-prompt.react.js
53	an undefined fid means the user hasn't been prompted to set a farcaster association yet

varun added inline comments.Nov 26 2024, 10:26 PM

native/components/display-community-directory-prompt.react.js
66	as @ashoat noted on a previous diff in this stack, this isn't a promise, but rather a function that returns a promise. i should revise this before publishing to avoid calling the keyserver endpoint multiple times

Harbormaster completed remote builds in B32879: Diff 46094.Nov 26 2024, 10:32 PM

fix up promises and use useCurrentLeafRouteName hook

varun published this revision for review.Tue, Feb 11, 8:15 PM

varun edited the summary of this revision. (Show Details)

varun edited the test plan for this revision. (Show Details)

This comment was removed by varun.

Harbormaster completed remote builds in B33476: Diff 47013.Tue, Feb 11, 8:27 PM

ashoat requested changes to this revision.Tue, Feb 11, 9:10 PM

ashoat added inline comments.

native/components/display-community-directory-prompt.react.js
17 ↗	(On Diff #47013)	Can you prioritize moving this constant, instead of adding more references to it? See comment here. You created a follow-up task for this four weeks ago, but this is the kind of thing that should be trivial and probably doesn't even need a Linear task. Should take 5 minutes to put up a diff
51–54 ↗	(On Diff #47013)	Why do we need this? Doesn't `useOnFirstLaunchEffect` already guarantee that the effect will only fire once?
61 ↗	(On Diff #47013)	Does this include `GENESIS`? If so, let's bump the check to 4
62 ↗	(On Diff #47013)	Hmm... this approach risks the prompt never being displayed, as it relies on the user foregrounding the app shortly after backgrounding it. Why did you opt for this approach instead of the one from the task description that references uses `connectFarcasterAlertInfo` (persisted Redux state)? This would allow us to show the prompt on the second time the app is cold started, which is more reliable. The whole goal here is simply to avoid showing the prompt at the same time as the NUX.