Paths

Table of Contentst

Differential D4921

[web] Use new tooltip in robotext message
ClosedPublic
Actions

Authored by • jacek on Aug 23 2022, 5:11 AM.

Tags

None

Referenced Files

	F1715015: D4921.id16297.diff
	Tue, May 7, 4:24 PM

	Unknown Object (File)
	Sun, Apr 28, 11:10 PM

	Unknown Object (File)
	Sun, Apr 28, 11:10 PM

	Unknown Object (File)
	Sun, Apr 28, 11:10 PM

	Unknown Object (File)
	Sun, Apr 28, 11:10 PM

	Unknown Object (File)
	Sun, Apr 28, 11:06 PM

	Unknown Object (File)
	Sun, Apr 28, 10:39 PM

	Unknown Object (File)
	Sat, Apr 27, 7:20 PM

View All 89 Files

Subscribers

Details

Reviewers

tomek
atul
• abosh

Commits

rCOMM6f5610db9d1c: [web] Use new tooltip in robotext message

Summary

Use new tooltip API in robotext message

Test Plan

Flow; Tested with the rest of diffs stack.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jacek created this revision.Aug 23 2022, 5:11 AM

• jacek held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptAug 23 2022, 5:11 AM

Harbormaster completed remote builds in B11539: Diff 15862.Aug 23 2022, 5:21 AM

• jacek added a parent revision: D4920: [web] Use new tooltip in composed message.Aug 23 2022, 5:29 AM

• jacek added a child revision: D4922: [web] Remove timestamp tooltip from `ChatMessageList`.

• jacek published this revision for review.Aug 23 2022, 8:06 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 ↗	(On Diff #15862)	Readonly

This revision now requires changes to proceed.Aug 26 2022, 9:28 AM

ashoat added inline comments.Aug 26 2022, 12:01 PM

web/chat/robotext-message.react.js
3 ↗	(On Diff #15862)	Why is this commented out

used shared logic

Harbormaster completed remote builds in B11797: Diff 16200.Sep 1 2022, 7:22 AM

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

• jacek added inline comments.Sep 2 2022, 5:30 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`.

• jacek added inline comments.Sep 2 2022, 5:33 AM

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

Harbormaster completed remote builds in B11837: Diff 16242.Sep 2 2022, 5:44 AM

tomek accepted this revision.Sep 2 2022, 6:33 AM

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

Closed by commit rCOMM6f5610db9d1c: [web] Use new tooltip in robotext message (authored by • jacek). · Explain WhySep 5 2022, 3:13 AM

This revision was automatically updated to reflect the committed changes.

• jacek added a commit: rCOMM6f5610db9d1c: [web] Use new tooltip in robotext message.

Revision Contents
Changeset List

Path

Size

web/

chat/

message.react.js

9 lines

robotext-message.react.js

88 lines

Diff 16297

web/chat/message.react.js

Loading...

web/chat/robotext-message.react.js

Loading...