Skip to content

Commit

Permalink
chore(tooltip): skip async state updates in jsdom env
Browse files Browse the repository at this point in the history
  • Loading branch information
gcornut committed Sep 26, 2024
1 parent 653e87d commit 152f0af
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 34 deletions.
11 changes: 1 addition & 10 deletions packages/lumx-react/src/components/popover/usePopoverStyle.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React, { useEffect, useMemo, useState } from 'react';
import { usePopper } from 'react-popper';
import memoize from 'lodash/memoize';
import { detectOverflow } from '@popperjs/core';

import { DOCUMENT, WINDOW } from '@lumx/react/constants';
import { PopoverProps } from '@lumx/react/components/popover/Popover';
import { usePopper } from '@lumx/react/hooks/usePopper';
import { ARROW_SIZE, FitAnchorWidth, Placement } from './constants';

/**
Expand Down Expand Up @@ -104,12 +104,6 @@ export function usePopoverStyle({
}: Options): Output {
const [popperElement, setPopperElement] = useState<null | HTMLElement>(null);

if (navigator.userAgent.includes('jsdom')) {
// Skip all logic; we don't need popover positioning in jsdom.
return { styles: {}, attributes: {}, isPositioned: true, popperElement, setPopperElement };
}

// eslint-disable-next-line react-hooks/rules-of-hooks
const [arrowElement, setArrowElement] = useState<null | HTMLElement>(null);

const actualOffset: [number, number] = [offset?.along ?? 0, (offset?.away ?? 0) + (hasArrow ? ARROW_SIZE : 0)];
Expand Down Expand Up @@ -142,16 +136,13 @@ export function usePopoverStyle({
);
}

// eslint-disable-next-line react-hooks/rules-of-hooks
const { styles, attributes, state, update } = usePopper(anchorRef.current, popperElement, { placement, modifiers });
// eslint-disable-next-line react-hooks/rules-of-hooks
useEffect(() => {
update?.();
}, [children, update]);

const position = state?.placement ?? placement;

// eslint-disable-next-line react-hooks/rules-of-hooks
const popoverStyle = useMemo(() => {
const newStyles = { ...style, ...styles.popper, zIndex };

Expand Down
46 changes: 28 additions & 18 deletions packages/lumx-react/src/components/tooltip/Tooltip.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';

import { Button, IconButton } from '@lumx/react';
import { screen, render, waitFor } from '@testing-library/react';
import { screen, render } from '@testing-library/react';
import { queryAllByTagName, queryByClassName } from '@lumx/react/testing/utils/queries';
import { commonTestsSuiteRTL } from '@lumx/react/testing/utils';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -48,10 +48,23 @@ describe(`<${Tooltip.displayName}>`, () => {
forceOpen: true,
});
expect(tooltip).toBeInTheDocument();
// Default placement
expect(tooltip).toHaveAttribute('data-popper-placement', 'bottom');
expect(anchorWrapper).toBeInTheDocument();
expect(anchorWrapper).toHaveAttribute('aria-describedby', tooltip?.id);
});

it('should render with custom placement', async () => {
const { tooltip } = await setup({
label: 'Tooltip label',
children: 'Anchor',
forceOpen: true,
placement: 'top',
});
// Custom placement
expect(tooltip).toHaveAttribute('data-popper-placement', 'top');
});

it('should wrap unknown children and not add aria-describedby when closed', async () => {
const { anchorWrapper } = await setup({
label: 'Tooltip label',
Expand Down Expand Up @@ -159,17 +172,17 @@ describe(`<${Tooltip.displayName}>`, () => {
expect(ref.current === element).toBe(true);
});

it.only('should render in closeMode=hide', async () => {
const { tooltip, anchorWrapper } = await setup({
it('should render in closeMode=hide', async () => {
const { tooltip } = await setup({
label: 'Tooltip label',
children: <Button>Anchor</Button>,
closeMode: 'hide',
forceOpen: false,
});
expect(tooltip).toBeInTheDocument();
expect(anchorWrapper).toBeInTheDocument();
expect(anchorWrapper).toHaveAttribute('aria-describedby', tooltip?.id);
expect(tooltip).toHaveClass('lumx-tooltip--is-hidden');
const button = screen.queryByRole('button', { name: 'Anchor' });
expect(button?.parentElement).toBe(anchorWrapper);
expect(button).toHaveAttribute('aria-describedby', tooltip?.id);
});
});

Expand All @@ -193,12 +206,11 @@ describe(`<${Tooltip.displayName}>`, () => {
expect(button).toHaveAttribute('aria-describedby', tooltip?.id);

// Un-hover anchor button
userEvent.unhover(button);
await waitFor(() => {
expect(button).not.toHaveFocus();
// Tooltip closed
expect(tooltip).not.toBeInTheDocument();
});
await userEvent.unhover(button);

expect(button).not.toHaveFocus();
// Tooltip closed
expect(tooltip).not.toBeInTheDocument();
});

it('should activate on hover anchor and then tooltip', async () => {
Expand All @@ -225,12 +237,10 @@ describe(`<${Tooltip.displayName}>`, () => {
expect(button).toHaveAttribute('aria-describedby', tooltip?.id);

// Un-hover tooltip
userEvent.unhover(tooltip);
await waitFor(() => {
expect(button).not.toHaveFocus();
// Tooltip closed
expect(tooltip).not.toBeInTheDocument();
});
await userEvent.unhover(tooltip);
expect(button).not.toHaveFocus();
// Tooltip closed
expect(tooltip).not.toBeInTheDocument();
});

it('should activate on anchor focus visible and close on escape', async () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/lumx-react/src/components/tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
/* eslint-disable react-hooks/rules-of-hooks */
import React, { forwardRef, ReactNode, useState } from 'react';
import { createPortal } from 'react-dom';
import { usePopper } from 'react-popper';

