Page MenuHomePhabricator
Differential D6122

[native] add starting payload to send reaction message dispatch action promise
ClosedPublic
Actions

Authored by ginsu on Dec 30 2022, 4:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 26, 5:24 PM
Unknown Object (File)
Tue, Nov 26, 11:55 AM
Unknown Object (File)
Oct 28 2024, 8:21 AM
Unknown Object (File)
Oct 7 2024, 6:53 PM
Unknown Object (File)
Sep 28 2024, 11:29 AM
Unknown Object (File)
Sep 28 2024, 11:29 AM
Unknown Object (File)
Sep 28 2024, 11:29 AM
Unknown Object (File)
Sep 28 2024, 11:29 AM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
atul
tomek
Commits
rCOMMbcca925e1d92: [native] add starting payload to send reaction message dispatch action promise
Summary

add starting payload to send reaction message dispatch action promise. This starting payload is the optimistically created reaction message.

Depends on D6121
Linear Task: ENG-2520

Test Plan

flow and ths will be further tested in a subsequent diff

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Dec 30 2022, 4:10 PM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 30 2022, 4:10 PM
ginsu edited the summary of this revision. (Show Details)Dec 30 2022, 4:11 PM
ginsu added reviewers: atul, tomek.
ginsu added a parent revision: D6120: [native] introduce localID field to onPressReact function.
ginsu edited the summary of this revision. (Show Details)
ginsu added a parent revision: D6121: [web] add starting payload to send reaction message dispatch action promise.
Harbormaster completed remote builds in B14991: Diff 20460.Dec 30 2022, 4:24 PM
ginsu requested review of this revision.Dec 30 2022, 4:24 PM
tomek requested changes to this revision.Jan 2 2023, 6:08 AM
tomek added inline comments.
native/chat/reaction-message-utils.js
31–41

Can we modify the type of viewerID so that null isn't allowed and invariant isn't necessary?

101–118

Is there a way to share more code between the platforms? In the previous diff we've done something similar for web.

This revision now requires changes to proceed.Jan 2 2023, 6:08 AM
ginsu added a child revision: D6123: [lib] add sendReactionMessageAction.started cases to message and local id reducers.Jan 3 2023, 10:27 AM
ginsu updated this revision to Diff 20546.Jan 3 2023, 2:34 PM

address feedback

Harbormaster completed remote builds in B15060: Diff 20546.Jan 3 2023, 2:48 PM
ginsu added inline comments.Jan 3 2023, 2:58 PM
native/chat/reaction-message-utils.js
31–41

The TooltipEntry onPress type in the master branch has viewerID set to maybe, I think this is something we can do when we work on ENG-2626

101–118

We can create a helper function for just creating the starting payload, but since native is using dispatchFunctions and bindServerCall and web are using the hook equivalents with useServerCall() and useDispatchActionPromise(), sharing that part of code is going to be tricky

tomek accepted this revision.Jan 5 2023, 4:40 AM
tomek added inline comments.
native/chat/reaction-message-utils.js
101–118

Ok, makes sense

This revision is now accepted and ready to land.Jan 5 2023, 4:40 AM
ginsu updated this revision to Diff 20717.Jan 9 2023, 9:41 AM

rebase before landing

Harbormaster completed remote builds in B15202: Diff 20717.Jan 9 2023, 9:56 AM
Closed by commit rCOMMbcca925e1d92: [native] add starting payload to send reaction message dispatch action promise (authored by ginsu). · Explain WhyJan 9 2023, 10:05 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMbcca925e1d92: [native] add starting payload to send reaction message dispatch action promise.

Revision Contents
Changeset List

PathSize
native/
chat/
18 lines

Diff 20460

View Options

native/chat/reaction-message-utils.js

Loading...