Page MenuHomePhabricator

[web] intorduce loading logic to relationship prompt button
ClosedPublic

Authored by ginsu on Jun 27 2024, 12:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 11, 8:13 AM
Unknown Object (File)
Mon, Nov 11, 3:56 AM
Unknown Object (File)
Mon, Nov 11, 3:38 AM
Unknown Object (File)
Mon, Nov 11, 2:55 AM
Unknown Object (File)
Sun, Nov 10, 7:37 PM
Unknown Object (File)
Sun, Nov 10, 2:38 PM
Unknown Object (File)
Fri, Nov 8, 10:38 PM
Unknown Object (File)
Fri, Nov 8, 7:14 AM
Subscribers

Details

Summary

This diff updates the relationship prompt button in web to consume the loading logic from useRelationshipPrompt

Depends on D12593

Test Plan

Confirmed that the loading logic works as expected in the relationship prompt button for both the relationship prompt and user profiles

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu added inline comments.
web/chat/relationship-prompt/relationship-prompt-button.js
16 ↗(On Diff #41767)

Made isLoading required so we can be more proactive in the future about always including loading state in our action buttons

web/chat/relationship-prompt/relationship-prompt.js
51 ↗(On Diff #41767)

Having every word be capitalized is a bit of an anti-pattern, went ahead and updated the copy to follow the convention of only the first word being capitalized but happy to revert this if this is unwanted

cc @ashoat

web/modals/user-profile/user-profile-action-buttons.react.js
49 ↗(On Diff #41767)

Can't really remember why we initially dropped the "friend request" in the copy but added it back to be consistent with what is found in RelationshipPrompt component. If this change is okay, will update this diff to dedup the copy-pasted strings before landing

cc @ashoat

ginsu requested review of this revision.Jun 27 2024, 1:07 PM
ashoat added inline comments.
web/chat/relationship-prompt/relationship-prompt.js
51 ↗(On Diff #41767)

Thank you for catching this!!

web/modals/user-profile/user-profile-action-buttons.react.js
49 ↗(On Diff #41767)

Thanks for making this change for consistency, and yes would be great to dedup!

Can you share a screenshot so we can make sure that the increased text doesn't overflow its bounds?

This revision is now accepted and ready to land.Jul 1 2024, 1:13 PM
web/modals/user-profile/user-profile-action-buttons.react.js
49 ↗(On Diff #41767)

Also want to make sure that the text doesn't look too scrunched up (I feel this way about D12596)

web/modals/user-profile/user-profile-action-buttons.react.js
49 ↗(On Diff #41767)

Screenshot 2024-07-02 at 11.18.05 PM.png (1×3 px, 885 KB)

The text isn't overflowing it's bounds and I feel like it isn't too scrunched up; however, based on feedback in D12596, I think we should drop the "friend request" so that the copy in the user profiles will be consistent between web and native

web/modals/user-profile/user-profile-action-buttons.react.js
49 ↗(On Diff #41767)

Yeah, good call on consistency – let's do it

This revision was landed with ongoing or failed builds.Jul 3 2024, 3:13 PM
This revision was automatically updated to reflect the committed changes.