Page MenuHomePhabricator
Differential D4912

[web] Refactor `MessageTooltip` component
ClosedPublic
Actions

Authored by • jacek on Aug 23 2022, 4:52 AM.
Tags
None
Referenced Files
F3345813: D4912.id16054.diff
Fri, Nov 22, 7:10 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:06 AM
Unknown Object (File)
Mon, Oct 28, 5:06 AM
Unknown Object (File)
Mon, Oct 28, 5:06 AM
View All Files
Subscribers
ashoat

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
rCOMM Comm
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

Will be changed in next diffs - change to avoid flow issue

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

Will be removed in next diffs

69

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

Please use variables in any new code

web/chat/message-tooltip.react.js
16–19

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

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

Yes, it doesn't have to be optional

• jacek updated this revision to Diff 16054.Aug 29 2022, 5:34 AM

Reabase & address feedback

Harbormaster completed remote builds in B11678: Diff 16054.Aug 29 2022, 5:44 AM
• jacek updated this revision to Diff 16072.Aug 29 2022, 7:20 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
• jacek updated this revision to Diff 16189.Sep 1 2022, 6:55 AM

add alignment parameter

Harbormaster completed remote builds in B11786: Diff 16189.Sep 1 2022, 7:06 AM
• jacek updated this revision to Diff 16204.Sep 1 2022, 7:27 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

Diff 15853

View Options

web/chat/composed-message.react.js

Loading...
View Options

web/chat/message-tooltip.css

Loading...
View Options

web/chat/message-tooltip.react.js

Loading...
View Options

web/chat/robotext-message.react.js

Loading...
View Options

web/chat/tooltip-utils.js

Loading...