Paths

Table of Contentst

Differential D4912

[web] Refactor `MessageTooltip` component
ClosedPublic
Actions

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

Tags

None

Referenced Files

	F146979: Screenshot_Google Chrome_2022-08-23_143424.png
	Aug 23 2022, 12:34 PM

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

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

• jacek held this revision as a draft.

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

Harbormaster completed remote builds in B11530: Diff 15853.Aug 23 2022, 12:01 PM

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

• 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, 12:34 PM

• jacek added inline comments.Aug 23 2022, 12:41 PM

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, 3:06 PM

tomek accepted this revision.Aug 25 2022, 6:02 PM

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, 6:02 PM

• abosh accepted this revision.Aug 25 2022, 6:25 PM

• 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, 10: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, 12:44 PM

Used JS constants in styles to support correct measuring size

Harbormaster failed remote builds in B11686: Diff 16072!Aug 29 2022, 2:21 PM

Harbormaster completed remote builds in B11686: Diff 16072.Aug 29 2022, 2:54 PM

add alignment parameter

Harbormaster completed remote builds in B11786: Diff 16189.Sep 1 2022, 2:06 PM

fix css name

Harbormaster completed remote builds in B11801: Diff 16204.Sep 1 2022, 2:34 PM

Closed by commit rCOMM36b1f6c71dcb: [web] Refactor `MessageTooltip` component. · Explain WhySep 5 2022, 10: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/

chat-constants.js

18 lines

composed-message.react.js

23 lines

message-tooltip.css

88 lines

message-tooltip.react.js

328 lines

robotext-message.react.js

13 lines

tooltip-utils.js

7 lines

Diff 16288

web/chat/chat-constants.js

Loading...

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