Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying zIndex for tooltip portals #1346

Merged
merged 10 commits into from
Oct 13, 2021
6 changes: 4 additions & 2 deletions packages/visx-tooltip/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ type Options = {
scroll?: boolean
/** You can optionally inject a resize-observer polyfill */
polyfill?: { new (cb: ResizeObserverCallback): ResizeObserver }
/** Optional z-index to set on the Portal div */
zIndex?: number | string;
}

useTooltipInPortal(
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this fix 🙏

problem](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context) for
`Tooltip`s.

For example, if your chart is rendered inside a stacking context with a lower `z-index` than a
Expand Down
2 changes: 1 addition & 1 deletion packages/visx-tooltip/src/Portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ReactDOM from 'react-dom';

export type PortalProps = {
/** Optional z-index to set on the Portal. */
zIndex?: number;
zIndex?: number | string;
/** Content to render in the Portal. */
children: NonNullable<React.ReactNode>;
};
Expand Down
14 changes: 9 additions & 5 deletions packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { useMemo } from 'react';
import useMeasure, { RectReadOnly, Options as BaseUseMeasureOptions } from 'react-use-measure';

import Portal from '../Portal';
import Portal, { PortalProps } from '../Portal';
import Tooltip, { TooltipProps } from '../tooltips/Tooltip';
import TooltipWithBounds from '../tooltips/TooltipWithBounds';

export type TooltipInPortalProps = TooltipProps & Pick<UseTooltipPortalOptions, 'detectBounds'>;
export type TooltipInPortalProps = TooltipProps &
Pick<UseTooltipPortalOptions, 'detectBounds' | 'zIndex'>;

export type UseTooltipInPortal = {
containerRef: (element: HTMLElement | SVGElement | null) => void;
Expand All @@ -14,7 +15,7 @@ export type UseTooltipInPortal = {
TooltipInPortal: React.FC<TooltipInPortalProps>;
};

export type UseTooltipPortalOptions = {
export type UseTooltipPortalOptions = Pick<PortalProps, 'zIndex'> & {
/** whether TooltipWithBounds should be used to auto-detect (page) boundaries and reposition itself. */
detectBounds?: boolean;
/** Debounce resize or scroll events in milliseconds (needed for positioning) */
Expand All @@ -31,6 +32,7 @@ export type UseTooltipPortalOptions = {
*/
export default function useTooltipInPortal({
detectBounds: detectBoundsOption = true,
zIndex: zIndexOption,
...useMeasureOptions
}: UseTooltipPortalOptions | undefined = {}): UseTooltipInPortal {
const [containerRef, containerBounds, forceRefreshBounds] = useMeasure(useMeasureOptions);
Expand All @@ -41,21 +43,23 @@ export default function useTooltipInPortal({
left: containerLeft = 0,
top: containerTop = 0,
detectBounds: detectBoundsProp, // allow override at component-level
zIndex: zIndexProp, // allow override at the component-level
...tooltipProps
}: TooltipInPortalProps) => {
const detectBounds = detectBoundsProp == null ? detectBoundsOption : detectBoundsProp;
const zIndex = zIndexProp == null ? zIndexOption : zIndexProp;
const TooltipComponent = detectBounds ? TooltipWithBounds : Tooltip;
// convert container coordinates to page coordinates
const portalLeft = containerLeft + (containerBounds.left || 0) + window.scrollX;
const portalTop = containerTop + (containerBounds.top || 0) + window.scrollY;

return (
<Portal>
<Portal zIndex={zIndex}>
<TooltipComponent left={portalLeft} top={portalTop} {...tooltipProps} />
</Portal>
);
},
[detectBoundsOption, containerBounds.left, containerBounds.top],
[detectBoundsOption, zIndexOption, containerBounds.left, containerBounds.top],
);

return {
Expand Down
36 changes: 36 additions & 0 deletions packages/visx-tooltip/test/useTooltipInPortal.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,43 @@
import React from 'react';
import { shallow } from 'enzyme';
import ResizeObserver from 'resize-observer-polyfill';
import { useTooltipInPortal } from '../src';
import { UseTooltipPortalOptions } from '../src/hooks/useTooltipInPortal';

interface TooltipWithZIndexProps {
zIndexOption?: UseTooltipPortalOptions['zIndex'];
zIndexProp?: UseTooltipPortalOptions['zIndex'];
}

const TooltipWithZIndex = ({ zIndexOption, zIndexProp }: TooltipWithZIndexProps) => {
const { TooltipInPortal } = useTooltipInPortal({
polyfill: ResizeObserver,
zIndex: zIndexOption,
});
return <TooltipInPortal zIndex={zIndexProp}>Hello</TooltipInPortal>;
};

describe('useTooltipInPortal()', () => {
test('it should be defined', () => {
expect(useTooltipInPortal).toBeDefined();
});

it('should pass zIndex prop from options to Portal', () => {
const wrapper = shallow(<TooltipWithZIndex zIndexOption={1} />, {
disableLifecycleMethods: true,
}).dive();
const zIndex = wrapper.find('Portal').prop('zIndex');
expect(zIndex).toEqual(1);
});

it('should pass zIndex prop from component to Portal', () => {
const wrapper = shallow(
<TooltipWithZIndex zIndexOption={1} zIndexProp="var(--tooltip-zindex)" />,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kangaechigai , is the setting for zIndexOption needed here since you're setting the zIndexProp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, but I wanted to illustrate that the prop overrides the option.

{
disableLifecycleMethods: true,
},
).dive();
const zIndex = wrapper.find('Portal').prop('zIndex');
expect(zIndex).toEqual('var(--tooltip-zindex)');
});
});
4 changes: 3 additions & 1 deletion packages/visx-xychart/src/components/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export type TooltipProps<Datum extends object> = {
*/
resizeObserverPolyfill?: UseTooltipPortalOptions['polyfill'];
} & Omit<BaseTooltipProps, 'left' | 'top' | 'children'> &
Pick<UseTooltipPortalOptions, 'debounce' | 'detectBounds' | 'scroll'>;
Pick<UseTooltipPortalOptions, 'debounce' | 'detectBounds' | 'scroll' | 'zIndex'>;

const INVISIBLE_STYLES: React.CSSProperties = {
position: 'absolute',
Expand Down Expand Up @@ -122,6 +122,7 @@ function TooltipInner<Datum extends object>({
snapTooltipToDatumX = false,
snapTooltipToDatumY = false,
verticalCrosshairStyle,
zIndex,
...tooltipProps
}: TooltipProps<Datum>) {
const { colorScale, theme, innerHeight, innerWidth, margin, xScale, yScale, dataRegistry } =
Expand All @@ -132,6 +133,7 @@ function TooltipInner<Datum extends object>({
detectBounds,
polyfill: resizeObserverPolyfill,
scroll,
zIndex,
});

// To correctly position itself in a Portal, the tooltip must know its container bounds
Expand Down
10 changes: 10 additions & 0 deletions packages/visx-xychart/test/components/Tooltip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,14 @@ describe('<Tooltip />', () => {
});
expect(container?.parentNode?.querySelectorAll('div.visx-tooltip-glyph')).toHaveLength(2);
});

it('should pass zIndex prop to Portal', () => {
const { container } = setup({
props: { zIndex: 123 },
context: { tooltipOpen: true },
});
const zIndex =
container?.parentNode?.querySelector('div.visx-tooltip')?.parentElement?.style.zIndex;
expect(zIndex).toBe('123');
});
});