we can fetch the farcaster avatar URL once the user has connected a farcaster account and then pass it through the registration wizard so that it's available on the avatar selection screen
Depends on D13099
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
When we last talked about this work, we were considering two options on how to proceed. It looks like you've taken the second approach – can you explain a bit about how you derisked this part?
To do this, we'd need to form a native media selection which might involve first saving the file to the file system
Curious if you've tested on both iOS and Android. Requesting changes to get an answer to the above (although might be best to document on Linear?)
| native/account/registration/connect-farcaster.react.js | ||
|---|---|---|
| 136 | What's the significance of pfpURL? getFCAvatarURLs appears to be introduced in the prior diff, but it's still in draft form | |
| 150 | Why do we use null here but undefined in setCachedSelections below? | |
| native/account/registration/connect-farcaster.react.js | ||
|---|---|---|
| 136 | pfpURL is the URL to the farcaster avatar provided by neynar | |
| 150 | i used undefined below because i had typed farcasterAvatarURL as +farcasterAvatarURL?: string, in CachedUserSelections (further below in diff). i can't think of a good reason to not change it to ?string and use null below, so will fix | |
Comment Actions
https://linear.app/comm/issue/ENG-9182/allow-users-to-set-farcaster-avatar-on-native we can discuss further here. i've opted for option 1
| native/account/registration/connect-farcaster.react.js | ||
|---|---|---|
| 136 | What's the significance of this parameter to getFCAvatarURLs? Why is it null here? When should it be null and when shouldn't it be? | |
Comment Actions
Okay, sure. When you've finished responding to my last review (whether on Linear or here), feel free to re-request review. Make sure you respond to all parts
| native/account/registration/connect-farcaster.react.js | ||
|---|---|---|
| 136 | export type BaseFCInfo = {
+fid?: ?string,
+farcasterUsername?: ?string,
+pfpURL?: ?string,
...
};
export type GetFCNames = <T: ?BaseFCInfo>(
users: $ReadOnlyArray<T>,
) => Promise<T[]>;
export type GetFCAvatarURLs = <T: ?BaseFCInfo>(
users: $ReadOnlyArray<T>,
) => Promise<T[]>;
const getFCAvatarURLs: GetFCAvatarURLs = <T: ?BaseFCInfo>(
users: $ReadOnlyArray<T>,
): Promise<T[]> => baseGetFCAvatarURLs(fcCache, users);
return {
client: neynarClient,
fcCache,
getFCNames,
getFCAvatarURLs,
};baseGetFCAvatarURLs expects an input param of type $ReadOnlyArray<?BaseFCInfo>, and we have to provide pfpURL if we want to access the farcaster avatar URLs returned by that function. pfpURL should always be null when included, because it will be set by getFCAvatarURLs. maybe a code comment would be helpful here | |
Comment Actions
Explanation for pfpURL: null doesn't make sense to me. Can you remove it, or explain exactly why you need it? It feels strange to be passing in pfpURL to a function that returns it, and it feels double strange to be setting it to null.
| native/account/registration/connect-farcaster.react.js | ||
|---|---|---|
| 135 ↗ | (On Diff #44308) | I don't follow why you need pfpURL: null here. In D13099, you define BaseFCInfo as such: export type BaseFCInfo = { +fid?: ?string, +farcasterUsername?: ?string, +pfpURL?: ?string, ... }; Doesn't that mean pfpURL can be skipped? |