Details
Careful code inspection and Flow
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/components/connect-farcaster-alert-handler.react.js | ||
|---|---|---|
| 61 | The purpose of this check was that we introduced the connect Farcaster alert at the same time as the community directory alert. We wanted to avoid showing them at the same time. We can keep the check in case somebody has not opened the app since before we introduced both of those alerts. | |
| 67 | There is a similar risk here for a user who signed up for Comm during our big October 2024 Farcaster launch. They would have likely connected their FID on registration, and perhaps have not opened their app since before the community directory was launched. It would be a frustrating experience to receive both prompts at once, so this check makes sure we show the prompts one-at-a-time. | |
This might be subtly broken:
If a user using this version dismisses the Farcaster connection and then connects from the profile tab, they won't see the DCs modal after the public release. This will happen because in shouldSkipConnectFarcasterAlert the alertInfo.totalAlerts will be greater than 0.
The intention behind D15148 was to work around this: before the public release, users won't see the modal at all, but then they will see either Farcaster or DC modal. This was reasonable because existing users have already connected or dismissed, and new users can connect during the registration flow.
So overall, this diff is a different compromise, and I'm not sure if it is a better one.
I don't follow how that would occur. Did you notice that there are two different alertInfos? If I'm reading the code right, the only way connectFarcasterDCsAlertInfo.totalAlerts will be greater than 0 is if we display the DCs modal.