import classNames from 'classnames';

import { DOCUMENT } from '@lumx/react/constants';
import { Comp, GenericProps, HasCloseMode } from '@lumx/react/utils/type';
import { getRootClassName, handleBasicClasses } from '@lumx/react/utils/className';
import { mergeRefs } from '@lumx/react/utils/mergeRefs';
import { useMergeRefs } from '@lumx/react/utils/mergeRefs';
import { Placement } from '@lumx/react/components/popover';
import { TooltipContextProvider } from '@lumx/react/components/tooltip/context';
import { useId } from '@lumx/react/hooks/useId';

import { useInjectTooltipRef } from './useInjectTooltipRef';
import { useTooltipOpen } from './useTooltipOpen';
import { usePopper } from '@lumx/react/hooks/usePopper';

/** Position of the tooltip relative to the anchor element. */
export type TooltipPlacement = Extract<Placement, 'top' | 'right' | 'bottom' | 'left'>;
Expand Down Expand Up @@ -94,13 +94,14 @@ export const Tooltip: Comp<TooltipProps, HTMLDivElement> = forwardRef((props, re

const labelLines = label ? label.split('\n') : [];

const tooltipRef = useMergeRefs(ref, setPopperElement, onPopperMount);
return (
<>
<TooltipContextProvider>{wrappedChildren}</TooltipContextProvider>
{isMounted &&
createPortal(
<div
ref={mergeRefs(ref, setPopperElement, onPopperMount)}
ref={tooltipRef}
{...forwardedProps}
id={id}
role="tooltip"
Expand Down
9 changes: 6 additions & 3 deletions packages/lumx-react/src/components/tooltip/useTooltipOpen.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { MutableRefObject, useEffect, useRef, useState } from 'react';
import { browserDoesNotSupportHover } from '@lumx/react/utils/browserDoesNotSupportHover';
import { TOOLTIP_HOVER_DELAY, TOOLTIP_LONG_PRESS_DELAY } from '@lumx/react/constants';
import { IS_BROWSER, TOOLTIP_HOVER_DELAY, TOOLTIP_LONG_PRESS_DELAY } from '@lumx/react/constants';
import { useCallbackOnEscape } from '@lumx/react/hooks/useCallbackOnEscape';
import { isFocusVisible } from '@lumx/react/utils/isFocusVisible';

Expand Down Expand Up @@ -31,9 +31,12 @@ export function useTooltipOpen(delay: number | undefined, anchorElement: HTMLEle
// Run timer to defer updating the isOpen state.
const deferUpdate = (duration: number) => {
if (timer) clearTimeout(timer);
timer = setTimeout(() => {
const update = () => {
setIsOpen(!!shouldOpen);
}, duration) as any;
};
// Skip timeout in fake browsers
if (!IS_BROWSER) update();
else timer = setTimeout(update, duration) as any;
};

const hoverNotSupported = browserDoesNotSupportHover();
Expand Down
5 changes: 5 additions & 0 deletions packages/lumx-react/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@ export const WINDOW = typeof window !== 'undefined' ? window : undefined;
* Optional global `document` instance (not defined when running SSR).
*/
export const DOCUMENT = typeof document !== 'undefined' ? document : undefined;

/**
* Check if we are running in a true browser
*/
export const IS_BROWSER = typeof navigator !== 'undefined' && !navigator.userAgent.includes('jsdom');
9 changes: 9 additions & 0 deletions packages/lumx-react/src/hooks/usePopper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { usePopper as usePopperHook } from 'react-popper';
import { IS_BROWSER } from '@lumx/react/constants';

/** Stub usePopper for use outside of browsers */
const useStubPopper: typeof usePopperHook = (_a, _p, { placement }: any) =>
({ attributes: { popper: { 'data-popper-placement': placement } }, styles: {} }) as any;

/** Switch hook implementation between environment */
export const usePopper: typeof usePopperHook = IS_BROWSER ? usePopperHook : useStubPopper;

0 comments on commit 152f0af

Please sign in to comment.