Tooltip should be in a Portal

[dgreif]

Describe the bug The Tooltip component currently uses a ::before pseudo-element to render its content, with a z-index of 1000001. While this works in a lot of scenarios, it does not match the Portal approach used by many of the other Overlays in PRC. In particular, tooltips can get covered up when their ancestor has created a new stacking context (with something like z-index: 0). It can also be a problem if the parent is clipped or has overflow: hidden.

Rather than relying on ancestors to be styled correctly, we should put the Tooltip in a Portal. This way it will always be rendered with a z-index: 1 at the page stacking context.

To Reproduce See https://codesandbox.io/s/magical-tesla-4pdhth?file=/src/App.js

Expected behavior Tooltip should always show up above anything with z-index: 1

I'm not sure if anchoredPosition has a 1-to-1 matching with the direction option, so we may need to add more options there, or else this will probably be a breaking change. likely a breaking change anyways given the Portal z-index is 1 at the page level.

[lesliecdubs]

Thanks @dgreif, we agree with this assertion. Moving to the backlog to address in the future.

[mperrotti]

Related: a SegmentedControl with icon-only buttons (which show a tooltip when hovered) needs this work to be done to be considered accessible.

[milemons]

Hello! Would it be possible to get this prioritized? 🙏 We currently can't use the tooltip component (and have been relying on title for most elements due to it being clipped (since we have overflow: hidden or zIndex: 1 set on many items.

We also have had some issues where the tooltip position does not change automatically and sometimes the tooltip is cut off the page (ie. if there's a button on the left side of the page, the tooltip will overflow off the edge of the page)

[milemons]

Related issue: https://github.com/github/repos/issues/3485 And related slack discussion for another issue: https://github.slack.com/archives/C0357UQ03KN/p1667487768081549

[josepmartins]

Adding more related slack discussions:

• Table cell in Memex: https://github.slack.com/archives/C01L618AEP9/p1659380173386559
• Repos https://github.slack.com/archives/C01L618AEP9/p1667487906150179

[lesliecdubs]

Thanks all, I've moved this back to the PRC Inbox to review again on Monday. We'll look into prioritizing it in time for Repos GA, which I've been told is planned for January.

[lesliecdubs]

Following up to share that I've added this into the queue for one of our new teammates (@radglob, see this GitHub-internal link for context) to look at in December once he's a bit more onboarded into PRC.

[lesliecdubs]

Noting for reference that this issue will block SegmentedControl from passing accessibility checks. See https://github.com/primer/react/pull/2815#issuecomment-1407088656.

[lesliecdubs]

In order to use popover API here, we need to update some deps which creates incompatibility with consuming apps.

[lesliecdubs]

Blocked until we figure out path forward for https://github.com/github/primer/issues/1626 fyi @siddharthkp.

Please drop a comment if this issue is blocking any other work.

[lesliecdubs]

👋 @broccolinisoup was just cleaning up some issues and stumbled upon this one - it seems to me like you may have moved this forward in https://github.com/primer/react/pull/3394. Is that right?

[broccolinisoup]

@lesliecdubs That is correct! It is also under the React Tooltip Epic task list. We can now remove theblocked label! I can also assign myself to the issue as well - Let me know if you have a different suggestion

[lesliecdubs]

That's great, thanks @broccolinisoup! I assigned you and moved from "blocked" to "ready for review" as https://github.com/primer/react/pull/3394 appears to be awaiting 👀 . Feel free to move this to another column in the Primer teams backlog if there is more to be done.