Page MenuHomePhabricator
Differential D4918

[web] Introduce `useMessageTooltipActions` hooks
ClosedPublic
Actions

Authored by • jacek on Aug 23 2022, 5:03 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 6:22 AM
Unknown Object (File)
Mon, Nov 11, 8:41 AM
Unknown Object (File)
Mon, Nov 11, 1:52 AM
Unknown Object (File)
Mon, Oct 28, 5:06 AM
Unknown Object (File)
Mon, Oct 28, 5:06 AM
Unknown Object (File)
Mon, Oct 28, 5:06 AM
Unknown Object (File)
Mon, Oct 28, 5:04 AM
Unknown Object (File)
Oct 23 2024, 9:57 AM
View All Files
Subscribers
ashoat

Details

Reviewers
tomek
atul
• abosh
Commits
rCOMM44d4262ea03e: [web] Introduce `useMessageTooltipActions` hooks
Summary

Introduce hooks return tooltip actions for robotext and composed message.

Test Plan

Flow; Tested with the rest of diffs stack.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jacek created this revision.Aug 23 2022, 5:03 AM
• jacek held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptAug 23 2022, 5:03 AM
Harbormaster completed remote builds in B11536: Diff 15859.Aug 23 2022, 5:13 AM
• jacek added a parent revision: D4917: [web] Introduce `useMessageTooltipReplyAction` hook.Aug 23 2022, 5:28 AM
• jacek added a child revision: D4919: [web] Wrap app content with `TooltipProvider`.
• jacek published this revision for review.Aug 23 2022, 8:06 AM
tomek requested changes to this revision.Aug 26 2022, 8:56 AM
tomek added inline comments.
web/chat/tooltip-utils.js
384–402

Do we have a way, based on ChatMessageInfoItem, to tell whether message is composed or robotext? If that's the case, we can simplify this a lot by modifying useMessageTooltipSidebarAction to return null for robotext.

But... is this logic correct? Why do we skip sidebar action for robotext but have reply action for it? Shouldn't this be the other way around?

This revision now requires changes to proceed.Aug 26 2022, 8:56 AM
tomek mentioned this in D4921: [web] Use new tooltip in robotext message.Aug 26 2022, 9:28 AM
• jacek added inline comments.Aug 29 2022, 7:09 AM
web/chat/tooltip-utils.js
384–402

Yes, I noticed I made a mistake here - of course sidebarAction should be the one available for robotext message

• jacek updated this revision to Diff 16197.Sep 1 2022, 7:10 AM

address feedback

• jacek retitled this revision from [web] Introduce `use...MessageTooltipActions` hooks to [web] Introduce `useMessageTooltipActions` hooks.Sep 1 2022, 7:10 AM
Harbormaster completed remote builds in B11794: Diff 16197.Sep 1 2022, 7:18 AM
tomek accepted this revision.Sep 2 2022, 3:08 AM
This revision is now accepted and ready to land.Sep 2 2022, 3:08 AM
Closed by commit rCOMM44d4262ea03e: [web] Introduce `useMessageTooltipActions` hooks (authored by • jacek). · Explain WhySep 5 2022, 3:13 AM
This revision was automatically updated to reflect the committed changes.
• jacek added a commit: rCOMM44d4262ea03e: [web] Introduce `useMessageTooltipActions` hooks.

Revision Contents
Changeset List

PathSize
web/
chat/
23 lines

Diff 15859

View Options

web/chat/tooltip-utils.js

Loading...