Details

Reviewers

atul
inka

Commits

rCOMM9761ffcc760f: [lib/native] introduce UserProfileMessageButton

Summary

This diff introduces the UserProfileMessageButton component that will be rendered on a user profile. This button will be used to navigate to a thread between the viewer and the user of the profile

Figma designs (left is when there in no block in either direction and right is when there is a block in either direction):

Screenshot 2023-09-22 at 4.34.33 PM.png (1×1 px, 277 KB)

Depends on D9247

Test Plan

Please see the demo video below

anonymous user:

Screenshot 2023-09-20 at 4.49.07 PM.png (1×1 px, 750 KB)

block in either direction is active:

Screenshot 2023-09-22 at 4.15.11 PM.png (1×1 px, 732 KB)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Sep 20 2023, 1:21 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 20 2023, 1:21 PM

ginsu edited the summary of this revision. (Show Details)Sep 20 2023, 1:22 PM

ginsu added a parent revision: D9247: [lib] introduce useUserProfileThreadInfo hook.

ginsu added a subscriber: kamil.Sep 20 2023, 1:45 PM

ginsu added inline comments.

native/components/user-profile-message-button.react.js
16–17 ↗	(On Diff #31326)	I broke down the prop types down instead of using `PendingPersonalThreadUserInfo` here. This was some feedback I got from @kamil in an older diff. I also personally think it makes it more readable and super clear to any dev that this is exactly what is needed for this component, but happy to change this if others think otherwise
native/components/user-profile.react.js
72–74 ↗	(On Diff #31326)	This is for the case if we view an anonymous profile

ginsu edited the test plan for this revision. (Show Details)Sep 20 2023, 1:49 PM

ginsu added reviewers: atul, inka.

ginsu attached a referenced file: F768057: Screenshot 2023-09-20 at 4.49.07 PM.png. (Show Details)

ginsu attached a referenced file: F768056: Screen Recording 2023-09-20 at 2.32.13 PM.mov. (Show Details)

Harbormaster completed remote builds in B22768: Diff 31326.Sep 20 2023, 2:02 PM

ginsu requested review of this revision.Sep 20 2023, 2:02 PM

ginsu added a child revision: D9258: [native] introduce UserProfileRelationshipButton.Sep 21 2023, 11:12 AM

we should also not show message button if the user and/or viewer is blocked

ginsu added inline comments.Sep 22 2023, 1:14 PM

native/components/user-profile-message-button.react.js
15–18	I broke down the prop types down instead of using `PendingPersonalThreadUserInfo` here. This was some feedback I got from @kamil in an older diff. I also personally think it makes it more readable and super clear to any dev that this is exactly what is needed for this component, but happy to change this if others think otherwise
native/components/user-profile.react.js
78–80	This is for the case if we view an anonymous profile or there is block active in either direction

ginsu edited the test plan for this revision. (Show Details)Sep 22 2023, 1:15 PM

ginsu attached a referenced file: F770444: Screenshot 2023-09-22 at 4.15.11 PM.png. (Show Details)

Harbormaster completed remote builds in B22807: Diff 31377.Sep 22 2023, 1:29 PM

ginsu edited the summary of this revision. (Show Details)Sep 22 2023, 1:35 PM

ginsu attached a referenced file: F770513: Screenshot 2023-09-22 at 4.34.33 PM.png. (Show Details)

Let's use existing relationshipBlockedInEitherDirection

native/components/user-profile-message-button.react.js
30	Can we give this a more descriptive name? Like const { dismiss: dismissBottomSheetModal } = useBottomSheetModal();
36–42	We could wrap this in `useMemo` to short-circuit re-renders, but benefit is probably marginal at best and this is more readable... so probably makes sense to leave as is
native/components/user-profile.react.js
26–29	Looking at this, had a strong hunch that this logic wasn't unique to `UserProfile` Looking at `relationships-utils`, we seem to have a `relationshipBlockedInEitherDirection`: which seems like what we want to use here?
92–102	Could wrap this higher-level component w/ `useMemo` to short circuit re-renders if you think that makes sense

This revision now requires changes to proceed.Sep 26 2023, 11:45 AM

atul attached a referenced file: F774781: 1075ef.png. (Show Details)Sep 26 2023, 11:45 AM

address @atul's comments

ginsu added inline comments.Sep 27 2023, 11:40 AM

native/components/user-profile.react.js
92–102	Left this as is, since the benefit is probably marginal at best and this is more readable

ginsu added inline comments.Sep 27 2023, 11:48 AM

lib/shared/relationship-utils.js
49 ↗	(On Diff #31461)	Made this optional so that we can use this function even if `userInfo` is potentially null/undefined. This should not change the logic, since a user should not have a block on a null (anonymous) user or a null user can't have a block on the viewer and will just still return false

ginsu retitled this revision from [native] introduce UserProfileMessageButton to [lib/native] introduce UserProfileMessageButton.Sep 27 2023, 11:48 AM

Harbormaster completed remote builds in B22874: Diff 31461.Sep 27 2023, 11:56 AM

Thanks for addressing feedback!

This revision is now accepted and ready to land.Sep 28 2023, 5:30 PM

rebase before landing

Harbormaster completed remote builds in B22918: Diff 31552.Sep 29 2023, 12:02 PM

Closed by commit rCOMM9761ffcc760f: [lib/native] introduce UserProfileMessageButton (authored by ginsu). · Explain WhySep 29 2023, 12:10 PM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM9761ffcc760f: [lib/native] introduce UserProfileMessageButton.