Page MenuHomePhabricator

[web] introduce message like button to message tooltip
ClosedPublic

Authored by ginsu on Dec 27 2022, 7:30 AM.
Tags
None
Referenced Files
F3374660: D6057.diff
Tue, Nov 26, 4:37 PM
Unknown Object (File)
Wed, Nov 13, 10:00 PM
Unknown Object (File)
Wed, Nov 13, 5:28 AM
Unknown Object (File)
Wed, Nov 13, 5:27 AM
Unknown Object (File)
Wed, Nov 13, 5:27 AM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Subscribers

Details

Summary

introduce the message like button to message tooltips. Will be syncing with @tomek about landing these changes since he is also working in tooltip-utils for the copy button on the tooltip. I handled the merge conflicts and rebased the copy button code in with the react button code in this diff.


Depends on D6056
Linear Task: ENG-2473

Test Plan

Please watch the demo video to see the changes I made:

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 27 2022, 7:32 AM
Harbormaster failed remote builds in B14804: Diff 20190!
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, tomek.
ginsu edited the summary of this revision. (Show Details)

rebase

get threadID from threadInfo not ChatMessageInfoItem

fixed bug where invariant was being triggered whenever sending new message

Will be syncing with @tomek about landing these changes since he is also working in tooltip-utils for the copy button on the tooltip.

The only thing we should synchronize is the ordering of the buttons. Other than that the diffs could be landed in any order and there shouldn't be significant conflicts.

web/utils/tooltip-utils.js
422 ↗(On Diff #20258)

We can transform this to boolean here instead of when rendering

442–444 ↗(On Diff #20258)

When a lambda has the same parameter as the only function called inside it, you should be able to simply use the inner function instead.

If that's not possible due to Flow, it means that we typed a callback as returning void instead of mixed. We should always prefer mixed when we don't care about the result to make our api more convenient to use.

452 ↗(On Diff #20258)

Resigning to unblock since accepted by @tomek.

This revision is now accepted and ready to land.Dec 29 2022, 1:01 PM