Page MenuHomePhabricator

[web] Introduce `calculateTooltipSize` util function
ClosedPublic

Authored by jacek on Aug 23 2022, 4:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 10:35 AM
Unknown Object (File)
Mon, Nov 25, 10:22 AM
Unknown Object (File)
Sun, Nov 24, 1:58 AM
Unknown Object (File)
Mon, Nov 11, 1:52 AM
Unknown Object (File)
Oct 28 2024, 5:06 AM
Unknown Object (File)
Oct 28 2024, 5:06 AM
Unknown Object (File)
Oct 28 2024, 5:06 AM
Unknown Object (File)
Oct 28 2024, 5:06 AM
Subscribers

Details

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 held this revision as a draft.
tomek requested changes to this revision.Aug 26 2022, 6:31 AM
tomek added inline comments.
web/chat/tooltip-utils.js
41–47

Can we synchronize these between CSS and JS so that we have a single source of truth?

50–52

Why do we need to change this?

190–191

Readonly

This revision now requires changes to proceed.Aug 26 2022, 6:31 AM
web/chat/tooltip-utils.js
50–52

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

tomek added inline comments.
web/chat/tooltip-utils.js
50–52

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.

This revision is now accepted and ready to land.Aug 30 2022, 9:05 AM