Allow specifying zIndex for tooltip portals #1346
Conversation
kangaechigai
force-pushed
the
tooltip-portal-zIndex
branch
from
34abd9d
to
5ba6651
Compare
|
|
||
| it("should pass zIndex prop from component to Portal", () => { | ||
| const wrapper = shallow( | ||
| <TooltipWithZIndex zIndexOption={1} zIndexProp="var(--tooltip-zindex)" />, |
There was a problem hiding this comment.
@kangaechigai , is the setting for zIndexOption needed here since you're setting the zIndexProp?
There was a problem hiding this comment.
Not needed, but I wanted to illustrate that the prop overrides the option.
There was a problem hiding this comment.
Hey @kangaechigai thanks so much for the contribution! Sorry for the slow review, I was gone on vacation for the past few weeks ⛱️
Looks like there may be a merge conflict but otherwise this looks great to me! If you don't have the bandwidth to fix the conflict let me know and I can take over. thanks again! ❤️
| @@ -154,8 +156,8 @@ interface RectReadOnly { | |||
|
|
|||
| `Portal` is a component which simply renders its children inside a `div` element appended to | |||
| `document.body` created by `ReactDOM`. A `Portal` can be an effective strategy for solving the | |||
| (`z-index` stacking context | |||
| problem)[rg/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context] for | |||
| [`z-index` stacking context | |||
| return <TooltipInPortal zIndex={zIndexProp}>Hello</TooltipInPortal>; | ||
| }; | ||
|
|
||
| describe("useTooltipInPortal()", () => { |
There was a problem hiding this comment.
thanks for adding tests!
you've already done the hard work with enzyme, but just noting that we recently added react testing library so that's also an option (no need to refactor unless you feel particularly inclined 😅 )
|
@williaster Thanks for the review (and for setting a good example of not reviewing while you were on vacation)! I resolved the merge conflict. I probably won't have bandwidth anytime soon to refactor tests, so hopefully the current enzyme ones are okay for now. (I might open another PR in the next couple weeks with some potential improvements to |
|
🎉 This PR is included in version |
🚀 Enhancements
@visx/tooltip:PortalacceptsstringforzIndexto allow for the use of CSS custom properties.useTooltipInPortalaccepts azIndexoption to pass through to thePortal. TheTooltipInPortalcomponent it returns also accepts azIndexprop to override the option.@visx/xychart:Tooltipaccepts azIndexprop to pass through to thePortal. Fixes Tooltips created by Tooltip in visx/xy-chart can appear behind the chart in some cases #1333.