Page MenuHomePhabricator

[web] introduce ThreadAvatar component
ClosedPublic

Authored by ginsu on Apr 2 2023, 11:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 10:29 PM
Unknown Object (File)
Sat, Apr 27, 10:28 PM
Unknown Object (File)
Sat, Apr 27, 10:28 PM
Unknown Object (File)
Sat, Apr 27, 10:26 PM
Unknown Object (File)
Sat, Apr 27, 9:58 PM
Unknown Object (File)
Fri, Apr 19, 7:03 AM
Unknown Object (File)
Mar 28 2024, 7:59 AM
Unknown Object (File)
Mar 28 2024, 7:59 AM
Subscribers

Details

Summary

After some discussion with @ashoat and @atul about how to fetch the ens avatars we decided to have two separate components to render avatars: UserAvatar and ThreadAvatar. This diff handles ThreadAvatar.

This diff is pretty much a copy of D7262

Depends on D7273

Test Plan

I was able to render all three avatar types: Emoji, Image, and ENS:

Emoji:

Screenshot 2023-04-03 at 2.25.49 AM.png (2×3 px, 939 KB)

Image (by hardcoding const avatarInfo = { type: 'image', uri: 'https://picsum.photos/200' }):

Screenshot 2023-04-03 at 2.34.26 AM.png (2×3 px, 951 KB)

ENS (by hardcoding const avatarInfo = { type: 'ens' } and the same ensAvatarURI I tested in D7262):

Screenshot 2023-04-03 at 2.26.01 AM.png (2×3 px, 942 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Apr 2 2023, 11:29 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: ashoat, atul.
ginsu edited the summary of this revision. (Show Details)

Thanks for testing those scenarios in Test Plan. Question about API inline.

web/components/thread-avatar.react.js
27 ↗(On Diff #24532)

Are we not going to run into the same issue as with UserAvatar of having to make sure we're drilling threadInfo everywhere (I don't know, haven't taken a close look)? Should the prop just be threadID to match what we're doing for UserAvatars?

This revision is now accepted and ready to land.Apr 3 2023, 10:41 AM
web/components/thread-avatar.react.js
27 ↗(On Diff #24532)

@ginsu initially did threadID, but I requested threadInfo after noticing all callsites already had threadInfo. So no extra threadInfo fetching/drilling was necessary, and some threadInfo fetching was saved

web/components/thread-avatar.react.js
27 ↗(On Diff #24532)

Gotcha makes sense

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

rebase before landing

This revision was automatically updated to reflect the committed changes.