Paths

Table of Contentst

Differential D4912

[web] Refactor `MessageTooltip` component
ClosedPublic
Actions

Authored by • jacek on Aug 23 2022, 4:52 AM.

Tags

None

Referenced Files

	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: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

Subscribers

Details

Reviewers

tomek
atul
• abosh

Commits

rCOMM36b1f6c71dcb: [web] Refactor `MessageTooltip` component

Summary

Major refactor of MessageTooltip that simplifies the component leaving only necessary properties and removing positioning logic out of the component

The new component:

Screenshot_Google Chrome_2022-08-23_143424.png (140×200 px, 10 KB)

Test Plan

Flow; Tested with the rest of diffs stack.

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Aug 23 2022, 4:52 AM

• jacek held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptAug 23 2022, 4:52 AM

Harbormaster completed remote builds in B11530: Diff 15853.Aug 23 2022, 5:01 AM

• jacek added a parent revision: D4911: [web] Introduce tooltip container with positioning in `TooltipContext`.Aug 23 2022, 5:26 AM

• jacek added a child revision: D4913: [web] Modify `findTooltipPosition` to support new tooltip positioning logic.

• jacek edited the summary of this revision. (Show Details)Aug 23 2022, 5:34 AM

• jacek added inline comments.Aug 23 2022, 5:41 AM

web/chat/composed-message.react.js
132 ↗	(On Diff #15853)	Will be changed in next diffs - change to avoid flow issue
web/chat/robotext-message.react.js
27 ↗	(On Diff #15853)	Will be removed in next diffs
69 ↗	(On Diff #15853)	Will be changed in next diffs

• jacek published this revision for review.Aug 23 2022, 8:06 AM

tomek accepted this revision.Aug 25 2022, 11:02 AM

tomek added inline comments.

web/chat/message-tooltip.css
6 ↗	(On Diff #15853)	Please use variables in any new code
web/chat/message-tooltip.react.js
16–19 ↗	(On Diff #15853)	If the classes are always enabled you can use a lot simpler syntax

This revision is now accepted and ready to land.Aug 25 2022, 11:02 AM

• abosh accepted this revision.Aug 25 2022, 11:25 AM

• abosh added inline comments.

web/chat/message-tooltip.react.js
11 ↗	(On Diff #15853)	What's a case when `messageTimestamp` would be optional? Doesn't every message have a timestamp?

• jacek added inline comments.Aug 29 2022, 3:32 AM

web/chat/message-tooltip.react.js
11 ↗	(On Diff #15853)	Yes, it doesn't have to be optional

Reabase & address feedback

Harbormaster completed remote builds in B11678: Diff 16054.Aug 29 2022, 5:44 AM

Used JS constants in styles to support correct measuring size

Harbormaster failed remote builds in B11686: Diff 16072!Aug 29 2022, 7:21 AM

Harbormaster completed remote builds in B11686: Diff 16072.Aug 29 2022, 7:54 AM

add alignment parameter

Harbormaster completed remote builds in B11786: Diff 16189.Sep 1 2022, 7:06 AM

fix css name

Harbormaster completed remote builds in B11801: Diff 16204.Sep 1 2022, 7:34 AM

Closed by commit rCOMM36b1f6c71dcb: [web] Refactor `MessageTooltip` component (authored by • jacek). · Explain WhySep 5 2022, 3:13 AM

This revision was automatically updated to reflect the committed changes.

• jacek added a commit: rCOMM36b1f6c71dcb: [web] Refactor `MessageTooltip` component.

Revision Contents
Changeset List

Path

Size

web/

chat/

composed-message.react.js

13 lines

message-tooltip.css

82 lines

message-tooltip.react.js

296 lines

robotext-message.react.js

12 lines

tooltip-utils.js

7 lines

Diff 16054

web/chat/composed-message.react.js

Loading...

web/chat/message-tooltip.css

Loading...

web/chat/message-tooltip.react.js

Loading...

web/chat/robotext-message.react.js

Loading...

web/chat/tooltip-utils.js

Loading...