Page MenuHomePhabricator

[web] introduce useTooltip hook
ClosedPublic

Authored by ginsu on Aug 16 2023, 12:10 AM.
Tags
None
Referenced Files
F3396156: D8829.id30386.diff
Sun, Dec 1, 10:43 AM
Unknown Object (File)
Fri, Nov 29, 2:58 AM
Unknown Object (File)
Mon, Nov 25, 4:50 PM
Unknown Object (File)
Mon, Nov 25, 4:20 PM
Unknown Object (File)
Sun, Nov 10, 6:49 PM
Unknown Object (File)
Sun, Nov 10, 4:01 PM
Unknown Object (File)
Sun, Nov 10, 3:40 PM
Unknown Object (File)
Sun, Nov 10, 2:05 PM
Subscribers

Details

Summary

This diff introduces a hook named useTooltip which is a hook that any tooltip can use (even ones we introduce in the future) and handles all the common core functionality for tooltips. To build this hook, I factored out all the message tooltip specific code and left that in the useMessageTooltip hook and moved over all the tooltip agnostic code into useTooltip

Depends on D8828

Test Plan

flow and confimred that the message tooltip still works and behaves as expected

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

fix types to be read only

looks good, but not really familiar with the tooltip code

ashoat added a subscriber: rohan.

@ginsu please use git blame and arc cover in the future when determining reviewers – it seems clear that @rohan is your best bet here

rohan added 1 blocking reviewer(s): atul.

Seams reasonable, but I don't really have much experience with tooltip (arc cover seems to show I've made the most modifications since I basically just moved code around to prevent a circular dependency in D7308's stack). Would defer to someone more knowledge for the final review

Also not super familiar with tooltip logic, but looks reasonable to me

This revision is now accepted and ready to land.Aug 27 2023, 11:00 AM
atul retitled this revision from [web] introuce useTooltip hook to [web] introduce useTooltip hook.Aug 27 2023, 11:00 AM
This revision was automatically updated to reflect the committed changes.