[web] extend navigation sidebar tooltip api
ClosedPublic
Actions

Authored by ginsu on Dec 27 2023, 9:37 PM.

Details

Reviewers

atul
rohan
kamil

Commits

rCOMM1e7bfece5f6a: [web] extend navigation sidebar tooltip api

Summary

In the new community header/topbar, we will be introducing some buttons that when hovered, a tooltip will be rendered below the element at a smaller margin (16px instead of 24px). At the moment, the only direction the navigation sidebar tooltip can go is right with 24px of margin. So this diff extends the api of the navigation sidebar tooltip to take in a tooltip position and a margin size and will render the navigation sidebar tooltip based off these new parameters.

Also please note, I am aware naming this component a navigation sidebar tooltip is probably now a bit dated, but in the very next diff I will update the name of this component to be something more generic

For context here is what the buttons on the community header/topbar will look like:

Screenshot 2023-12-28 at 12.39.47 AM.png (568×596 px, 36 KB)

Depends on D10471

Test Plan

Please see the screenshots below. Also witch each example below I confirmed that the tooltip size returned by calculateNavigationSidebarTooltipSize correctly matched the size of the element

right w/ 16px of margin

Screenshot 2023-12-28 at 12.44.59 AM.png (222×390 px, 13 KB)

left w/24 px of margin

Screenshot 2023-12-28 at 12.45.28 AM.png (322×354 px, 18 KB)

top w/ 24px of margin

Screenshot 2023-12-28 at 12.46.23 AM.png (302×432 px, 18 KB)

bottom w/ 16px of margin

Screenshot 2023-12-28 at 12.47.01 AM.png (300×308 px, 17 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Dec 27 2023, 9:37 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 27 2023, 9:37 PM

ginsu edited the summary of this revision. (Show Details)Dec 27 2023, 9:43 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, rohan, kamil.

ginsu attached a referenced file: F1010681: Screenshot 2023-12-28 at 12.39.47 AM.png. (Show Details)

ginsu edited the test plan for this revision. (Show Details)Dec 27 2023, 9:47 PM

ginsu attached a referenced file: F1010688: Screenshot 2023-12-28 at 12.47.01 AM.png. (Show Details)

ginsu attached a referenced file: F1010689: Screenshot 2023-12-28 at 12.46.23 AM.png. (Show Details)

ginsu attached a referenced file: F1010687: Screenshot 2023-12-28 at 12.45.28 AM.png. (Show Details)

ginsu attached a referenced file: F1010686: Screenshot 2023-12-28 at 12.44.59 AM.png. (Show Details)

ginsu edited the summary of this revision. (Show Details)Dec 27 2023, 9:49 PM

ginsu added a parent revision: D10471: [web] introduce AppList.

ginsu mentioned this in D10473: [web] rename navigation sidebar tooltip to label tooltip.Dec 27 2023, 9:54 PM

ginsu added a child revision: D10473: [web] rename navigation sidebar tooltip to label tooltip.Dec 27 2023, 9:54 PM

Harbormaster completed remote builds in B25360: Diff 35039.Dec 27 2023, 9:55 PM

ginsu requested review of this revision.Dec 27 2023, 9:55 PM

Seems reasonable, just a note about making tooltipMarginStyle more concise.

(As an aside, it seems like the tooltip calculating/styling code keeps getting more complex. Still feel that going with something like https://floating-ui.com would've been better so we could just not think about tooltips.)

web/navigation-sidebar/navigation-sidebar-toolitp.react.js

22–44 ↗

(On Diff #35039)

Could we replace this with something like:

const tooltipMarginStyle = React.useMemo(() => ({
  marginLeft: position === tooltipPositions.RIGHT ? tooltipMargin : 0,
  marginRight: position === tooltipPositions.LEFT ? tooltipMargin : 0,
  marginTop: position === tooltipPositions.BOTTOM ? tooltipMargin : 0,
  marginBottom: position === tooltipPositions.TOP ? tooltipMargin : 0,
}), [position, tooltipMargin]);