From ac66f31fef4b38193dea0130fe174797f25feb54 Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Wed, 14 Sep 2022 11:49:06 -0400 Subject: [PATCH 1/8] Enable custom portal container for tooltip --- .../src/sandboxes/visx-tooltip/Example.tsx | 89 +++++++++++++++++-- packages/visx-tooltip/src/Portal.tsx | 9 +- .../src/hooks/useTooltipInPortal.tsx | 28 ++++-- packages/visx-tooltip/test/Portal.test.tsx | 45 ++++++++++ .../test/useTooltipInPortal.test.tsx | 81 +++++++++++++---- 5 files changed, 223 insertions(+), 29 deletions(-) diff --git a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx index d0627443e..14fbcb444 100644 --- a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx @@ -1,4 +1,5 @@ import React, { useState, useCallback } from 'react'; +import ReactDOM from 'react-dom'; import { Tooltip, TooltipWithBounds, @@ -28,12 +29,17 @@ const tooltipStyles = { export default function Example({ width, height, showControls = true }: TooltipProps) { const [tooltipShouldDetectBounds, setTooltipShouldDetectBounds] = useState(true); + const [tooltipPortalShouldUseCustomContainer, setTooltipPortalShouldUseCustomContainer] = useState(true); const [renderTooltipInPortal, setRenderTooltipInPortal] = useState(false); + const overlayRootRef = React.useRef(null); - const { containerRef, containerBounds, TooltipInPortal } = useTooltipInPortal({ - scroll: true, - detectBounds: tooltipShouldDetectBounds, - }); + const { containerRef, containerBounds, TooltipInPortal } = useTooltipInPortal( + { + scroll: true, + detectBounds: tooltipShouldDetectBounds, + portalContainer: tooltipPortalShouldUseCustomContainer ? overlayRootRef.current ?? undefined : undefined, + } + ); const { showTooltip, @@ -81,6 +87,12 @@ export default function Example({ width, height, showControls = true }: TooltipP style={{ width, height }} onPointerMove={handlePointerMove} > +
+ {tooltipOpen ? ( <>
) : ( -
Move or touch the canvas to see the tooltip
+
+ Move or touch the canvas to see the tooltip +
)} +
I have an annoying z-index. Try  + {renderTooltipInPortal && ( + + )} +
)} @@ -205,7 +236,55 @@ export default function Example({ width, height, showControls = true }: TooltipP padding: 16px; line-height: 1.2em; } + .overlay-root { + z-index: 3000; + position: relative; + } + .overlay-layer { + position: absolute; + border-radius: 8px; + padding: 8px; + } + .overlay-under-tooltip { + top: 30px; + right: 10px; + background: rgba(52, 235, 180, 0.8); + } + .overlay-over-tooltip { + top: 70px; + right: 30px; + background: rgba(250, 235, 180, 0.8); + } `} ); } + +type OverlayLayerProps = { + container: HTMLDivElement | null; + text: string; + className?: string; + placeAfterTooltipInDom?: boolean; +}; + +const OverlayLayer = function OverlayLayer({ + className, + container, + placeAfterTooltipInDom, + text, +}: OverlayLayerProps) { + if (container) { + // Since we re-render the tooltip every time the pointer moves and its DOM node + // is placed at the end of the container, if placeAfterTooltipInDom is true we + // also want to re-render the overlay layer + const key = placeAfterTooltipInDom ? Math.random() : "overlay-under"; + return ReactDOM.createPortal( +
+ {text} +
, + container + ); + } else { + return null; + } +}; diff --git a/packages/visx-tooltip/src/Portal.tsx b/packages/visx-tooltip/src/Portal.tsx index c32614029..e9c8e5be6 100644 --- a/packages/visx-tooltip/src/Portal.tsx +++ b/packages/visx-tooltip/src/Portal.tsx @@ -2,6 +2,8 @@ import React from 'react'; import ReactDOM from 'react-dom'; export type PortalProps = { + /** Optional container for the Portal. */ + container?: HTMLDivElement; /** Optional z-index to set on the Portal. */ zIndex?: number | string; /** Content to render in the Portal. */ @@ -13,13 +15,18 @@ export default class Portal extends React.PureComponent { private node?: HTMLDivElement; componentWillUnmount() { - if (this.node && document.body) { + if (this.node && document.body && !this.props.container) { document.body.removeChild(this.node); delete this.node; } } render() { + if (!this.node && this.props.container) { + this.node = this.props.container; + if (this.props.zIndex != null) this.node.style.zIndex = `${this.props.zIndex}`; + } + // SSR check if (!this.node && typeof document !== 'undefined') { this.node = document.createElement('div'); diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index 093ed132b..b6341c7d5 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -6,7 +6,7 @@ import Tooltip, { TooltipProps } from '../tooltips/Tooltip'; import TooltipWithBounds from '../tooltips/TooltipWithBounds'; export type TooltipInPortalProps = TooltipProps & - Pick; + Pick; export type UseTooltipInPortal = { containerRef: (element: HTMLElement | SVGElement | null) => void; @@ -24,6 +24,8 @@ export type UseTooltipPortalOptions = Pick & { scroll?: boolean; /** You can optionally inject a ResizeObserver polyfill. */ polyfill?: BaseUseMeasureOptions['polyfill']; + /** Optional container for the portal. */ + portalContainer?: HTMLDivElement; }; /** @@ -32,6 +34,7 @@ export type UseTooltipPortalOptions = Pick & { */ export default function useTooltipInPortal({ detectBounds: detectBoundsOption = true, + portalContainer: portalContainerOption, zIndex: zIndexOption, ...useMeasureOptions }: UseTooltipPortalOptions | undefined = {}): UseTooltipInPortal { @@ -40,26 +43,35 @@ export default function useTooltipInPortal({ const TooltipInPortal = useMemo( () => function ({ - left: containerLeft = 0, - top: containerTop = 0, + left: tooltipLeft = 0, + top: tooltipTop = 0, detectBounds: detectBoundsProp, // allow override at component-level + portalContainer: portalContainerProp, // allow override at component-level zIndex: zIndexProp, // allow override at the component-level ...tooltipProps }: TooltipInPortalProps) { const detectBounds = detectBoundsProp == null ? detectBoundsOption : detectBoundsProp; + const portalContainer = portalContainerProp == null ? portalContainerOption : portalContainerProp; + const portalContainerRect = portalContainer?.getBoundingClientRect(); 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; + const portalLeft = portalContainer + ? tooltipLeft - (portalContainerRect?.left || 0) + (containerBounds.left || 0) + : tooltipLeft + (containerBounds.left || 0) + window.scrollX; + const portalTop = portalContainer + ? tooltipTop - (portalContainerRect?.top || 0) + (containerBounds.top || 0) + : tooltipTop + (containerBounds.top || 0) + window.scrollY; + + const additionalTooltipProps = portalContainer ? { parentRect: containerBounds } : {}; return ( - - + + ); }, - [detectBoundsOption, zIndexOption, containerBounds.left, containerBounds.top], + [detectBoundsOption, portalContainerOption, zIndexOption, containerBounds.left, containerBounds.top], ); return { diff --git a/packages/visx-tooltip/test/Portal.test.tsx b/packages/visx-tooltip/test/Portal.test.tsx index 329a33fb9..804734937 100644 --- a/packages/visx-tooltip/test/Portal.test.tsx +++ b/packages/visx-tooltip/test/Portal.test.tsx @@ -1,7 +1,52 @@ +import React from 'react'; +import '@testing-library/jest-dom'; +import { render, screen, within } from '@testing-library/react'; import { Portal } from '../src'; +const PortalWithContainer = () => { + // useState rather than useRef so it will react to the ref being available for testing purposes + const [portalContainer, setPortalContainer] = React.useState(null); + const onRefChange = React.useCallback( + (node) => { + setPortalContainer(node); + }, + [] + ); + return ( + <> +
+ {portalContainer && ( + +
+
+ )} + + ) +}; + describe('Portal', () => { test('it should be defined', () => { expect(Portal).toBeDefined(); }); + + it('should render children at the document root', async () => { + render( + <> +
+ +
+
+ , + ); + const elementInPortal = await screen.findByTestId('element-in-portal'); + expect(elementInPortal).toBeInTheDocument(); + expect(within(screen.getByTestId('inner-div')).queryByTestId('element-in-portal')).not.toBeInTheDocument(); + }); + + it('should render children in the provided portal container', async () => { + render(); + const elementInPortal = await screen.findByTestId('element-in-portal'); + expect(elementInPortal).toBeInTheDocument(); + expect(within(screen.getByTestId('inner-div')).getByTestId('element-in-portal')).toBeInTheDocument(); + }); }); diff --git a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx index a6ad86ad4..8603d2ba0 100644 --- a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx +++ b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx @@ -1,4 +1,6 @@ import React from 'react'; +import '@testing-library/jest-dom'; +import { render, screen, within } from '@testing-library/react'; import { shallow } from 'enzyme'; import { ResizeObserver } from '@juggle/resize-observer'; import { useTooltipInPortal } from '../src'; @@ -17,27 +19,76 @@ const TooltipWithZIndex = ({ zIndexOption, zIndexProp }: TooltipWithZIndexProps) return Hello; }; +interface TooltipWithPortalContainerProps { + shouldUsePortalContainer: boolean; +} + +const TooltipWithPortalContainer = ({ shouldUsePortalContainer }: TooltipWithPortalContainerProps) => { + // useState rather than useRef so it will react to the ref being available for testing purposes + const [portalContainer, setPortalContainer] = React.useState(null); + const onRefChange = React.useCallback( + (node) => { + setPortalContainer(node); + }, + [] + ); + + const { TooltipInPortal } = useTooltipInPortal( + { + polyfill: ResizeObserver, + portalContainer: shouldUsePortalContainer ? portalContainer ?? undefined : undefined, + } + ); + + return ( + <> +
+ +
+
+ + ) +}; + describe('useTooltipInPortal()', () => { test('it should be defined', () => { expect(useTooltipInPortal).toBeDefined(); }); - it('should pass zIndex prop from options to Portal', () => { - const wrapper = shallow(, { - disableLifecycleMethods: true, - }).dive(); - const zIndex = wrapper.find('Portal').prop('zIndex'); - expect(zIndex).toBe(1); + describe('zIndex', () => { + it('should pass zIndex prop from options to Portal', () => { + const wrapper = shallow(, { + disableLifecycleMethods: true, + }).dive(); + const zIndex = wrapper.find('Portal').prop('zIndex'); + expect(zIndex).toBe(1); + }); + + it('should pass zIndex prop from component to Portal', () => { + const wrapper = shallow( + , + { + disableLifecycleMethods: true, + }, + ).dive(); + const zIndex = wrapper.find('Portal').prop('zIndex'); + expect(zIndex).toBe('var(--tooltip-zindex)'); + }); }); - it('should pass zIndex prop from component to Portal', () => { - const wrapper = shallow( - , - { - disableLifecycleMethods: true, - }, - ).dive(); - const zIndex = wrapper.find('Portal').prop('zIndex'); - expect(zIndex).toBe('var(--tooltip-zindex)'); + describe('portalContainer', () => { + it('it should render tooltip at the document root', async () => { + render(); + const elementInPortal = await screen.findByTestId('element-in-tooltip'); + expect(elementInPortal).toBeInTheDocument(); + expect(within(screen.getByTestId('inner-div')).queryByTestId('element-in-tooltip')).not.toBeInTheDocument(); + }); + + it('it should render tooltip in the provided portal container', async () => { + render(); + const elementInPortal = await screen.findByTestId('element-in-tooltip'); + expect(elementInPortal).toBeInTheDocument(); + expect(within(screen.getByTestId('inner-div')).getByTestId('element-in-tooltip')).toBeInTheDocument(); + }); }); }); From ae56a476b5b122269ac1864a3e505c456541c210 Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Wed, 14 Sep 2022 12:18:18 -0400 Subject: [PATCH 2/8] Prettier + Readme update --- .../src/sandboxes/visx-tooltip/Example.tsx | 35 +++++++++-------- packages/visx-tooltip/Readme.md | 5 ++- .../src/hooks/useTooltipInPortal.tsx | 18 +++++++-- packages/visx-tooltip/test/Portal.test.tsx | 27 ++++++------- .../test/useTooltipInPortal.test.tsx | 39 ++++++++++--------- 5 files changed, 71 insertions(+), 53 deletions(-) diff --git a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx index 14fbcb444..d26408cb5 100644 --- a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx @@ -29,17 +29,18 @@ const tooltipStyles = { export default function Example({ width, height, showControls = true }: TooltipProps) { const [tooltipShouldDetectBounds, setTooltipShouldDetectBounds] = useState(true); - const [tooltipPortalShouldUseCustomContainer, setTooltipPortalShouldUseCustomContainer] = useState(true); + const [tooltipPortalShouldUseCustomContainer, setTooltipPortalShouldUseCustomContainer] = + useState(true); const [renderTooltipInPortal, setRenderTooltipInPortal] = useState(false); const overlayRootRef = React.useRef(null); - const { containerRef, containerBounds, TooltipInPortal } = useTooltipInPortal( - { - scroll: true, - detectBounds: tooltipShouldDetectBounds, - portalContainer: tooltipPortalShouldUseCustomContainer ? overlayRootRef.current ?? undefined : undefined, - } - ); + const { containerRef, containerBounds, TooltipInPortal } = useTooltipInPortal({ + scroll: true, + detectBounds: tooltipShouldDetectBounds, + portalContainer: tooltipPortalShouldUseCustomContainer + ? overlayRootRef.current ?? undefined + : undefined, + }); const { showTooltip, @@ -91,7 +92,7 @@ export default function Example({ width, height, showControls = true }: TooltipP {tooltipOpen ? ( <> @@ -125,15 +126,13 @@ export default function Example({ width, height, showControls = true }: TooltipP ) : ( -
- Move or touch the canvas to see the tooltip -
+
Move or touch the canvas to see the tooltip
)}
I have an annoying z-index. Try  @@ -171,7 +170,9 @@ export default function Example({ width, height, showControls = true }: TooltipP setTooltipPortalShouldUseCustomContainer(!tooltipPortalShouldUseCustomContainer)} + onChange={() => + setTooltipPortalShouldUseCustomContainer(!tooltipPortalShouldUseCustomContainer) + } />  Tooltip portal in custom container @@ -274,15 +275,15 @@ const OverlayLayer = function OverlayLayer({ text, }: OverlayLayerProps) { if (container) { - // Since we re-render the tooltip every time the pointer moves and its DOM node + // Since we re-render the tooltip every time the pointer moves and its DOM node // is placed at the end of the container, if placeAfterTooltipInDom is true we // also want to re-render the overlay layer - const key = placeAfterTooltipInDom ? Math.random() : "overlay-under"; + const key = placeAfterTooltipInDom ? Math.random() : 'overlay-under'; return ReactDOM.createPortal(
{text}
, - container + container, ); } else { return null; diff --git a/packages/visx-tooltip/Readme.md b/packages/visx-tooltip/Readme.md index 931186ad4..1d9314971 100644 --- a/packages/visx-tooltip/Readme.md +++ b/packages/visx-tooltip/Readme.md @@ -113,7 +113,8 @@ object or inject it cleanly using the `polyfill` config option below. `useTooltipInPortal` is a hook which gives you a `TooltipInPortal` component for rendering `Tooltip` or `TooltipWithBounds` in a `Portal`, outside of your component DOM tree which can be useful in many -circumstances (see below for more on `Portal`s). +circumstances (see below for more on `Portal`s). A custom portal container can be provided +via the `portalContainer` config option. ##### API @@ -128,6 +129,8 @@ type Options = { scroll?: boolean /** You can optionally inject a resize-observer polyfill */ polyfill?: { new (cb: ResizeObserverCallback): ResizeObserver } + /** Optional container for the portal. */ + portalContainer?: HTMLDivElement; /** Optional z-index to set on the Portal div */ zIndex?: number | string; } diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index b6341c7d5..1775eb5bd 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -51,7 +51,8 @@ export default function useTooltipInPortal({ ...tooltipProps }: TooltipInPortalProps) { const detectBounds = detectBoundsProp == null ? detectBoundsOption : detectBoundsProp; - const portalContainer = portalContainerProp == null ? portalContainerOption : portalContainerProp; + const portalContainer = + portalContainerProp == null ? portalContainerOption : portalContainerProp; const portalContainerRect = portalContainer?.getBoundingClientRect(); const zIndex = zIndexProp == null ? zIndexOption : zIndexProp; const TooltipComponent = detectBounds ? TooltipWithBounds : Tooltip; @@ -67,11 +68,22 @@ export default function useTooltipInPortal({ return ( - + ); }, - [detectBoundsOption, portalContainerOption, zIndexOption, containerBounds.left, containerBounds.top], + [ + detectBoundsOption, + portalContainerOption, + zIndexOption, + containerBounds.left, + containerBounds.top, + ], ); return { diff --git a/packages/visx-tooltip/test/Portal.test.tsx b/packages/visx-tooltip/test/Portal.test.tsx index 804734937..b6536b2ee 100644 --- a/packages/visx-tooltip/test/Portal.test.tsx +++ b/packages/visx-tooltip/test/Portal.test.tsx @@ -6,22 +6,19 @@ import { Portal } from '../src'; const PortalWithContainer = () => { // useState rather than useRef so it will react to the ref being available for testing purposes const [portalContainer, setPortalContainer] = React.useState(null); - const onRefChange = React.useCallback( - (node) => { - setPortalContainer(node); - }, - [] - ); + const onRefChange = React.useCallback((node) => { + setPortalContainer(node); + }, []); return ( <> -
+
{portalContainer && ( -
+
)} - ) + ); }; describe('Portal', () => { @@ -32,21 +29,25 @@ describe('Portal', () => { it('should render children at the document root', async () => { render( <> -
+
-
+
, ); const elementInPortal = await screen.findByTestId('element-in-portal'); expect(elementInPortal).toBeInTheDocument(); - expect(within(screen.getByTestId('inner-div')).queryByTestId('element-in-portal')).not.toBeInTheDocument(); + expect( + within(screen.getByTestId('inner-div')).queryByTestId('element-in-portal'), + ).not.toBeInTheDocument(); }); it('should render children in the provided portal container', async () => { render(); const elementInPortal = await screen.findByTestId('element-in-portal'); expect(elementInPortal).toBeInTheDocument(); - expect(within(screen.getByTestId('inner-div')).getByTestId('element-in-portal')).toBeInTheDocument(); + expect( + within(screen.getByTestId('inner-div')).getByTestId('element-in-portal'), + ).toBeInTheDocument(); }); }); diff --git a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx index 8603d2ba0..0e6fe4de1 100644 --- a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx +++ b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx @@ -23,31 +23,28 @@ interface TooltipWithPortalContainerProps { shouldUsePortalContainer: boolean; } -const TooltipWithPortalContainer = ({ shouldUsePortalContainer }: TooltipWithPortalContainerProps) => { +const TooltipWithPortalContainer = ({ + shouldUsePortalContainer, +}: TooltipWithPortalContainerProps) => { // useState rather than useRef so it will react to the ref being available for testing purposes const [portalContainer, setPortalContainer] = React.useState(null); - const onRefChange = React.useCallback( - (node) => { - setPortalContainer(node); - }, - [] - ); + const onRefChange = React.useCallback((node) => { + setPortalContainer(node); + }, []); - const { TooltipInPortal } = useTooltipInPortal( - { - polyfill: ResizeObserver, - portalContainer: shouldUsePortalContainer ? portalContainer ?? undefined : undefined, - } - ); + const { TooltipInPortal } = useTooltipInPortal({ + polyfill: ResizeObserver, + portalContainer: shouldUsePortalContainer ? portalContainer ?? undefined : undefined, + }); return ( <> -
+
-
+
- ) + ); }; describe('useTooltipInPortal()', () => { @@ -63,7 +60,7 @@ describe('useTooltipInPortal()', () => { const zIndex = wrapper.find('Portal').prop('zIndex'); expect(zIndex).toBe(1); }); - + it('should pass zIndex prop from component to Portal', () => { const wrapper = shallow( , @@ -81,14 +78,18 @@ describe('useTooltipInPortal()', () => { render(); const elementInPortal = await screen.findByTestId('element-in-tooltip'); expect(elementInPortal).toBeInTheDocument(); - expect(within(screen.getByTestId('inner-div')).queryByTestId('element-in-tooltip')).not.toBeInTheDocument(); + expect( + within(screen.getByTestId('inner-div')).queryByTestId('element-in-tooltip'), + ).not.toBeInTheDocument(); }); it('it should render tooltip in the provided portal container', async () => { render(); const elementInPortal = await screen.findByTestId('element-in-tooltip'); expect(elementInPortal).toBeInTheDocument(); - expect(within(screen.getByTestId('inner-div')).getByTestId('element-in-tooltip')).toBeInTheDocument(); + expect( + within(screen.getByTestId('inner-div')).getByTestId('element-in-tooltip'), + ).toBeInTheDocument(); }); }); }); From 3fbf8a9456cc6313ee44961bf5d4bffa38326292 Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Wed, 14 Sep 2022 15:15:36 -0400 Subject: [PATCH 3/8] Fix linting --- packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx | 3 +-- packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx | 8 +------- packages/visx-tooltip/test/Portal.test.tsx | 4 ++-- packages/visx-tooltip/test/useTooltipInPortal.test.tsx | 8 ++++---- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx index d26408cb5..888ccfe9d 100644 --- a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx @@ -285,7 +285,6 @@ const OverlayLayer = function OverlayLayer({
, container, ); - } else { - return null; } + return null; }; diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index 1775eb5bd..2b071eb85 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -77,13 +77,7 @@ export default function useTooltipInPortal({ ); }, - [ - detectBoundsOption, - portalContainerOption, - zIndexOption, - containerBounds.left, - containerBounds.top, - ], + [containerBounds, detectBoundsOption, portalContainerOption, zIndexOption], ); return { diff --git a/packages/visx-tooltip/test/Portal.test.tsx b/packages/visx-tooltip/test/Portal.test.tsx index b6536b2ee..ae449d8ad 100644 --- a/packages/visx-tooltip/test/Portal.test.tsx +++ b/packages/visx-tooltip/test/Portal.test.tsx @@ -14,7 +14,7 @@ const PortalWithContainer = () => {
{portalContainer && ( -
+
)} @@ -31,7 +31,7 @@ describe('Portal', () => { <>
-
+
, ); diff --git a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx index 0e6fe4de1..8d38b383f 100644 --- a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx +++ b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx @@ -41,7 +41,7 @@ const TooltipWithPortalContainer = ({ <>
-
+
); @@ -74,7 +74,7 @@ describe('useTooltipInPortal()', () => { }); describe('portalContainer', () => { - it('it should render tooltip at the document root', async () => { + it('should render tooltip at the document root', async () => { render(); const elementInPortal = await screen.findByTestId('element-in-tooltip'); expect(elementInPortal).toBeInTheDocument(); @@ -83,8 +83,8 @@ describe('useTooltipInPortal()', () => { ).not.toBeInTheDocument(); }); - it('it should render tooltip in the provided portal container', async () => { - render(); + it('should render tooltip in the provided portal container', async () => { + render(); const elementInPortal = await screen.findByTestId('element-in-tooltip'); expect(elementInPortal).toBeInTheDocument(); expect( From 7a5e3b3bf42fd1399a2d9bba8638c0953f168c14 Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Fri, 16 Sep 2022 18:06:02 -0400 Subject: [PATCH 4/8] Code review changes --- .../src/sandboxes/visx-tooltip/Example.tsx | 9 ++------- packages/visx-tooltip/Readme.md | 2 +- packages/visx-tooltip/src/Portal.tsx | 3 +-- .../visx-tooltip/src/hooks/useTooltipInPortal.tsx | 13 +++++++------ packages/visx-tooltip/test/Portal.test.tsx | 5 ++--- .../visx-tooltip/test/useTooltipInPortal.test.tsx | 5 ++--- 6 files changed, 15 insertions(+), 22 deletions(-) diff --git a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx index 888ccfe9d..860034a9f 100644 --- a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx @@ -268,12 +268,7 @@ type OverlayLayerProps = { placeAfterTooltipInDom?: boolean; }; -const OverlayLayer = function OverlayLayer({ - className, - container, - placeAfterTooltipInDom, - text, -}: OverlayLayerProps) { +function OverlayLayer({ className, container, placeAfterTooltipInDom, text }: OverlayLayerProps) { if (container) { // Since we re-render the tooltip every time the pointer moves and its DOM node // is placed at the end of the container, if placeAfterTooltipInDom is true we @@ -287,4 +282,4 @@ const OverlayLayer = function OverlayLayer({ ); } return null; -}; +} diff --git a/packages/visx-tooltip/Readme.md b/packages/visx-tooltip/Readme.md index 1d9314971..4fb856bcf 100644 --- a/packages/visx-tooltip/Readme.md +++ b/packages/visx-tooltip/Readme.md @@ -131,7 +131,7 @@ type Options = { polyfill?: { new (cb: ResizeObserverCallback): ResizeObserver } /** Optional container for the portal. */ portalContainer?: HTMLDivElement; - /** Optional z-index to set on the Portal div */ + /** Optional z-index to set on the Portal div (not applicable when a specific portal container is provided) */ zIndex?: number | string; } diff --git a/packages/visx-tooltip/src/Portal.tsx b/packages/visx-tooltip/src/Portal.tsx index e9c8e5be6..7a3f42eea 100644 --- a/packages/visx-tooltip/src/Portal.tsx +++ b/packages/visx-tooltip/src/Portal.tsx @@ -4,7 +4,7 @@ import ReactDOM from 'react-dom'; export type PortalProps = { /** Optional container for the Portal. */ container?: HTMLDivElement; - /** Optional z-index to set on the Portal. */ + /** Optional z-index to set on the Portal (not applicable when a specific portal container is provided). */ zIndex?: number | string; /** Content to render in the Portal. */ children: NonNullable; @@ -24,7 +24,6 @@ export default class Portal extends React.PureComponent { render() { if (!this.node && this.props.container) { this.node = this.props.container; - if (this.props.zIndex != null) this.node.style.zIndex = `${this.props.zIndex}`; } // SSR check diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index 2b071eb85..c86b1a23d 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -34,26 +34,27 @@ export type UseTooltipPortalOptions = Pick & { */ export default function useTooltipInPortal({ detectBounds: detectBoundsOption = true, - portalContainer: portalContainerOption, + portalContainer, zIndex: zIndexOption, ...useMeasureOptions }: UseTooltipPortalOptions | undefined = {}): UseTooltipInPortal { const [containerRef, containerBounds, forceRefreshBounds] = useMeasure(useMeasureOptions); + const portalContainerRect = useMemo( + () => portalContainer?.getBoundingClientRect(), + [portalContainer], + ); + const TooltipInPortal = useMemo( () => function ({ left: tooltipLeft = 0, top: tooltipTop = 0, detectBounds: detectBoundsProp, // allow override at component-level - portalContainer: portalContainerProp, // allow override at component-level zIndex: zIndexProp, // allow override at the component-level ...tooltipProps }: TooltipInPortalProps) { const detectBounds = detectBoundsProp == null ? detectBoundsOption : detectBoundsProp; - const portalContainer = - portalContainerProp == null ? portalContainerOption : portalContainerProp; - const portalContainerRect = portalContainer?.getBoundingClientRect(); const zIndex = zIndexProp == null ? zIndexOption : zIndexProp; const TooltipComponent = detectBounds ? TooltipWithBounds : Tooltip; // convert container coordinates to page coordinates @@ -77,7 +78,7 @@ export default function useTooltipInPortal({ ); }, - [containerBounds, detectBoundsOption, portalContainerOption, zIndexOption], + [containerBounds, detectBoundsOption, portalContainer, portalContainerRect, zIndexOption], ); return { diff --git a/packages/visx-tooltip/test/Portal.test.tsx b/packages/visx-tooltip/test/Portal.test.tsx index ae449d8ad..be5a9726a 100644 --- a/packages/visx-tooltip/test/Portal.test.tsx +++ b/packages/visx-tooltip/test/Portal.test.tsx @@ -28,12 +28,11 @@ describe('Portal', () => { it('should render children at the document root', async () => { render( - <> -
+
- , +
, ); const elementInPortal = await screen.findByTestId('element-in-portal'); expect(elementInPortal).toBeInTheDocument(); diff --git a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx index 8d38b383f..2bce5a5da 100644 --- a/packages/visx-tooltip/test/useTooltipInPortal.test.tsx +++ b/packages/visx-tooltip/test/useTooltipInPortal.test.tsx @@ -38,12 +38,11 @@ const TooltipWithPortalContainer = ({ }); return ( - <> -
+
- +
); }; From 99066ae20b45d7f296a565db74196749c08d04cb Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Thu, 10 Nov 2022 14:14:53 -0500 Subject: [PATCH 5/8] Handle portal container resizing --- packages/visx-tooltip/package.json | 1 + .../src/hooks/useTooltipInPortal.tsx | 17 +++++++++++++---- yarn.lock | 19 +++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/visx-tooltip/package.json b/packages/visx-tooltip/package.json index 8b7702107..85c4a2c3b 100644 --- a/packages/visx-tooltip/package.json +++ b/packages/visx-tooltip/package.json @@ -28,6 +28,7 @@ }, "homepage": "https://github.com/airbnb/visx#readme", "dependencies": { + "@react-hook/resize-observer": "^1.2.6", "@types/react": "*", "@visx/bounds": "3.0.0", "classnames": "^2.3.1", diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index c86b1a23d..20d1bb15a 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -1,5 +1,6 @@ -import React, { useMemo } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import useMeasure, { RectReadOnly, Options as BaseUseMeasureOptions } from 'react-use-measure'; +import useResizeObserver from '@react-hook/resize-observer'; import Portal, { PortalProps } from '../Portal'; import Tooltip, { TooltipProps } from '../tooltips/Tooltip'; @@ -40,11 +41,19 @@ export default function useTooltipInPortal({ }: UseTooltipPortalOptions | undefined = {}): UseTooltipInPortal { const [containerRef, containerBounds, forceRefreshBounds] = useMeasure(useMeasureOptions); - const portalContainerRect = useMemo( - () => portalContainer?.getBoundingClientRect(), - [portalContainer], + const [portalContainerRect, setPortalContainerRect] = useState( + portalContainer?.getBoundingClientRect() ?? null, ); + const updatePortalContainerRect = useCallback(() => { + if (portalContainer) { + setPortalContainerRect(portalContainer?.getBoundingClientRect()); + } + }, [portalContainer]); + + React.useEffect(updatePortalContainerRect, [containerBounds, portalContainer]); + useResizeObserver(portalContainer ?? null, updatePortalContainerRect); + const TooltipInPortal = useMemo( () => function ({ diff --git a/yarn.lock b/yarn.lock index 4e1b0f741..9aa87fe99 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2863,6 +2863,25 @@ "@octokit/openapi-types" "^4.0.0" "@types/node" ">= 8" +"@react-hook/latest@^1.0.2": + version "1.0.3" + resolved "https://registry.yarnpkg.com/@react-hook/latest/-/latest-1.0.3.tgz#c2d1d0b0af8b69ec6e2b3a2412ba0768ac82db80" + integrity sha512-dy6duzl+JnAZcDbNTfmaP3xHiKtbXYOaz3G51MGVljh548Y8MWzTr+PHLOfvpypEVW9zwvl+VyKjbWKEVbV1Rg== + +"@react-hook/passive-layout-effect@^1.2.0": + version "1.2.1" + resolved "https://registry.yarnpkg.com/@react-hook/passive-layout-effect/-/passive-layout-effect-1.2.1.tgz#c06dac2d011f36d61259aa1c6df4f0d5e28bc55e" + integrity sha512-IwEphTD75liO8g+6taS+4oqz+nnroocNfWVHWz7j+N+ZO2vYrc6PV1q7GQhuahL0IOR7JccFTsFKQ/mb6iZWAg== + +"@react-hook/resize-observer@^1.2.6": + version "1.2.6" + resolved "https://registry.yarnpkg.com/@react-hook/resize-observer/-/resize-observer-1.2.6.tgz#9a8cf4c5abb09becd60d1d65f6bf10eec211e291" + integrity sha512-DlBXtLSW0DqYYTW3Ft1/GQFZlTdKY5VAFIC4+km6IK5NiPPDFchGbEJm1j6pSgMqPRHbUQgHJX7RaR76ic1LWA== + dependencies: + "@juggle/resize-observer" "^3.3.1" + "@react-hook/latest" "^1.0.2" + "@react-hook/passive-layout-effect" "^1.2.0" + "@react-spring/animated@~9.4.5": version "9.4.5" resolved "https://registry.yarnpkg.com/@react-spring/animated/-/animated-9.4.5.tgz#dd9921c716a4f4a3ed29491e0c0c9f8ca0eb1a54" From 6aeb835d0834a172723219aa909035ce53d093ff Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Thu, 10 Nov 2022 14:16:55 -0500 Subject: [PATCH 6/8] Fix TooltipWithBounds for tooltips in portal --- .../src/hooks/useTooltipInPortal.tsx | 16 +++++++- .../src/tooltips/TooltipWithBounds.tsx | 41 +++++++++++++++---- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index 20d1bb15a..ad343726f 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -74,7 +74,21 @@ export default function useTooltipInPortal({ ? tooltipTop - (portalContainerRect?.top || 0) + (containerBounds.top || 0) : tooltipTop + (containerBounds.top || 0) + window.scrollY; - const additionalTooltipProps = portalContainer ? { parentRect: containerBounds } : {}; + const additionalTooltipProps = + detectBounds && portalContainer + ? { + portalContainerPosition: { + left: portalContainerRect?.left || 0, + top: portalContainerRect?.top || 0, + }, + visualParentRect: { + width: containerBounds.width, + height: containerBounds.height, + left: containerBounds.left, + top: containerBounds.top, + }, + } + : {}; return ( diff --git a/packages/visx-tooltip/src/tooltips/TooltipWithBounds.tsx b/packages/visx-tooltip/src/tooltips/TooltipWithBounds.tsx index 05045689f..ef59fe34e 100644 --- a/packages/visx-tooltip/src/tooltips/TooltipWithBounds.tsx +++ b/packages/visx-tooltip/src/tooltips/TooltipWithBounds.tsx @@ -6,7 +6,20 @@ import { TooltipPositionProvider } from '../context/TooltipPositionContext'; export type TooltipWithBoundsProps = TooltipProps & React.HTMLAttributes & - WithBoundingRectsProps & { nodeRef?: React.Ref }; + WithBoundingRectsProps & { + nodeRef?: React.Ref; + /** + * When the tooltip is in a portal, this is the position of the portal + * container to be used for offsetting calculations around bounds as a consequence. + */ + portalContainerPosition?: { left: number; top: number }; + /** + * When the tooltip is in a portal, the portal container becomes the direct parent of the tooltip. + * Often the portal is not what we want the tooltip to be bound to. Another visual parent can be specified + * by specifying its dimensions here. + */ + visualParentRect?: { width: number; height: number; left: number; top: number }; + }; function TooltipWithBounds({ children, @@ -20,19 +33,26 @@ function TooltipWithBounds({ top: initialTop = 0, unstyled = false, nodeRef, + portalContainerPosition, + visualParentRect: visualParentBounds = parentBounds, ...otherProps }: TooltipWithBoundsProps) { let transform: React.CSSProperties['transform']; let placeTooltipLeft = false; let placeTooltipUp = false; - if (ownBounds && parentBounds) { + if (ownBounds && visualParentBounds) { let left = initialLeft; let top = initialTop; - if (parentBounds.width) { - const rightPlacementClippedPx = left + offsetLeft + ownBounds.width - parentBounds.width; - const leftPlacementClippedPx = ownBounds.width - left - offsetLeft; + if (visualParentBounds.width) { + const leftInVisualParent = + left + + (portalContainerPosition?.left || 0) - + (portalContainerPosition ? visualParentBounds.left : 0); + const rightPlacementClippedPx = + leftInVisualParent + offsetLeft + ownBounds.width - visualParentBounds.width; + const leftPlacementClippedPx = ownBounds.width - leftInVisualParent - offsetLeft; placeTooltipLeft = rightPlacementClippedPx > 0 && rightPlacementClippedPx > leftPlacementClippedPx; } else { @@ -42,9 +62,14 @@ function TooltipWithBounds({ rightPlacementClippedPx > 0 && rightPlacementClippedPx > leftPlacementClippedPx; } - if (parentBounds.height) { - const bottomPlacementClippedPx = top + offsetTop + ownBounds.height - parentBounds.height; - const topPlacementClippedPx = ownBounds.height - top - offsetTop; + if (visualParentBounds.height) { + const topInVisualParent = + top + + (portalContainerPosition?.top || 0) - + (portalContainerPosition ? visualParentBounds.top : 0); + const bottomPlacementClippedPx = + topInVisualParent + offsetTop + ownBounds.height - visualParentBounds.height; + const topPlacementClippedPx = ownBounds.height - topInVisualParent - offsetTop; placeTooltipUp = bottomPlacementClippedPx > 0 && bottomPlacementClippedPx > topPlacementClippedPx; } else { From 0ec63f5a087fc11ef57f0c1aa9163d3ea7910c6a Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Wed, 18 Jan 2023 20:37:03 -0500 Subject: [PATCH 7/8] Fix linting --- .../src/sandboxes/visx-tooltip/Example.tsx | 46 +++++++++---------- .../src/hooks/useTooltipInPortal.tsx | 6 ++- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx index 860034a9f..1a8431226 100644 --- a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx @@ -27,6 +27,29 @@ const tooltipStyles = { padding: 12, }; +type OverlayLayerProps = { + container: HTMLDivElement | null; + text: string; + className?: string; + placeAfterTooltipInDom?: boolean; +}; + +function OverlayLayer({ className, container, placeAfterTooltipInDom, text }: OverlayLayerProps) { + if (container) { + // Since we re-render the tooltip every time the pointer moves and its DOM node + // is placed at the end of the container, if placeAfterTooltipInDom is true we + // also want to re-render the overlay layer + const key = placeAfterTooltipInDom ? Math.random() : 'overlay-under'; + return ReactDOM.createPortal( +
+ {text} +
, + container, + ); + } + return null; +} + export default function Example({ width, height, showControls = true }: TooltipProps) { const [tooltipShouldDetectBounds, setTooltipShouldDetectBounds] = useState(true); const [tooltipPortalShouldUseCustomContainer, setTooltipPortalShouldUseCustomContainer] = @@ -260,26 +283,3 @@ export default function Example({ width, height, showControls = true }: TooltipP ); } - -type OverlayLayerProps = { - container: HTMLDivElement | null; - text: string; - className?: string; - placeAfterTooltipInDom?: boolean; -}; - -function OverlayLayer({ className, container, placeAfterTooltipInDom, text }: OverlayLayerProps) { - if (container) { - // Since we re-render the tooltip every time the pointer moves and its DOM node - // is placed at the end of the container, if placeAfterTooltipInDom is true we - // also want to re-render the overlay layer - const key = placeAfterTooltipInDom ? Math.random() : 'overlay-under'; - return ReactDOM.createPortal( -
- {text} -
, - container, - ); - } - return null; -} diff --git a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx index ad343726f..dd76cf314 100644 --- a/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx +++ b/packages/visx-tooltip/src/hooks/useTooltipInPortal.tsx @@ -51,7 +51,11 @@ export default function useTooltipInPortal({ } }, [portalContainer]); - React.useEffect(updatePortalContainerRect, [containerBounds, portalContainer]); + React.useEffect(updatePortalContainerRect, [ + containerBounds, + portalContainer, + updatePortalContainerRect, + ]); useResizeObserver(portalContainer ?? null, updatePortalContainerRect); const TooltipInPortal = useMemo( From ec94a27e8aef875870a31d4a1158317aa9eee41e Mon Sep 17 00:00:00 2001 From: Florent Bouquet Date: Wed, 18 Jan 2023 21:08:29 -0500 Subject: [PATCH 8/8] Simplify variable name --- .../visx-demo/src/sandboxes/visx-tooltip/Example.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx index 1a8431226..81c4c19ba 100644 --- a/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-tooltip/Example.tsx @@ -52,15 +52,14 @@ function OverlayLayer({ className, container, placeAfterTooltipInDom, text }: Ov export default function Example({ width, height, showControls = true }: TooltipProps) { const [tooltipShouldDetectBounds, setTooltipShouldDetectBounds] = useState(true); - const [tooltipPortalShouldUseCustomContainer, setTooltipPortalShouldUseCustomContainer] = - useState(true); + const [tooltipShouldUseCustomContainer, setTooltipShouldUseCustomContainer] = useState(true); const [renderTooltipInPortal, setRenderTooltipInPortal] = useState(false); const overlayRootRef = React.useRef(null); const { containerRef, containerBounds, TooltipInPortal } = useTooltipInPortal({ scroll: true, detectBounds: tooltipShouldDetectBounds, - portalContainer: tooltipPortalShouldUseCustomContainer + portalContainer: tooltipShouldUseCustomContainer ? overlayRootRef.current ?? undefined : undefined, }); @@ -192,9 +191,9 @@ export default function Example({ width, height, showControls = true }: TooltipP