Details

Reviewers

ashoat
inka

Commits

rCOMM8e487bb3cb67: [lib] introduce useJoinCommunity

Summary

Move diff. This diff introduces useJoinCommunity. useJoinCommunity factors out the shared logic between joining a community through accepting an invite link and joining a community through the auto join community handler. Subsequent diffs will handle making useJoinCommunity more agnostic so that the hook can be used at both callsites

Depends on D12702

Test Plan

flow + confirmed that there were no regressions with the joining a community through accepitng an invite link user flow

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Jul 9 2024, 9:49 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 9 2024, 9:49 AM

ginsu edited the summary of this revision. (Show Details)Jul 9 2024, 9:51 AM

ginsu added reviewers: ashoat, inka.

ginsu added a parent revision: D12702: [keyserver] include keyserver url in the farcaster channel tag blob.

ginsu added inline comments.Jul 9 2024, 9:54 AM

lib/shared/community-utils.js
281 ↗	(On Diff #42147)	The change here is that we set `threadID` to the `threadID` hook param rather than `verificationResponse.thread?.id`
334 ↗	(On Diff #42147)	The changes here are because we already declared `threadID` in the upper scope as a hook param, so we needed a new variable name here

Harbormaster completed remote builds in B30226: Diff 42147.Jul 9 2024, 10:05 AM

ginsu requested review of this revision.Jul 9 2024, 10:05 AM

Didn't review the code closely since it's marked as a move diff

I think it would be good to lift the state back into the component

lib/hooks/invite-links.js
187 ↗	(On Diff #42147)	Rather than having `useJoinCommunity` create the state and pass it back out, I think it would be more readable for this component to create the state and then pass it in In general, it's best to have state "lifted" to the shared common ancestor