Details

Reviewers

tomek
atul
• abosh

Commits

rCOMM7b34eea1a231: [web] Introduce `calculateTooltipSize` util function

Summary

Introduce calculation of tooltip size basing on labels size and number of actions inside.
Some parts of this logic was in findTooltipPosition before

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

• jacek held this revision as a draft.

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

Harbormaster completed remote builds in B11532: Diff 15855.Aug 23 2022, 5:05 AM

• jacek added a parent revision: D4913: [web] Modify `findTooltipPosition` to support new tooltip positioning logic.Aug 23 2022, 5:27 AM

• jacek added a child revision: D4915: [web] Introduce `getMessageActionTooltipStyle` util function.

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

tomek requested changes to this revision.Aug 26 2022, 6:31 AM

tomek added inline comments.

web/chat/tooltip-utils.js
41–47 ↗	(On Diff #15855)	Can we synchronize these between CSS and JS so that we have a single source of truth?
50–52 ↗	(On Diff #15855)	Why do we need to change this?
190–191 ↗	(On Diff #15855)	Readonly

This revision now requires changes to proceed.Aug 26 2022, 6:31 AM

• jacek added inline comments.Aug 29 2022, 5:25 AM

web/chat/tooltip-utils.js
50–52 ↗	(On Diff #15855)	After web redesign we changed the fonts and now our font stack looks like: :root { --font-stack: 'Inter', -apple-system, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica Neue', ui-sans-serif; So update here is necessary to calculate the size correctly

Used constants from constants file

Harbormaster completed remote builds in B11687: Diff 16073.Aug 29 2022, 7:33 AM

tomek accepted this revision.Aug 30 2022, 9:05 AM

tomek added inline comments.

web/chat/tooltip-utils.js
50–52 ↗	(On Diff #15855)	We can consider reading this from the stylesheet by using `getComputedStyle` with `getPropertyValue`. Not sure if this approach will work with SSR, but it might be a good solution to avoid the issue in the future. Another option is to modify `calculateTooltipSize` to always use the default font - we always call this function with it, but that would make it less reusable.