fix(Popup): fix tests in related components

gravity-ui · Dec 17, 2024 · b673030 · b673030
1 parent 68dfa75
commit b673030
Show file tree

Hide file tree

Showing 19 changed files with 100 additions and 82 deletions.
diff --git a/src/components/ActionTooltip/ActionTooltip.tsx b/src/components/ActionTooltip/ActionTooltip.tsx
@@ -63,7 +63,6 @@ export function ActionTooltip(props: ActionTooltipProps) {
                 anchorElement={anchorElement}
                 disableEscapeKeyDown
                 disableOutsideClick
-                disableLayer
                 qa={qa}
             >
                 <div className={b('content', contentClassName)}>

diff --git a/src/components/ActionTooltip/__tests__/ActionTooltip.test.tsx b/src/components/ActionTooltip/__tests__/ActionTooltip.test.tsx
@@ -2,7 +2,7 @@ import React from 'react';
 
 import userEvent from '@testing-library/user-event';
 
-import {createEvent, fireEvent, render, screen} from '../../../../test-utils/utils';
+import {createEvent, fireEvent, render, screen, waitFor} from '../../../../test-utils/utils';
 import {ActionTooltip} from '../ActionTooltip';
 
 export function fireAnimationEndEvent(el: Node | Window, animationName = 'animation') {
@@ -46,7 +46,9 @@ test('should show tooltip on hover and hide on un hover', async () => {
 
     fireAnimationEndEvent(tooltip);
 
-    expect(tooltip).not.toBeInTheDocument();
+    await waitFor(() => {
+        expect(tooltip).not.toBeInTheDocument();
+    });
 });
 
 test('should show tooltip on focus and hide on blur', async () => {
@@ -71,7 +73,9 @@ test('should show tooltip on focus and hide on blur', async () => {
     fireAnimationEndEvent(tooltip);
 
     expect(button).not.toHaveFocus();
-    expect(tooltip).not.toBeInTheDocument();
+    await waitFor(() => {
+        expect(tooltip).not.toBeInTheDocument();
+    });
 });
 
 test('should hide on press Escape', async () => {
@@ -96,7 +100,9 @@ test('should hide on press Escape', async () => {
     fireAnimationEndEvent(tooltip);
 
     expect(button).toHaveFocus();
-    expect(tooltip).not.toBeInTheDocument();
+    await waitFor(() => {
+        expect(tooltip).not.toBeInTheDocument();
+    });
 });
 
 test('should show on focus and hide on un hover', async () => {
@@ -124,5 +130,7 @@ test('should show on focus and hide on un hover', async () => {
     fireAnimationEndEvent(tooltip);
 
     expect(button).toHaveFocus();
-    expect(tooltip).not.toBeInTheDocument();
+    await waitFor(() => {
+        expect(tooltip).not.toBeInTheDocument();
+    });
 });
diff --git a/src/components/Dialog/DialogFooter/DialogFooter.tsx b/src/components/Dialog/DialogFooter/DialogFooter.tsx
@@ -135,7 +135,6 @@ export class DialogFooter extends React.Component<DialogFooterInnerProps> {
                         open={showError}
                         anchorRef={this.errorTooltipRef}
                         placement={['bottom', 'top']}
-                        disableLayer
                         disablePortal
                         hasArrow
                     >

diff --git a/src/components/DropdownMenu/DropdownMenuPopup.tsx b/src/components/DropdownMenu/DropdownMenuPopup.tsx
@@ -57,15 +57,19 @@ export const DropdownMenuPopup = <T,>({
     const handleMouseEnter = React.useCallback(
         (event: React.MouseEvent<HTMLDivElement>) => {
             setActiveMenuPath(path);
-            popupProps?.onMouseEnter?.(event);
+            (popupProps?.floatingProps?.onMouseEnter as React.MouseEventHandler | undefined)?.(
+                event,
+            );
         },
         [path, popupProps, setActiveMenuPath],
     );
 
     const handleMouseLeave = React.useCallback(
         (event: React.MouseEvent<HTMLDivElement>) => {
             activateParent();
-            popupProps?.onMouseLeave?.(event);
+            (popupProps?.floatingProps?.onMouseLeave as React.MouseEventHandler | undefined)?.(
+                event,
+            );
         },
         [activateParent, popupProps],
     );
@@ -144,8 +148,11 @@ export const DropdownMenuPopup = <T,>({
             onClose={onClose}
             placement="bottom-start"
             {...popupProps}
-            onMouseEnter={handleMouseEnter}
-            onMouseLeave={handleMouseLeave}
+            floatingProps={{
+                ...popupProps?.floatingProps,
+                onMouseEnter: handleMouseEnter,
+                onMouseLeave: handleMouseLeave,
+            }}
         >
             {children || (
                 <Menu className={cnDropdownMenu('menu')} size={size} {...menuProps}>

diff --git a/src/components/Popover/Popover.scss b/src/components/Popover/Popover.scss
@@ -24,13 +24,11 @@ $block: '.#{variables.$ns}popover';
         --_--close-offset: 8px;
         --_--close-size: 24px;
 
-        &-popup-content {
-            box-sizing: border-box;
-            min-height: 40px;
-            max-width: var(--g-popover-max-width, 300px);
-            padding: var(--g-popover-padding, var(--_--padding));
-            cursor: default;
-        }
+        box-sizing: border-box;
+        min-height: 40px;
+        max-width: var(--g-popover-max-width, 300px);
+        padding: var(--g-popover-padding, var(--_--padding));
+        cursor: default;
 
         &-title {
             @include mixins.text-subheader-3();

diff --git a/src/components/Popover/Popover.tsx b/src/components/Popover/Popover.tsx
@@ -37,7 +37,6 @@ export const Popover = React.forwardRef<PopoverInstanceProps, PopoverProps & QAP
         offset = {},
         tooltipOffset,
         tooltipClassName,
-        tooltipContentClassName,
         theme = 'info',
         size = 's',
         hasArrow = true,
@@ -64,7 +63,6 @@ export const Popover = React.forwardRef<PopoverInstanceProps, PopoverProps & QAP
         focusTrap,
         autoFocus,
         restoreFocusRef,
-        middlewares,
     },
     ref,
 ) {
@@ -138,19 +136,16 @@ export const Popover = React.forwardRef<PopoverInstanceProps, PopoverProps & QAP
                 },
                 tooltipClassName,
             )}
-            contentClassName={cnPopover('tooltip-popup-content', tooltipContentClassName)}
             open={isOpen}
             placement={popupPlacement}
             hasArrow={hasArrow}
             offset={tooltipOffset}
             onClose={hasAnchor ? undefined : closeTooltip}
             qa={qa ? `${qa}-tooltip` : ''}
             disablePortal={disablePortal}
-            focusTrap={focusTrap}
             autoFocus={autoFocus}
-            restoreFocus={true}
-            restoreFocusRef={restoreFocusRef || controlRef}
-            middlewares={middlewares}
+            modalFocus={focusTrap}
+            returnFocus={restoreFocusRef}
             aria-labelledby={title ? popoverTitleId : undefined}
         >
             <React.Fragment>

diff --git a/src/components/Popover/__stories__/Popover.stories.tsx b/src/components/Popover/__stories__/Popover.stories.tsx
@@ -75,7 +75,6 @@ const meta: Meta<typeof Popover> = {
         tooltipCancelButton: {control: 'object'},
         tooltipOffset: {control: 'object'},
         tooltipClassName: {control: 'text'},
-        tooltipContentClassName: {control: 'text'},
         className: {control: 'text'},
         onClick: {action: 'onClick'},
         onOpenChange: {action: 'onOpenChange'},

diff --git a/src/components/Popover/__tests__/Popover.test.tsx b/src/components/Popover/__tests__/Popover.test.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 
 import {setupTimersMock} from '../../../../test-utils/setupTimersMock';
-import {act, fireEvent, render, screen} from '../../../../test-utils/utils';
+import {act, fireEvent, render, screen, waitFor} from '../../../../test-utils/utils';
 import {Popover} from '../Popover';
 import {PopoverBehavior, delayByBehavior} from '../config';
 import type {PopoverProps} from '../types';
@@ -32,11 +32,13 @@ const checkIfPopoverOpened = () => {
     expect(popover).toHaveClass('g-popup_open');
 };
 
-const checkIfPopoverClosed = () => {
+const checkIfPopoverClosed = async () => {
     const popover = screen.queryByTestId('popover-tooltip');
 
     if (popover) {
-        expect(popover).not.toHaveClass('g-popup_open');
+        await waitFor(() => {
+            expect(popover).not.toHaveClass('g-popup_open');
+        });
     } else {
         expect(true).toBe(true);
     }

diff --git a/src/components/Popover/types.ts b/src/components/Popover/types.ts
@@ -44,8 +44,6 @@ export interface PopoverExternalProps {
     tooltipOffset?: PopupOffset;
     /** Tooltip's css class */
     tooltipClassName?: string;
-    /** Tooltip's content css class */
-    tooltipContentClassName?: string;
     /** css class for the control */
     className?: string;
     /**
@@ -124,7 +122,7 @@ export type PopoverDefaultProps = {
 
 export type PopoverProps = Pick<
     PopupProps,
-    'anchorElement' | 'anchorRef' | 'strategy' | 'placement' | 'middlewares'
+    'anchorElement' | 'anchorRef' | 'strategy' | 'placement'
 > &
     PopoverExternalProps &
     PopoverBehaviorProps &

diff --git a/src/components/Popup/Popup.scss b/src/components/Popup/Popup.scss
@@ -26,7 +26,7 @@ $transition-distance: 10px;
     transition-property: opacity, transform;
     transition-timing-function: ease-out;
 
-    &_mounted {
+    &_open {
         visibility: visible;
     }
 

diff --git a/src/components/Popup/Popup.tsx b/src/components/Popup/Popup.tsx
@@ -119,8 +119,12 @@ export interface PopupProps extends DOMProps, AriaLabelingProps, QAProps {
     id?: string;
     /** CSS property `z-index` */
     zIndex?: number;
+    /** Callback called when `Popup` is opened and "in" transition is started */
+    onTransitionIn?: () => void;
     /** Callback called when `Popup` is opened and "in" transition is completed */
     onTransitionInComplete?: () => void;
+    /** Callback called when `Popup` is closed and "out" transition is started */
+    onTransitionOut?: () => void;
     /** Callback called when `Popup` is closed and "out" transition is completed */
     onTransitionOutComplete?: () => void;
 }
@@ -160,6 +164,8 @@ export function Popup({
     id,
     role: roleProp,
     zIndex = 1000,
+    onTransitionIn,
+    onTransitionOut,
     onTransitionInComplete,
     onTransitionOutComplete,
     ...restProps
@@ -231,8 +237,8 @@ export function Popup({
     });
 
     const role = useRole(context, {
-        enabled: Boolean(roleProp),
-        role: roleProp,
+        enabled: Boolean(roleProp || modalFocus),
+        role: roleProp ?? (modalFocus ? 'dialog' : undefined),
     });
     const dismiss = useDismiss(context, {
         enabled: !disableOutsideClick || !disableEscapeKeyDown,
@@ -274,19 +280,25 @@ export function Popup({
         [status, onTransitionInComplete],
     );
 
-    // Cannot use transitionend event for "out" transition due to unmounting from the DOM
+    // Cannot use transitionend event for these callbacks due to unmounting from the DOM
     React.useEffect(() => {
+        if (status === 'initial' && previousStatus === 'unmounted') {
+            onTransitionIn?.();
+        }
+        if (status === 'close' && previousStatus === 'open') {
+            onTransitionOut?.();
+        }
         if (status === 'unmounted' && previousStatus === 'close') {
             onTransitionOutComplete?.();
         }
-    }, [status, previousStatus, onTransitionOutComplete]);
+    }, [status, previousStatus, onTransitionIn, onTransitionOut, onTransitionOutComplete]);
 
     return isMounted || keepMounted ? (
         <Portal disablePortal={disablePortal}>
             <FloatingFocusManager
                 context={context}
-                disabled={!autoFocus}
-                modal={modalFocus}
+                disabled={!autoFocus || !isMounted}
+                modal={modalFocus && isMounted}
                 initialFocus={initialFocus ?? initialFocusRef}
                 returnFocus={returnFocus}
                 visuallyHiddenDismiss={disableFocusVisuallyHiddenDismiss ? false : i18n('close')}
@@ -305,11 +317,12 @@ export function Popup({
                     }}
                     data-floating-ui-placement={finalPlacement}
                     data-floating-ui-status={status}
+                    aria-modal={modalFocus && isMounted ? true : undefined}
                     {...getFloatingProps({...floatingProps, onTransitionEnd: handleTransitionEnd})}
                 >
                     <div
                         ref={contentRef}
-                        className={b({mounted: isMounted}, className)}
+                        className={b({open: isMounted}, className)}
                         style={style}
                         data-qa={qa}
                         id={id}

diff --git a/src/components/Popup/__tests__/Popup.test.tsx b/src/components/Popup/__tests__/Popup.test.tsx
@@ -23,14 +23,6 @@ describe('Popup', () => {
         expect(popup).toHaveClass(arbitratyClassName);
     });
 
-    test('should pass arbitraty className to content', () => {
-        const arbitratyClassName = 'arbitratyClassName';
-        render(<Popup open qa={qaId} contentClassName={arbitratyClassName}></Popup>);
-        const popup = screen.getByTestId(qaId);
-        /* eslint-disable-next-line testing-library/no-node-access */
-        expect(popup.firstChild).toHaveClass(arbitratyClassName);
-    });
-
     test('should open on click', async () => {
         const btnText = 'Click me';
         function Test() {
@@ -57,27 +49,27 @@ describe('Popup', () => {
         expect(popup).not.toHaveAttribute('role');
     });
 
-    test('should set aria-modal to true and role to dialog  if focusTrap is true', async () => {
-        render(<Popup open focusTrap />);
+    test('should set aria-modal to true and role to dialog  if modalFocus is true', async () => {
+        render(<Popup open autoFocus modalFocus />);
         const popup = screen.getByRole('dialog');
         expect(popup).toHaveAttribute('aria-modal', 'true');
     });
 
-    test('should use role from props if focusTrap is true', async () => {
-        render(<Popup open focusTrap role="alertdialog" />);
+    test('should use role from props if modalFocus is true', async () => {
+        render(<Popup open autoFocus modalFocus role="alertdialog" />);
         const popup = screen.getByRole('alertdialog');
         expect(popup).toHaveAttribute('aria-modal', 'true');
     });
 
-    test('should use aria-modal from props if focusTrap is true', async () => {
-        render(<Popup open qa={qaId} focusTrap aria-modal={false} />);
+    test('should not set aria-modal from props if modalFocus is true', async () => {
+        render(<Popup open qa={qaId} autoFocus modalFocus={false} />);
         const popup = screen.getByTestId(qaId);
         expect(popup).not.toHaveAttribute('aria-modal');
         expect(popup).not.toHaveAttribute('role');
     });
 
     test('should remove aria-modal if popup is closed', async () => {
-        render(<Popup aria-modal keepMounted />);
+        render(<Popup keepMounted autoFocus modalFocus />);
         const popup = screen.getByRole('dialog');
         expect(popup).not.toHaveAttribute('aria-modal');
     });

diff --git a/src/components/Select/Select.tsx b/src/components/Select/Select.tsx
@@ -221,14 +221,6 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
         disabled: filterable,
     });
 
-    React.useEffect(() => {
-        if (open) {
-            if (filterable) {
-                filterRef.current?.focus();
-            }
-        }
-    }, [open, filterable]);
-
     const mods: CnMods = {
         ...(width === 'max' && {width}),
     };
@@ -355,6 +347,13 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
                 virtualized={virtualized}
                 mobile={mobile}
                 placement={popupPlacement}
+                onAfterOpen={
+                    filterable
+                        ? () => {
+                              filterRef.current?.focus();
+                          }
+                        : undefined
+                }
                 onAfterClose={
                     filterable
                         ? () => {