Page MenuHomePhabricator
Differential D13160

[lib] implement sending DM text messages
ClosedPublic
Actions

Authored by kamil on Aug 23 2024, 7:01 AM.
Tags
None
Referenced Files
F3367037: D13160.diff
Mon, Nov 25, 1:30 PM
Unknown Object (File)
Fri, Nov 22, 2:14 PM
Unknown Object (File)
Fri, Nov 8, 11:06 PM
Unknown Object (File)
Fri, Nov 8, 2:09 AM
Unknown Object (File)
Fri, Nov 8, 2:09 AM
Unknown Object (File)
Fri, Nov 8, 1:42 AM
Unknown Object (File)
Oct 25 2024, 2:37 AM
Unknown Object (File)
Oct 18 2024, 8:57 PM
View All 44 Files
Subscribers
ashoat

Details

Reviewers
tomek
inka
Commits
rCOMMb292958afbf5: [lib] implement sending DM text messages
Summary

ENG-8423.

Depends on D13159

Test Plan

Test sending text message

Note: This stack is tested end-to-end according to the process described here.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Aug 23 2024, 7:01 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptAug 23 2024, 7:01 AM
kamil edited the summary of this revision. (Show Details)Aug 23 2024, 7:02 AM
kamil added a parent revision: D13159: [web][native] implement DM messages status visible in UI.
kamil removed a parent revision: D13156: [lib] implement sending DM messages with tracking of success/failures.
Harbormaster failed remote builds in B31244: Diff 43617!Aug 23 2024, 7:05 AM
kamil added a child revision: D13161: [lib] implement retrying DM text messages.Aug 23 2024, 7:18 AM
Harbormaster completed remote builds in B31244: Diff 43617.Aug 23 2024, 7:19 AM
kamil published this revision for review.Aug 23 2024, 7:28 AM
kamil added inline comments.
native/input/input-state-container.react.js
486

wondering, if we should use something like this here

tomek accepted this revision.Aug 26 2024, 4:38 AM
tomek added inline comments.
native/input/input-state-container.react.js
486

It is an interesting idea, but we would need to make sure that the local IDs are UUIDs. But even then, I'm not sure if it would really be beneficial. On the other hand, it is a bit confusing why the messageInfo has an ID that isn't used here.

491–492

Shouldn't we send this message to all the users? We should inform other devices of the viewer about this operation.

This revision is now accepted and ready to land.Aug 26 2024, 4:38 AM
kamil updated this revision to Diff 43643.Aug 26 2024, 7:42 AM

send message to other sender devices

kamil added inline comments.Aug 26 2024, 7:44 AM
native/input/input-state-container.react.js
486

I'll leave it as it is to not interfere with how our UUIDs work

491–492

not to all users, but we should not remove the sender from recipientIDs

Harbormaster completed remote builds in B31268: Diff 43643.Aug 26 2024, 7:59 AM
Closed by commit rCOMMb292958afbf5: [lib] implement sending DM text messages (authored by kamil). · Explain WhyAug 26 2024, 9:05 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMb292958afbf5: [lib] implement sending DM text messages.

Revision Contents
Changeset List

PathSize
native/
input/
41 lines
web/
input/
41 lines

Diff 43617

View Options

native/input/input-state-container.react.js

Loading...
View Options

web/input/input-state-container.react.js

Loading...