Page MenuHomePhabricator

[web] Use new tooltip in robotext message
ClosedPublic

Authored by jacek on Aug 23 2022, 5:11 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 12:42 PM
Unknown Object (File)
Mon, Nov 11, 1:52 AM
Unknown Object (File)
Sun, Nov 3, 1:26 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:06 AM
Unknown Object (File)
Mon, Oct 28, 5:05 AM
Subscribers

Details

Summary

Use new tooltip API in robotext 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 held this revision as a draft.
tomek requested changes to this revision.Aug 26 2022, 9:28 AM

This is also affected by issues from D4920. If we take an approach with unified useMessageTooltipActions from D4918 the code would be really similar. Regardless, are there any parts that can be reused between this and D4920?

web/chat/robotext-message.react.js
47

Readonly

This revision now requires changes to proceed.Aug 26 2022, 9:28 AM
web/chat/robotext-message.react.js
3

Why is this commented out

tomek requested changes to this revision.Sep 2 2022, 3:36 AM
tomek added inline comments.
web/chat/robotext-message.react.js
43–44 ↗(On Diff #16200)

Is it possible for one of these to be truthy and the other falsy? I think it should never happen.

This revision now requires changes to proceed.Sep 2 2022, 3:36 AM
web/chat/robotext-message.react.js
43–44 ↗(On Diff #16200)

You're right - it's due to flow error in line 49, as InlineSidebar must get non-optional ThreadInfo.

web/chat/robotext-message.react.js
43–44 ↗(On Diff #16200)

However, introducing containsInlineSidebar prop is probably redundant here, so I'll leave only if(this.props.item.threadCreatedFromMessage) as before

remove containsInlineSidebar prop

This revision is now accepted and ready to land.Sep 2 2022, 6:33 AM