diff --git a/package.json b/package.json index 22bde2286..7d4033a55 100644 --- a/package.json +++ b/package.json @@ -102,7 +102,7 @@ "prettier": "^2", "puppeteer": "^22.12.1", "react": "^17", - "react-dom": "^16.13.1", + "react-dom": "^17", "react-helmet": "^5.2.1", "react-router-dom": "^5.3.0", "remark-frontmatter": "^5.0.0", diff --git a/packages/react/package.json b/packages/react/package.json index f75950a90..13e46318e 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -25,7 +25,7 @@ "dependencies": { "@popperjs/core": "^2.5.4", "classnames": "^2.2.6", - "focus-trap-react": "8", + "focus-trap-react": "^10.2.3", "focusable": "^2.3.0", "keyname": "^0.1.0", "react-id-generator": "^3.0.1", @@ -65,8 +65,9 @@ "postcss-cli": "^7.1.1", "postcss-import": "^12.0.1", "postcss-loader": "^3.0.0", - "react": "^16.13.1", - "react-dom": "^16.13.1", + "prop-types": "^15.8.1", + "react": "^17", + "react-dom": "^17", "rollup": "^2.23.0", "sinon": "^10.0.0", "ts-node": "^10.9.2", diff --git a/packages/react/src/components/ClickOutsideListener/index.tsx b/packages/react/src/components/ClickOutsideListener/index.tsx index 8def1a971..b504856b4 100644 --- a/packages/react/src/components/ClickOutsideListener/index.tsx +++ b/packages/react/src/components/ClickOutsideListener/index.tsx @@ -60,15 +60,15 @@ function ClickOutsideListener( useEffect(() => { typeof mouseEvent === 'string' && - document.addEventListener(mouseEvent, handleEvent); + document.addEventListener(mouseEvent, handleEvent, true); typeof touchEvent === 'string' && - document.addEventListener(touchEvent, handleEvent); + document.addEventListener(touchEvent, handleEvent, true); return () => { typeof mouseEvent === 'string' && - document.removeEventListener(mouseEvent, handleEvent); + document.removeEventListener(mouseEvent, handleEvent, true); typeof touchEvent === 'string' && - document.removeEventListener(touchEvent, handleEvent); + document.removeEventListener(touchEvent, handleEvent, true); }; }, [mouseEvent, touchEvent]); diff --git a/packages/react/src/components/Combobox/Combobox.test.tsx b/packages/react/src/components/Combobox/Combobox.test.tsx index f98b9adbe..b0e13e06f 100644 --- a/packages/react/src/components/Combobox/Combobox.test.tsx +++ b/packages/react/src/components/Combobox/Combobox.test.tsx @@ -515,6 +515,9 @@ test('should prevent default with enter keypress and open listbox', () => { // rtl doesn't let us mock preventDefault // see: https://github.com/testing-library/react-testing-library/issues/572 event.preventDefault = preventDefault; + if (type === 'focus') { + fireEvent.focusIn(combobox); + } fireEvent(combobox, event); }; @@ -545,6 +548,9 @@ test('should not prevent default with enter keypress and closed listbox', () => // rtl doesn't let us mock preventDefault // see: https://github.com/testing-library/react-testing-library/issues/572 event.preventDefault = preventDefault; + if (type === 'focus') { + fireEvent.focusIn(combobox); + } fireEvent(combobox, event); }; diff --git a/packages/react/src/components/OptionsMenu/OptionsMenu.test.tsx b/packages/react/src/components/OptionsMenu/OptionsMenu.test.tsx index 9b1892722..3ebf796e5 100644 --- a/packages/react/src/components/OptionsMenu/OptionsMenu.test.tsx +++ b/packages/react/src/components/OptionsMenu/OptionsMenu.test.tsx @@ -1,5 +1,6 @@ import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; import OptionsMenu from './'; import axe from '../../axe'; @@ -10,8 +11,10 @@ const trigger = (props: React.HTMLAttributes) => ( ); test('should render menu', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -22,8 +25,10 @@ test('should render menu', () => { }); test('should render children', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -33,8 +38,10 @@ test('should render children', () => { }); test('should render trigger prop', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
    •  ); @@ -43,8 +50,10 @@ test('should render trigger prop', () => { }); test('should support className prop', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -56,13 +65,15 @@ test('should support className prop', () => { }); test('should support align prop', () => { + const onSelect = jest.fn(); + render( <> - +
  • option 1
  • option 2
    •  - +
  • option 1
  • option 2
    •  @@ -79,8 +90,10 @@ test('should support align prop', () => { test('should support menuRef prop', () => { const menuRef = React.createRef(); + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -90,81 +103,101 @@ test('should support menuRef prop', () => { expect(menuRef.current).toEqual(screen.getByRole('menu')); }); -test('should toggle menu on trigger clicks', () => { +test('should toggle menu on trigger clicks', async () => { + const user = userEvent.setup(); + const onSelect = jest.fn(); + render( - +
  • option 1
    •  ); - fireEvent.click(screen.getByRole('button')); - expect(screen.getByRole('button')).toHaveAttribute('aria-expanded', 'true'); + const button = screen.getByRole('button', { name: 'trigger' }); + + await user.click(button); + expect(button).toHaveAttribute('aria-expanded', 'true'); expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'true'); - fireEvent.click(screen.getByRole('button')); - expect(screen.getByRole('button')).toHaveAttribute('aria-expanded', 'false'); + await user.click(button); + + expect(button).toHaveAttribute('aria-expanded', 'false'); expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'false'); }); test('should click trigger with down key on trigger', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
    •  ); - fireEvent.keyDown(screen.getByRole('button'), { keyCode: 40 }); - expect(screen.getByRole('button')).toHaveAttribute('aria-expanded', 'true'); + const button = screen.getByRole('button'); + + fireEvent.keyDown(button, { keyCode: 40 }); + expect(button).toHaveAttribute('aria-expanded', 'true'); }); -test('should focus trigger on close', () => { +test('should focus trigger on close', async () => { + const user = userEvent.setup(); + const onSelect = jest.fn(); + render( - +
  • option 1
    •  ); const button = screen.getByRole('button'); - fireEvent.click(button); - fireEvent.click(button); // to close + await user.click(button); // opens menu + await user.click(button); // closes menu expect(button).toHaveFocus(); }); -test('should call onClose when closed', () => { +test('should call onClose when closed', async () => { + const user = userEvent.setup(); const onClose = jest.fn(); + const onSelect = jest.fn(); render( - +
  • option 1
    •  ); const option = screen.getByRole('menuitem'); - fireEvent.click(option); + await user.click(option); expect(onClose).toBeCalled(); }); -test('should close menu when click outside event occurs', () => { +test('should close menu when click outside event occurs', async () => { + const user = userEvent.setup(); + const onSelect = jest.fn(); + render( <> - +
  • option 1
    •  ); - fireEvent.click(screen.getByRole('button', { name: 'trigger' })); + + await user.click(screen.getByRole('button', { name: 'trigger' })); expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'true'); - fireEvent.click(screen.getByRole('button', { name: 'Click me!' })); + await user.click(screen.getByRole('button', { name: 'Click me!' })); expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'false'); }); test('should return no axe violations when hidden', async () => { + const onSelect = jest.fn(); const { container } = render( - +
  • option 1
    •  ); @@ -173,8 +206,9 @@ test('should return no axe violations when hidden', async () => { }); test('should return no axe violations when shown', async () => { + const onSelect = jest.fn(); const { container } = render( - +
  • option 1
    •  ); diff --git a/packages/react/src/components/OptionsMenu/OptionsMenu.tsx b/packages/react/src/components/OptionsMenu/OptionsMenu.tsx index 6e1459a1b..370dd5a6e 100644 --- a/packages/react/src/components/OptionsMenu/OptionsMenu.tsx +++ b/packages/react/src/components/OptionsMenu/OptionsMenu.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useState, useRef } from 'react'; import OptionsMenuWrapper from './OptionsMenuWrapper'; import OptionsMenuList from './OptionsMenuList'; import setRef from '../../utils/setRef'; @@ -28,45 +28,38 @@ export interface OptionsMenuProps extends OptionsMenuAlignmentProps { children: React.ReactNode; } -interface OptionsMenuState { - show: boolean; -} - type AllOptionsMenuProps = OptionsMenuProps & React.HTMLAttributes; -export default class OptionsMenu extends Component< - AllOptionsMenuProps, - OptionsMenuState -> { - static defaultProps = { - // eslint-disable-next-line @typescript-eslint/no-empty-function - onClose: () => {}, - // eslint-disable-next-line @typescript-eslint/no-empty-function - onSelect: () => {}, - align: 'right' - }; - - private triggerRef: React.RefObject; +const OptionsMenu = ({ + children, + className, + closeOnSelect, + menuRef, + trigger, + align = 'right', + onClose = () => {}, // eslint-disable-line @typescript-eslint/no-empty-function + onSelect = () => {}, // eslint-disable-line @typescript-eslint/no-empty-function + ...other +}: AllOptionsMenuProps) => { + const [show, setShow] = useState(false); + const triggerRef = useRef(null); - constructor(props: AllOptionsMenuProps) { - super(props); - this.state = { show: false }; - this.triggerRef = React.createRef(); - } + const toggleMenu = (e: React.MouseEvent) => { + if (e.defaultPrevented) { + return; + } - toggleMenu = (event: React.MouseEvent) => { - this.setState(({ show }) => ({ show: !show })); + setShow(!show); }; - handleClose = () => { - const { onClose = OptionsMenu.defaultProps.onClose } = this.props; - this.setState({ show: false }); + const handleClose = () => { + setShow(false); onClose(); - this.triggerRef.current?.focus(); + triggerRef.current?.focus(); }; - handleTriggerKeyDown = (event: React.KeyboardEvent) => { + const handleTriggerKeyDown = (event: React.KeyboardEvent) => { const { which, target } = event; if (which === down) { event.preventDefault(); @@ -74,45 +67,30 @@ export default class OptionsMenu extends Component< } }; - render() { - const { toggleMenu, triggerRef, handleTriggerKeyDown } = this; - /* eslint-disable @typescript-eslint/no-unused-vars */ - const { - children, - className, - closeOnSelect, - menuRef, - trigger, - align, - onClose, - ...other - } = this.props; - - /* eslint-enable @typescript-eslint/no-unused-vars */ - const { show } = this.state; + return ( + + {trigger && + trigger({ + onClick: toggleMenu, + 'aria-expanded': show, + ref: triggerRef, + onKeyDown: handleTriggerKeyDown + })} + { + if (menuRef) { + setRef(menuRef, el); + } + }} + onClose={handleClose} + onSelect={onSelect} + {...other} + > + {children} + + + ); +}; - return ( - - {trigger && - trigger({ - onClick: toggleMenu, - 'aria-expanded': show, - ref: triggerRef, - onKeyDown: handleTriggerKeyDown - })} - { - if (menuRef) { - setRef(menuRef, el); - } - }} - onClose={this.handleClose} - {...other} - > - {children} - - - ); - } -} +export default OptionsMenu; diff --git a/packages/react/src/components/OptionsMenu/OptionsMenuItem.test.tsx b/packages/react/src/components/OptionsMenu/OptionsMenuItem.test.tsx index 77eff6e4d..2f603145c 100644 --- a/packages/react/src/components/OptionsMenu/OptionsMenuItem.test.tsx +++ b/packages/react/src/components/OptionsMenu/OptionsMenuItem.test.tsx @@ -1,5 +1,6 @@ import React from 'react'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import OptionsMenu, { OptionsMenuItem } from './'; import axe from '../../axe'; @@ -15,32 +16,35 @@ const defaultMenuProps = { ) }; -test('should call onSelect when menuitem is clicked', () => { +test('should call onSelect when menuitem is clicked', async () => { + const user = userEvent.setup(); const onSelect = jest.fn(); render( - + option 1 ); - fireEvent.click(screen.getByRole('menuitem')); + await user.click(screen.getByRole('menuitem')); expect(onSelect).toBeCalled(); }); -test('should not call onSelect when menuitem is disabled', () => { +test('should not call onSelect when menuitem is disabled', async () => { + const user = userEvent.setup(); + const menuOnSelect = jest.fn(); const onSelect = jest.fn(); render( - + option 1 ); - fireEvent.click(screen.getByRole('menuitem')); + await user.click(screen.getByRole('menuitem')); expect(onSelect).not.toBeCalled(); }); @@ -49,7 +53,7 @@ test('should return no axe violations', async () => { const onSelect = jest.fn(); const { container } = render( - + option 1 option 2 diff --git a/packages/react/src/components/OptionsMenu/OptionsMenuItem.tsx b/packages/react/src/components/OptionsMenu/OptionsMenuItem.tsx index 52205d1e3..a6c873b3f 100644 --- a/packages/react/src/components/OptionsMenu/OptionsMenuItem.tsx +++ b/packages/react/src/components/OptionsMenu/OptionsMenuItem.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { forwardRef } from 'react'; export interface OptionsMenuItemProps extends Pick< @@ -11,39 +11,39 @@ export interface OptionsMenuItemProps onSelect: (e: React.MouseEvent) => void; } -class OptionsMenuItemComponent extends React.Component { - static defaultProps = { - // eslint-disable-next-line @typescript-eslint/no-empty-function - onSelect: () => {} - }; - - handleClick = (event: React.MouseEvent) => { - const { disabled, onSelect } = this.props; +function OptionsMenuItemComponent({ + disabled, + className, + menuItemRef, + onSelect = () => {}, // eslint-disable-line @typescript-eslint/no-empty-function + ...other +}: OptionsMenuItemProps) { + function handleClick(event: React.MouseEvent) { if (!disabled) { onSelect(event); } - }; - - render() { - const { handleClick, props } = this; - const { menuItemRef, disabled, onSelect, ...other } = props; - return ( - // keydown happens in OptionsMenu which proxies to click - // eslint-disable-next-line jsx-a11y/click-events-have-key-events -
  • - ); } + + return ( + // keydown happens in OptionsMenu which proxies to click + // eslint-disable-next-line jsx-a11y/click-events-have-key-events +
  • + ); } -export default React.forwardRef(function OptionsMenuItem( - props: OptionsMenuItemProps, - ref: React.Ref -) { - return ; -}); +const OptionsMenuItem = forwardRef( + ({ ...props }, ref) => ( + + ) +); + +OptionsMenuItem.displayName = 'OptionsMenuItem'; + +export default OptionsMenuItem; diff --git a/packages/react/src/components/OptionsMenu/OptionsMenuList.test.tsx b/packages/react/src/components/OptionsMenu/OptionsMenuList.test.tsx index ae9be4eb4..da04ae6e8 100644 --- a/packages/react/src/components/OptionsMenu/OptionsMenuList.test.tsx +++ b/packages/react/src/components/OptionsMenu/OptionsMenuList.test.tsx @@ -1,5 +1,6 @@ import React from 'react'; import { render, fireEvent, screen } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; import OptionsMenuList from './'; import axe from '../../axe'; @@ -13,8 +14,10 @@ beforeEach(() => { }); test('should render children', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    • @@ -24,8 +27,10 @@ test('should render children', () => { }); test('should not render falsy children', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
    • {false &&
  • option 2
    • }
  • option 3
    • @@ -36,8 +41,10 @@ test('should not render falsy children', () => { }); test('should cycle through children', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -61,8 +68,9 @@ test('should cycle through children', () => { }); test('should call onClose given enter keydown', () => { + const onSelect = jest.fn(); render( - +
  • option 1
  • option 2
    •  @@ -78,8 +86,10 @@ test('should call onClose given enter keydown', () => { }); test('should call onClose given space keydown', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -95,8 +105,10 @@ test('should call onClose given space keydown', () => { }); test('should call onClose given escape keydown', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -112,8 +124,10 @@ test('should call onClose given escape keydown', () => { }); test('should call onClose given tab keydown', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -128,15 +142,38 @@ test('should call onClose given tab keydown', () => { expect(defaultProps.onClose).toBeCalled(); }); -test('should call onClose when clicked outside', () => { - render( - -
  • option 1
    • -
  • option 2
    • -     +test('should call onClose when clicked outside', async () => { + const onSelect = jest.fn(); + const user = userEvent.setup(); + + const { rerender } = render( + <> + + +
  • option 1
    • +
  • option 2
    • +     + + ); + + // Focus the trigger button + const triggerButton = screen.getByTestId('trigger'); + triggerButton.focus(); + expect(triggerButton).toHaveFocus(); + + // Rerender with show=true + rerender( + <> + + +
  • option 1
    • +
  • option 2
    • +     + ); - fireEvent.click(document.body); + // Simulate clicking outside + await user.click(document.body); expect(defaultProps.onClose).toBeCalled(); }); @@ -195,8 +232,10 @@ test('should fire onSelect when menu item is selected with enter', () => { }); test('should fire onClose when menu item is selected', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -208,6 +247,8 @@ test('should fire onClose when menu item is selected', () => { }); test('should not fire onClose when menu item is selected and default prevented', () => { + const onSelect = jest.fn(); + const event = new MouseEvent('click', { bubbles: true, cancelable: true @@ -216,7 +257,7 @@ test('should not fire onClose when menu item is selected and default prevented', event.preventDefault(); render( - +
  • option 1
  • option 2
    •  @@ -228,10 +269,11 @@ test('should not fire onClose when menu item is selected and default prevented', }); test('should click child links with click events', () => { + const onSelect = jest.fn(); const onClick = jest.fn(); render( - +
  • Click me! @@ -247,6 +289,7 @@ test('should click child links with click events', () => { }); test('should call onClick handler when Enter key is pressed on a link', () => { + const onSelect = jest.fn(); const onClick = jest.fn(); const event = new KeyboardEvent('keydown', { key: 'Enter', @@ -255,7 +298,7 @@ test('should call onClick handler when Enter key is pressed on a link', () => { }); render( - +
  • Click me! @@ -272,8 +315,10 @@ test('should call onClick handler when Enter key is pressed on a link', () => { }); test('should call onClose when an item is selected and closeOnSelect is true', () => { + const onSelect = jest.fn(); + render( - +
  • option 1
  • option 2
    •  @@ -285,8 +330,9 @@ test('should call onClose when an item is selected and closeOnSelect is true', ( }); test('should return no axe violations', async () => { + const onSelect = jest.fn(); const { container } = render( - +
  • option 1
  • option 2
    •  @@ -296,8 +342,9 @@ test('should return no axe violations', async () => { }); test('should return no axe violations when closed', async () => { + const onSelect = jest.fn(); const { container } = render( - +
  • option 1
  • option 2
    •  diff --git a/packages/react/src/components/OptionsMenu/OptionsMenuList.tsx b/packages/react/src/components/OptionsMenu/OptionsMenuList.tsx index 4b8088a28..d6c74d4ad 100644 --- a/packages/react/src/components/OptionsMenu/OptionsMenuList.tsx +++ b/packages/react/src/components/OptionsMenu/OptionsMenuList.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useState, useRef, useEffect } from 'react'; import { OptionsMenuProps } from './OptionsMenu'; import ClickOutsideListener from '../ClickOutsideListener'; import classnames from 'classnames'; @@ -11,56 +11,43 @@ export interface OptionsMenuListProps className?: string; } -interface OptionsMenuListState { - itemIndex: number; -} - -export default class OptionsMenuList extends React.Component< - OptionsMenuListProps, - OptionsMenuListState -> { - static defaultProps = { - closeOnSelect: true, - // eslint-disable-next-line @typescript-eslint/no-empty-function - onSelect: () => {}, - // eslint-disable-next-line @typescript-eslint/no-empty-function - onClose: () => {} - }; - - private itemRefs: Array; - private menuRef: HTMLUListElement | null; - - constructor(props: OptionsMenuProps) { - super(props); - this.itemRefs = []; - this.state = { itemIndex: 0 }; - } - - componentDidUpdate( - prevProps: OptionsMenuProps, - prevState: OptionsMenuListState - ) { - const { itemIndex } = this.state; - const { show } = this.props; - - if (!prevProps.show && show && this.itemRefs.length) { +const OptionsMenuList = ({ + children, + menuRef, + show, + className, + onClose = () => {}, // eslint-disable-line @typescript-eslint/no-empty-function + onSelect = () => {}, // eslint-disable-line @typescript-eslint/no-empty-function + closeOnSelect = true, + ...other +}: OptionsMenuListProps) => { + const [itemIndex, setItemIndex] = useState(0); + const itemRefs = useRef>([]); + const menuRefInternal = useRef(null); + const triggerRef = useRef(null); + + useEffect(() => { + if (show && itemRefs.current.length) { // handles opens - this.itemRefs[0]?.focus(); - this.setState({ itemIndex: 0 }); - } else if (prevState.itemIndex !== itemIndex) { - // handle up/down arrows - this.itemRefs[itemIndex]?.focus(); + triggerRef.current = document.activeElement as HTMLButtonElement; + itemRefs.current[0]?.focus(); + setItemIndex(0); } - } + if (!show) { + triggerRef.current = null; + } + }, [show]); + + useEffect(() => { + itemRefs.current[itemIndex]?.focus(); + }, [itemIndex]); - private handleKeyDown = (e: KeyboardEvent) => { - const { onClose = OptionsMenuList.defaultProps.onClose } = this.props; + const handleKeyDown = (e: KeyboardEvent) => { const { which, target } = e; switch (which) { case up: case down: { - const { itemIndex } = this.state; - const itemCount = this.itemRefs.length; + const itemCount = itemRefs.current.length; let newIndex = which === 38 ? itemIndex - 1 : itemIndex + 1; // circularity @@ -71,21 +58,16 @@ export default class OptionsMenuList extends React.Component< } e.preventDefault(); - this.setState({ - itemIndex: newIndex - }); - + setItemIndex(newIndex); break; } case esc: onClose(); - break; case enter: case space: e.preventDefault(); (target as HTMLElement).click(); - break; case tab: e.preventDefault(); @@ -93,14 +75,22 @@ export default class OptionsMenuList extends React.Component< } }; - private handleClick = (e: React.MouseEvent) => { - const { menuRef, props } = this; - const { onSelect, onClose = OptionsMenuList.defaultProps.onClose } = props; - if (menuRef && menuRef.contains(e.target as HTMLElement)) { - if (!e.defaultPrevented && props.closeOnSelect) { + useEffect(() => { + const currentMenuRef = menuRefInternal.current; + currentMenuRef?.addEventListener('keydown', handleKeyDown); + return () => { + currentMenuRef?.removeEventListener('keydown', handleKeyDown); + }; + }, [handleKeyDown]); + + const handleClick = (e: React.MouseEvent) => { + if ( + menuRefInternal.current && + menuRefInternal.current.contains(e.target as HTMLElement) + ) { + if (!e.defaultPrevented && closeOnSelect) { onClose(); } - onSelect(e); } @@ -110,82 +100,65 @@ export default class OptionsMenuList extends React.Component< } }; - private handleClickOutside = () => { - const { onClose = OptionsMenuList.defaultProps.onClose, show } = this.props; + const handleClickOutside = (e: MouseEvent | TouchEvent) => { + const target = e.target as Node; + const triggerElement = triggerRef.current; + if (target === triggerElement || triggerElement?.contains(target)) { + return; + } if (show) { + e.preventDefault(); onClose(); } }; - componentDidMount() { - // see https://github.com/dequelabs/cauldron-react/issues/150 - this.menuRef?.addEventListener('keydown', this.handleKeyDown); - } - - componentWillUnmount() { - this.menuRef?.removeEventListener('keydown', this.handleKeyDown); - } - - render() { - const { props, handleClick } = this; - /* eslint-disable @typescript-eslint/no-unused-vars */ - const { - children, - menuRef, - show, - className, - onClose, - onSelect, - closeOnSelect, - ...other - } = props; - /* eslint-enable @typescript-eslint/no-unused-vars */ - - const items = React.Children.toArray(children).map((child, i) => { - const { className, ...other } = (child as React.ReactElement).props; - return React.cloneElement(child as React.ReactElement, { - key: `list-item-${i}`, - className: classnames('OptionsMenu__list-item', className), - tabIndex: -1, - role: 'menuitem', - ref: (el: HTMLLIElement) => (this.itemRefs[i] = el), - ...other - }); + const items = React.Children.toArray(children).map((child, i) => { + const { className: childClassName, ...childProps } = ( + child as React.ReactElement + ).props; + return React.cloneElement(child as React.ReactElement, { + key: `list-item-${i}`, + className: classnames('OptionsMenu__list-item', childClassName), + tabIndex: -1, + role: 'menuitem', + ref: (el: HTMLLIElement) => (itemRefs.current[i] = el), + ...childProps }); - - // This allows the ClickOutsideListener to only be activated when the menu is - // currently open. This prevents an obscure behavior where the activation of a - // different menu would cause all menus to close - const clickOutsideEventActive = !show ? false : undefined; - - // Key event is being handled in componentDidMount - /* eslint-disable jsx-a11y/click-events-have-key-events */ - /* eslint-disable jsx-a11y/role-supports-aria-props */ - return ( - +
      { + menuRefInternal.current = el; + if (menuRef) { + setRef(menuRef, el); + } + }} > -
        { - this.menuRef = el; - if (menuRef) { - setRef(menuRef, el); - } - }} - > - {items} -
      - - ); - /* eslint-enable jsx-a11y/click-events-have-key-events */ - } -} + {items} +
    +     + ); +}; + +export default OptionsMenuList; diff --git a/packages/react/src/components/Popover/index.test.tsx b/packages/react/src/components/Popover/index.test.tsx index 55cbe6fbc..2081a8714 100644 --- a/packages/react/src/components/Popover/index.test.tsx +++ b/packages/react/src/components/Popover/index.test.tsx @@ -1,43 +1,35 @@ -import React, { useRef } from 'react'; +import React, { useRef, useState } from 'react'; import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; import axe from '../../axe'; import Popover from '../Popover'; +import Button from '../Button'; import AriaIsolate from '../../utils/aria-isolate'; -let wrapperNode: HTMLDivElement | null; -beforeEach(() => { - wrapperNode = document.createElement('div'); - wrapperNode.innerHTML = ` - - `; - document.body.appendChild(wrapperNode); -}); - -afterEach(() => { - document.body.innerHTML = ''; - wrapperNode = null; -}); - -const Wrapper = ({ - buttonProps = {}, - tooltipProps = {} -}: { +interface PopoverProps { buttonProps?: React.HTMLAttributes; tooltipProps?: Omit>, 'variant'>; -}) => { - const ref = useRef(null); - const onClose = jest.fn(); +} + +const Wrapper = ({ buttonProps = {}, tooltipProps = {} }: PopoverProps) => { + const buttonRef = useRef(null); + const [show, setShow] = useState(true); + const onClose = jest.fn(() => setShow(!show)); return ( <>

    Popover title

    - + { const ref = useRef(null); const onClose = jest.fn(); + return ( - + <> { - + ); }; const WrapperPrompt = ({ buttonProps = {}, tooltipProps = {} -}: { - buttonProps?: React.HTMLAttributes; - tooltipProps?: Omit>, 'variant'>; -}) => { +}: PopoverProps) => { const ref = useRef(null); const onClose = jest.fn(); const onApply = jest.fn(); return ( - + <> - + ); }; @@ -109,7 +99,7 @@ test('renders without blowing up', async () => { test('should auto-generate id', async () => { render(); const popover = screen.getByRole('dialog'); - const button = screen.getByText('button'); + const button = screen.getByText('Popover button'); expect(popover).toBeTruthy(); const id = popover?.getAttribute('id'); expect(id).toBeTruthy(); @@ -121,7 +111,7 @@ test('should attach attribute aria-expanded correctly based on shown state', asy expect( screen.queryByRole('button', { - name: 'button', + name: 'Popover button', hidden: true, expanded: true }) @@ -129,7 +119,7 @@ test('should attach attribute aria-expanded correctly based on shown state', asy rerender(); expect( screen.queryByRole('button', { - name: 'button', + name: 'Popover button', hidden: false, expanded: false }) @@ -149,7 +139,7 @@ test('should not overwrite user provided id and aria-describedby', async () => { const tooltipProps = { id: 'popoverid' }; render(); const popover = screen.getByRole('dialog'); - const button = screen.getByText('button'); + const button = screen.getByText('Popover button'); expect(popover).toHaveAttribute('id', 'popoverid'); expect(button.getAttribute('aria-describedby')).toEqual('foo popoverid'); }); @@ -163,10 +153,9 @@ test('should call onClose on escape keypress', async () => { test('should call onClose on clicking outside', async () => { const onClose = jest.fn(); + const user = userEvent.setup(); render(); - const outsideButton = await screen.findByText(/Click Me!/i); - expect(outsideButton).toBeTruthy(); - fireEvent.click(outsideButton); + await user.click(document.body); await waitFor(() => expect(onClose).toBeCalled()); }); @@ -189,13 +178,23 @@ test('onClose should be called, when close button in prompt popover is clicked', const handleClose = jest.fn(); render(); fireEvent.click(screen.getByText('Close')); - await waitFor(() => expect(handleClose).toHaveBeenCalled()); + await waitFor(() => expect(handleClose).toHaveBeenCalledTimes(1)); +}); + +test('close popover when clicking on the popover trigger button', async () => { + render(); + const popoverTriggerButton = screen.getByText('Popover button'); + const popoverContent = screen.getByText('Popover content'); + expect(popoverContent).toBeInTheDocument(); + fireEvent.click(popoverTriggerButton); + expect(popoverContent).not.toBeInTheDocument(); }); test('onApply should be called, when apply button in prompt popover is clicked', async () => { const applyFunc = jest.fn(); + const user = userEvent.setup(); render(); - fireEvent.click(screen.getByText('Apply')); + await user.click(screen.getByText('Apply')); await waitFor(() => expect(applyFunc).toHaveBeenCalled()); }); @@ -277,7 +276,7 @@ test('should have no axe violations for prompt variant', async () => { expect(await axe(baseElement)).toHaveNoViolations(); }); -test('aria-labelleddby should exist for variant="custom"', async () => { +test('aria-labelledby should exist for variant="custom"', async () => { render(); const popover = screen.getByRole('dialog'); @@ -286,7 +285,7 @@ test('aria-labelleddby should exist for variant="custom"', async () => { expect(ariaLabelledById).toBeTruthy(); }); -test('aria-labelleddby should not exist if aria-label provided for variant="prompt"', async () => { +test('aria-labelledby should not exist if aria-label provided for variant="prompt"', async () => { render( ( } }, [targetElement, id, show]); - const handleClickOutside = () => { + const handleClickOutside = (e: MouseEvent | TouchEvent) => { + if (e.target === targetElement) { + return; + } if (show) { handleClosePopover(); } diff --git a/packages/react/src/components/TopBar/TopBarMenu.test.tsx b/packages/react/src/components/TopBar/TopBarMenu.test.tsx index dc800a0e4..12422b9e7 100644 --- a/packages/react/src/components/TopBar/TopBarMenu.test.tsx +++ b/packages/react/src/components/TopBar/TopBarMenu.test.tsx @@ -18,7 +18,7 @@ const defaultProps = { }; const optionsMenu = ( - +
  • option 1
  • option 2
    •  diff --git a/packages/react/yarn.lock b/packages/react/yarn.lock index 3453107ac..b06144ee0 100644 --- a/packages/react/yarn.lock +++ b/packages/react/yarn.lock @@ -3210,19 +3210,20 @@ find-up@^4.0.0, find-up@^4.1.0: locate-path "^5.0.0" path-exists "^4.0.0" -focus-trap-react@8: - version "8.11.3" - resolved "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-8.11.3.tgz" - integrity sha512-y126gMYuB1aVYiEZSP6/v9bAfVmAIUVixanhcoMelkz7bOh+l0c3h05CEHC8S63ztxdRI2AAPS9AsTat6jlDeQ== +focus-trap-react@^10.2.3: + version "10.2.3" + resolved "https://registry.yarnpkg.com/focus-trap-react/-/focus-trap-react-10.2.3.tgz#a5a2ea7fbb042ffa4337fde72758325ed0fb793a" + integrity sha512-YXBpFu/hIeSu6NnmV2xlXzOYxuWkoOtar9jzgp3lOmjWLWY59C/b8DtDHEAV4SPU07Nd/t+nS/SBNGkhUBFmEw== dependencies: - focus-trap "^6.9.4" + focus-trap "^7.5.4" + tabbable "^6.2.0" -focus-trap@^6.9.4: - version "6.9.4" - resolved "https://registry.npmjs.org/focus-trap/-/focus-trap-6.9.4.tgz" - integrity sha512-v2NTsZe2FF59Y+sDykKY+XjqZ0cPfhq/hikWVL88BqLivnNiEffAsac6rP6H45ff9wG9LL5ToiDqrLEP9GX9mw== +focus-trap@^7.5.4: + version "7.5.4" + resolved "https://registry.yarnpkg.com/focus-trap/-/focus-trap-7.5.4.tgz#6c4e342fe1dae6add9c2aa332a6e7a0bbd495ba2" + integrity sha512-N7kHdlgsO/v+iD/dMoJKtsSqs5Dz/dXZVebRgJw23LDk+jMi/974zyiOYDziY2JPp8xivq9BmUGwIJMiuSBi7w== dependencies: - tabbable "^5.3.3" + tabbable "^6.2.0" focusable@^2.3.0: version "2.3.0" @@ -5079,9 +5080,9 @@ prompts@^2.0.1: kleur "^3.0.3" sisteransi "^1.0.5" -prop-types@^15.6.2: +prop-types@^15.8.1: version "15.8.1" - resolved "https://registry.npmjs.org/prop-types/-/prop-types-15.8.1.tgz" + resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.8.1.tgz#67d87bf1a694f48435cf332c24af10214a3140b5" integrity sha512-oj87CgZICdulUohogVAR7AjlC0327U4el4L6eAvOqCeudMDVU0NThNaV+b9Df4dXgSP1gXMTnPdhfe/2qDH5cg== dependencies: loose-envify "^1.4.0" @@ -5125,15 +5126,14 @@ queue-microtask@^1.2.2: resolved "https://registry.yarnpkg.com/queue-microtask/-/queue-microtask-1.2.3.tgz#4929228bbc724dfac43e0efb058caf7b6cfb6243" integrity sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A== -react-dom@^16.13.1: - version "16.14.0" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.14.0.tgz#7ad838ec29a777fb3c75c3a190f661cf92ab8b89" - integrity sha512-1gCeQXDLoIqMgqD3IO2Ah9bnf0w9kzhwN5q4FGnHZ67hBm9yePzB5JJAIQCc8x3pFnNlwFq4RidZggNAAkzWWw== +react-dom@^17: + version "17.0.2" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-17.0.2.tgz#ecffb6845e3ad8dbfcdc498f0d0a939736502c23" + integrity sha512-s4h96KtLDUQlsENhMn1ar8t2bEa+q/YAtj8pPPdIjPDGBDIVNsrD9aXNWqspUe6AzKCIG0C1HZZLqLV7qpOBGA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.19.1" + scheduler "^0.20.2" react-fast-compare@^3.0.1: version "3.2.2" @@ -5147,7 +5147,7 @@ react-id-generator@^3.0.1: react-is@^16.13.1: version "16.13.1" - resolved "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== react-is@^17.0.1: @@ -5179,14 +5179,13 @@ react-syntax-highlighter@^15.5.0: prismjs "^1.27.0" refractor "^3.6.0" -react@^16.13.1: - version "16.14.0" - resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" - integrity sha512-0X2CImDkJGApiAlcf0ODKIneSwBPhqJawOa5wCtKbu7ZECrmS26NvtSILynQ66cgkT/RJ4LidJOc3bUESwmU8g== +react@^17: + version "17.0.2" + resolved "https://registry.yarnpkg.com/react/-/react-17.0.2.tgz#d0b5cc516d29eb3eee383f75b62864cfb6800037" + integrity sha512-gnhPt75i/dq/z3/6q/0asP78D0u592D5L1pd7M8P+dck6Fu/jJeL6iVVK23fptSUZj8Vjf++7wXA8UNclGQcbA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" - prop-types "^15.6.2" read-cache@^1.0.0: version "1.0.0" @@ -5402,10 +5401,10 @@ saxes@^6.0.0: dependencies: xmlchars "^2.2.0" -scheduler@^0.19.1: - version "0.19.1" - resolved "https://registry.npmjs.org/scheduler/-/scheduler-0.19.1.tgz" - integrity sha512-n/zwRWRYSUj0/3g/otKDRPMh6qv2SYMWNq85IEa8iZyAv8od9zDYpGSnpBEjNgcMNq6Scbu5KfIPxNF72R/2EA== +scheduler@^0.20.2: + version "0.20.2" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.20.2.tgz#4baee39436e34aa93b4874bddcbf0fe8b8b50e91" + integrity sha512-2eWfGgAqqWFGqtdMmcL5zCMK1U8KlXv8SQFGglL3CEtd0aDVDWgeF/YoCmvln55m5zSk3J/20hTaSBeSObsQDQ== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" @@ -5718,10 +5717,10 @@ symbol-tree@^3.2.4: resolved "https://registry.yarnpkg.com/symbol-tree/-/symbol-tree-3.2.4.tgz#430637d248ba77e078883951fb9aa0eed7c63fa2" integrity sha512-9QNk5KwDF+Bvz+PyObkmSYjI5ksVUYtjW7AU22r2NKcfLJcXp96hkDWU3+XndOsUb+AQ9QhfzfCT2O+CNWT5Tw== -tabbable@^5.3.3: - version "5.3.3" - resolved "https://registry.npmjs.org/tabbable/-/tabbable-5.3.3.tgz" - integrity sha512-QD9qKY3StfbZqWOPLp0++pOrAVb/HbUi5xCc8cUo4XjP19808oaMiDzn0leBY5mCespIBM0CIZePzZjgzR83kA== +tabbable@^6.2.0: + version "6.2.0" + resolved "https://registry.yarnpkg.com/tabbable/-/tabbable-6.2.0.tgz#732fb62bc0175cfcec257330be187dcfba1f3b97" + integrity sha512-Cat63mxsVJlzYvN51JmVXIgNoUokrIaT2zLclCXjRd8boZ0004U4KCs/sToJ75C6sdlByWxpYnb5Boif1VSFew== test-exclude@^6.0.0: version "6.0.0" diff --git a/yarn.lock b/yarn.lock index abd5ffeab..2db7b57d2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10096,15 +10096,14 @@ raw-body@2.5.2: iconv-lite "0.4.24" unpipe "1.0.0" -react-dom@^16.13.1: - version "16.14.0" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.14.0.tgz#7ad838ec29a777fb3c75c3a190f661cf92ab8b89" - integrity sha512-1gCeQXDLoIqMgqD3IO2Ah9bnf0w9kzhwN5q4FGnHZ67hBm9yePzB5JJAIQCc8x3pFnNlwFq4RidZggNAAkzWWw== +react-dom@^17: + version "17.0.2" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-17.0.2.tgz#ecffb6845e3ad8dbfcdc498f0d0a939736502c23" + integrity sha512-s4h96KtLDUQlsENhMn1ar8t2bEa+q/YAtj8pPPdIjPDGBDIVNsrD9aXNWqspUe6AzKCIG0C1HZZLqLV7qpOBGA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.19.1" + scheduler "^0.20.2" react-fast-compare@^2.0.2: version "2.0.4" @@ -10723,10 +10722,10 @@ sax@~1.2.1: resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.4.tgz#2816234e2378bddc4e5354fab5caa895df7100d9" integrity sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw== -scheduler@^0.19.1: - version "0.19.1" - resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.19.1.tgz#4f3e2ed2c1a7d65681f4c854fa8c5a1ccb40f196" - integrity sha512-n/zwRWRYSUj0/3g/otKDRPMh6qv2SYMWNq85IEa8iZyAv8od9zDYpGSnpBEjNgcMNq6Scbu5KfIPxNF72R/2EA== +scheduler@^0.20.2: + version "0.20.2" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.20.2.tgz#4baee39436e34aa93b4874bddcbf0fe8b8b50e91" + integrity sha512-2eWfGgAqqWFGqtdMmcL5zCMK1U8KlXv8SQFGglL3CEtd0aDVDWgeF/YoCmvln55m5zSk3J/20hTaSBeSObsQDQ== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1"