Details
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/components/edit-avatar.react.js | ||
|---|---|---|
| 29–30 | One potential alternative would be to explicitly limit children to being React.ComponentType<typeof UserAvatar> | React.ComponentType<typeof ThreadAvatar> here. (Alternately, React.ComponentType<typeof UserAvatar | typeof ThreadAvatar> might work... not sure.) Then you could introspect on the children you were passed at runtime by eg. checking children.name, which would obviate the need for childAvatarType. Not sure how easily this will work or if it's worth it to do now. | |
| 42–60 | It feels like this component has two sets of functionality now – saving thread avatars and saving user avatars. How much code do the two workflows really share? If there's a substantial amount of shared code, is there a better way to share that code than to have them in the same component? I suspect we probably don't need two separate components here, but could be wrong. Maybe we can discuss when you get into the office today? | |
| native/components/edit-avatar.react.js | ||
|---|---|---|
| 29–30 | Great idea, tinkered with it locally and it is not difficult at all, will have an update for this out here shortly | |
| 42–60 |
Up this point in the stack the two workflows don't have a ton to share outside of the action sheet and edit badge. However, the plan is to also have callbacks for onPressAvatarPhotoGalleryFlow and onPressAvatarCameraFlow in this component as well, and those will be directly used and shared by both workflows. When these callbacks get introduced I was also planning on introducing the necessary loaders for thread avatars like I did for user avatars in this diff. I just didn't do it yet because you can't set a thread avatar to an ENS avatar type. Just like the EmojiAvatarCreation component I thought it would be the best design to have EditAvatar be a general purpose component for both user and thread avatars, but having it general purpose would mean being able to save a user and thread avatar. Hopefully this gives you more context into my thinking, but happy to discuss more IRL or here if you think there is still a better way to design this. | |
Talked with this offline with @ginsu and we concluded that EditAvatar is being overloaded and doing two things instead of one. We concluded we're going to split this component and EmojiAvatarCreation into two components