[lib] introduce getUserAvatarForThread function
ClosedPublic
Actions

Authored by ginsu on Mar 25 2023, 10:50 PM.

Details

Reviewers

ashoat
atul

Commits

rCOMM82a9fdc14701: [lib] introduce getUserAvatarForThread function

Summary

The getUserAvatarForThread function returns the user avatar for threads that are either PERSONAL or PRIVATE. We inject the user avatars in threadInfoFromRawThreadInfo so that it is already set, and when we later call useGetAvatarForThread (introduced in D7132) we will just return that

Depends on D7177

Test Plan

Please see the screenshots below to see that the correct user avatar is being rendered on to a PERSONAL and PRIVATE thread

For reference my friend's avatar:

Screenshot 2023-03-26 at 2.03.53 AM.png (1×1 px, 750 KB)

For reference my avatar:

Screenshot 2023-03-26 at 2.03.55 AM.png (1×1 px, 810 KB)

Personal Threads:

Screenshot 2023-03-26 at 2.05.59 AM.png (1×1 px, 929 KB)

Private Thread:

Screenshot 2023-03-26 at 2.05.44 AM.png (262×704 px, 59 KB)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Mar 25 2023, 10:50 PM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMar 25 2023, 10:50 PM

ginsu mentioned this in D7177: [keyserver/lib/native/web] introduce color-utils and move color functions from thread-utils to color-utils.Mar 25 2023, 10:58 PM

Harbormaster completed remote builds in B17636: Diff 24138.Mar 25 2023, 11:04 PM

ginsu requested review of this revision.Mar 25 2023, 11:04 PM

ginsu edited the summary of this revision. (Show Details)Mar 25 2023, 11:16 PM

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

ginsu added reviewers: ashoat, atul.

ginsu attached a referenced file: F445423: Screenshot 2023-03-26 at 2.05.44 AM.png. (Show Details)

ginsu attached a referenced file: F445424: Screenshot 2023-03-26 at 2.05.59 AM.png. (Show Details)

ginsu attached a referenced file: F445425: Screenshot 2023-03-26 at 2.03.55 AM.png. (Show Details)

ginsu attached a referenced file: F445426: Screenshot 2023-03-26 at 2.03.53 AM.png. (Show Details)

ginsu added inline comments.

lib/shared/avatar-utils.js
68	This function was heavily inspired by how we used to get the `uiName` of a thread before we introduced all the logic for entities. https://github.com/CommE2E/comm/blob/5cfc7a2966c7929872af3fa1ed901413532ba745/lib/shared/thread-utils.js#L695-L717
83	I didn't want to return your own user avatar since that might cause confusion where the user thinks they are on their `PRIVATE` thread, so defaulted to this, but maybe we could also just do a random generated thread avatar too..
lib/shared/thread-utils.js
884	We inject the user avatar here so that it is preset and when we later call `useGetAvatarForThread` (introduced in D7132) we will just return that
lib/types/thread-types.js
303	We needed to include avatar in `ClientDBThreadInfo` to make flow happy

ginsu added a child revision: D7132: [lib] introduce useGetAvatarForThread hook.Mar 25 2023, 11:34 PM

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

ginsu added a parent revision: D7177: [keyserver/lib/native/web] introduce color-utils and move color functions from thread-utils to color-utils.

ginsu mentioned this in D7132: [lib] introduce useGetAvatarForThread hook.Mar 25 2023, 11:41 PM

ginsu mentioned this in D7133: [lib] introduce getAvatarForThreadEntity function.

ashoat accepted this revision.Mar 26 2023, 5:53 AM

ashoat added inline comments.

lib/shared/avatar-utils.js
77	If `getUserAvatarForThread` is only meant to be used for `PRIVATE` and `PERSONAL`, can you add an `invariant` here that `threadInfo.type === threadTypes.PERSONAL`? That way it will be more clear to the reader
83	No this is good. I think this should only happen if somebody has a `PERSONAL` chat with you and then they delete their account. `defaultAnonymousUserEmojiAvatar` is a good default for that

This revision is now accepted and ready to land.Mar 26 2023, 5:53 AM

ginsu added inline comments.Mar 26 2023, 6:35 PM

lib/shared/thread-utils.js
884	This was where we got the circular dependency issue from D7177 when all the color logic was initially located in this file. Again this was because `getUserAvatarForThread` attempted to access `selectedThreadColor` before it got initialized.