Use new tooltip API in composed message.
Details
- Reviewers
tomek atul • abosh - Commits
- rCOMMa7c1314455db: [web] Use new tooltip in composed message
Flow; Tested with the rest of diffs stack.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
web/chat/composed-message.react.js | ||
---|---|---|
204–205 ↗ | (On Diff #15861) | I guess you could simplify it by using a spread |
206–210 ↗ | (On Diff #15861) | It would be more performant to memoize separately all the things that don't depend on event, e.g. tooltipLabels and tooltipSize change only when action or messageTimestamp change, which means that they can be reused for a couple of different onMouseEnter calls. |
214–215 ↗ | (On Diff #15861) | preventDisplayingBelowSource doesn't seem really needed because we can just use availableTooltipPositions without BOTTOM. This will simplify the api. |
224 ↗ | (On Diff #15861) | Use shorthand |
234 ↗ | (On Diff #15861) | Can we use early exit for this? It is wasteful to compute all the non-hook things if we're not going to use them |
237 ↗ | (On Diff #15861) | It is a little confusing to see style assigned to a position |
web/chat/composed-message.react.js | ||
---|---|---|
204–205 ↗ | (On Diff #15861) | There is some flow issue, when I try to do it - getBoundingClientRect() in Flow is defined to return ClientRect instead of DomRect from specification what makes impossible to remove x and y from the object: declare class HTMLElement extends Element { ... getBoundingClientRect(): ClientRect; ... |
214–215 ↗ | (On Diff #15861) | No, when we display tooltip in LEFT, LEFT_TOP, RIGHT, RIGHT_TOP position it may be higher that the message, what will cause the tooltip to cover the inline sidebar. |