Details

Reviewers

inka
ashoat

Summary

This diff introduces the loading logic to RelationshipPrompt component

Depends on D12596

Test Plan

Confirmed that the relationship prompt buttons in the relationship prompt show a loading spinner when the action is in progress

Not loading:

Screenshot 2024-07-03 at 12.33.34 AM.png (1×944 px, 670 KB)

Screenshot 2024-07-03 at 12.33.49 AM.png (1×944 px, 667 KB)

Loading:

Screenshot 2024-07-03 at 12.33.37 AM.png (1×944 px, 666 KB)

Screenshot 2024-07-03 at 12.33.54 AM.png (1×944 px, 658 KB)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Thu, Jun 27, 2:08 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptThu, Jun 27, 2:08 PM

update

ginsu edited the summary of this revision. (Show Details)Thu, Jun 27, 2:12 PM

ginsu added reviewers: inka, ashoat.

ginsu added a parent revision: D12596: [native] introduce loading logic to relationship prompt button.

update

native/chat/relationship-prompt.react.js
169	Hardcoded the height here so that we guarantee that when the button state goes to loading then the height of the button never changes; however, recently got some feedback about hardcoding the height in CSS being a no-no. Looks like `visibility: hidden` is not supported in react native, but if we still don't want this, I can explore other ways of doing this This height was determined by using the element inspector in my simulator

forgot to hit save

native/chat/relationship-prompt.react.js
169 ↗	(On Diff #41777)	Hardcoded the height here so that we guarantee that when the button state goes to loading then the height of the button never changes; however, recently got some feedback about hardcoding the height in CSS being a no-no. Looks like `visibility: hidden` is not supported in react native, but if we still don't want this, I can explore other ways of doing this This height was determined by using the element inspector in my simulator

Harbormaster completed remote builds in B29975: Diff 41774.Thu, Jun 27, 2:26 PM

Harbormaster completed remote builds in B29974: Diff 41773.Thu, Jun 27, 2:29 PM

Harbormaster completed remote builds in B29977: Diff 41776.Thu, Jun 27, 2:34 PM

Harbormaster completed remote builds in B29978: Diff 41777.Thu, Jun 27, 2:38 PM

ginsu requested review of this revision.Thu, Jun 27, 2:38 PM

Passing back for the same reasons as D12596. Using RelationshipButton everywhere seems like a good idea, but we should be thoughtful about the complexity if we end up having to render different things in different surfaces

This revision now requires changes to proceed.Mon, Jul 1, 1:22 PM

address comments

ginsu retitled this revision from [native] use RelationshipButton in RelationshipPrompt to [native] introduce loading logic to RelationshipPrompt.Tue, Jul 2, 9:38 PM

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

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

Using RelationshipButton everywhere seems like a good idea, but we should be thoughtful about the complexity if we end up having to render different things in different surfaces

I went ahead and abandoned this idea since the "Accept friend request" + "Reject friend request" text was too scrunched up in the button and in general it felt like this idea was creating a lot more added complexity

ginsu attached a referenced file: F2173156: Screenshot 2024-07-03 at 12.33.54 AM.png. (Show Details)Tue, Jul 2, 9:42 PM

ginsu attached a referenced file: F2173155: Screenshot 2024-07-03 at 12.33.37 AM.png. (Show Details)

ginsu attached a referenced file: F2173153: Screenshot 2024-07-03 at 12.33.49 AM.png. (Show Details)

ginsu attached a referenced file: F2173150: Screenshot 2024-07-03 at 12.33.34 AM.png. (Show Details)