From 3ed2d853180db7325ab090ebdd4b2ef1096d20b4 Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Mon, 1 Mar 2021 15:44:04 -0500 Subject: [PATCH] feat!: Make DropdownAPI consistent and fix keyboard handling (#843) BREAKING CHANGE: Dropdown does not inject props or accept a children render function (it just works) --- src/Dropdown.tsx | 71 +++++----- src/DropdownMenu.tsx | 94 ++++++------- src/DropdownToggle.tsx | 30 +++-- src/mergeOptionsWithPopperConfig.ts | 4 +- test/DropdownSpec.js | 199 ++++++++++++++-------------- www/gatsby-config.js | 1 + www/src/examples/Dropdown.mdx | 104 ++++++++++----- 7 files changed, 275 insertions(+), 228 deletions(-) diff --git a/src/Dropdown.tsx b/src/Dropdown.tsx index aa60594e..9d5e9da1 100644 --- a/src/Dropdown.tsx +++ b/src/Dropdown.tsx @@ -4,8 +4,8 @@ import React, { useCallback, useRef, useEffect, useMemo } from 'react'; import PropTypes from 'prop-types'; import { useUncontrolledProp } from 'uncontrollable'; import usePrevious from '@restart/hooks/usePrevious'; -import useCallbackRef from '@restart/hooks/useCallbackRef'; import useForceUpdate from '@restart/hooks/useForceUpdate'; +import useGlobalListener from '@restart/hooks/useGlobalListener'; import useEventCallback from '@restart/hooks/useEventCallback'; import DropdownContext, { DropDirection } from './DropdownContext'; @@ -24,7 +24,7 @@ const propTypes = { * }, * }) => React.Element} */ - children: PropTypes.func.isRequired, + children: PropTypes.node, /** * Determines the direction and location of the Menu in relation to it's Toggle. @@ -90,10 +90,24 @@ export interface DropdownProps { alignEnd?: boolean; defaultShow?: boolean; show?: boolean; - onToggle: (nextShow: boolean, event?: React.SyntheticEvent) => void; + onToggle: (nextShow: boolean, event?: React.SyntheticEvent | Event) => void; itemSelector?: string; focusFirstItemOnShow?: false | true | 'keyboard'; - children: (arg: { props: DropdownInjectedProps }) => React.ReactNode; + children: React.ReactNode; +} + +function useRefWithUpdate() { + const forceUpdate = useForceUpdate(); + const ref = useRef(null); + const attachRef = useCallback( + (element: null | HTMLElement) => { + ref.current = element; + // ensure that a menu set triggers an update for consumers + forceUpdate(); + }, + [forceUpdate], + ); + return [ref, attachRef] as const; } /** @@ -109,39 +123,30 @@ function Dropdown({ focusFirstItemOnShow, children, }: DropdownProps) { - const forceUpdate = useForceUpdate(); const [show, onToggle] = useUncontrolledProp( rawShow, defaultShow!, rawOnToggle, ); - const [toggleElement, setToggle] = useCallbackRef(); - // We use normal refs instead of useCallbackRef in order to populate the // the value as quickly as possible, otherwise the effect to focus the element // may run before the state value is set - const menuRef = useRef(null); + const [menuRef, setMenu] = useRefWithUpdate(); const menuElement = menuRef.current; - const setMenu = useCallback( - (ref: null | HTMLElement) => { - menuRef.current = ref; - // ensure that a menu set triggers an update for consumers - forceUpdate(); - }, - [forceUpdate], - ); + const [toggleRef, setToggle] = useRefWithUpdate(); + const toggleElement = toggleRef.current; const lastShow = usePrevious(show); const lastSourceEvent = useRef(null); const focusInDropdown = useRef(false); const toggle = useCallback( - (event) => { - onToggle(!show, event); + (nextShow: boolean, event?: Event | React.SyntheticEvent) => { + onToggle(nextShow, event); }, - [onToggle, show], + [onToggle], ); const context = useMemo( @@ -223,20 +228,21 @@ function Dropdown({ return items[index]; }; - const handleKeyDown = (event: React.KeyboardEvent) => { + useGlobalListener('keydown', (event: KeyboardEvent) => { const { key } = event; const target = event.target as HTMLElement; + const fromMenu = menuRef.current?.contains(target); + const fromToggle = toggleRef.current?.contains(target); + // Second only to https://github.com/twbs/bootstrap/blob/8cfbf6933b8a0146ac3fbc369f19e520bd1ebdac/js/src/dropdown.js#L400 // in inscrutability const isInput = /input|textarea/i.test(target.tagName); - if ( - isInput && - (key === ' ' || - (key !== 'Escape' && - menuRef.current && - menuRef.current.contains(target))) - ) { + if (isInput && (key === ' ' || (key !== 'Escape' && fromMenu))) { + return; + } + + if (!fromMenu && !fromToggle) { return; } @@ -253,7 +259,7 @@ function Dropdown({ case 'ArrowDown': event.preventDefault(); if (!show) { - toggle(event); + onToggle(true, event); } else { const next = getNextFocusedChild(target, 1); if (next && next.focus) next.focus(); @@ -261,15 +267,20 @@ function Dropdown({ return; case 'Escape': case 'Tab': + if (key === 'Escape') { + event.preventDefault(); + event.stopPropagation(); + } + onToggle(false, event); break; default: } - }; + }); return ( - {children({ props: { onKeyDown: handleKeyDown } })} + {children} ); } diff --git a/src/DropdownMenu.tsx b/src/DropdownMenu.tsx index 2de0afab..17aef1c6 100644 --- a/src/DropdownMenu.tsx +++ b/src/DropdownMenu.tsx @@ -1,14 +1,20 @@ import PropTypes from 'prop-types'; import React, { useContext, useRef } from 'react'; import useCallbackRef from '@restart/hooks/useCallbackRef'; -import DropdownContext from './DropdownContext'; -import usePopper, { UsePopperOptions, Placement, Offset } from './usePopper'; +import DropdownContext, { DropdownContextValue } from './DropdownContext'; +import usePopper, { + UsePopperOptions, + Placement, + Offset, + UsePopperState, +} from './usePopper'; import useRootClose, { RootCloseOptions } from './useRootClose'; import mergeOptionsWithPopperConfig from './mergeOptionsWithPopperConfig'; export interface UseDropdownMenuOptions { flip?: boolean; show?: boolean; + fixed?: boolean; alignEnd?: boolean; usePopper?: boolean; offset?: Offset; @@ -16,22 +22,24 @@ export interface UseDropdownMenuOptions { popperConfig?: Omit; } -export interface UseDropdownMenuValue { +export type UserDropdownMenuProps = Record & { + ref: React.RefCallback; + style?: React.CSSProperties; + 'aria-labelledby'?: string; +}; + +export type UserDropdownMenuArrowProps = Record & { + ref: React.RefCallback; + style: React.CSSProperties; +}; + +export interface UseDropdownMenuMetadata { show: boolean; alignEnd?: boolean; hasShown: boolean; - close: (e: Event) => void; - update: () => void; - forceUpdate: () => void; - props: Record & { - ref: React.RefCallback; - style?: React.CSSProperties; - 'aria-labelledby'?: string; - }; - arrowProps: Record & { - ref: React.RefCallback; - style: React.CSSProperties; - }; + toggle?: DropdownContextValue['toggle']; + popper: UsePopperState | null; + arrowProps: Partial; } const noop: any = () => {}; @@ -57,11 +65,12 @@ export function useDropdownMenu(options: UseDropdownMenuOptions = {}) { flip, offset, rootCloseEvent, + fixed = false, popperConfig = {}, usePopper: shouldUsePopper = !!context, } = options; - const show = context?.show == null ? options.show : context.show; + const show = context?.show == null ? !!options.show : context.show; const alignEnd = context?.alignEnd == null ? options.alignEnd : context.alignEnd; @@ -80,7 +89,7 @@ export function useDropdownMenu(options: UseDropdownMenuOptions = {}) { else if (drop === 'right') placement = alignEnd ? 'right-end' : 'right-start'; else if (drop === 'left') placement = alignEnd ? 'left-end' : 'left-start'; - const { styles, attributes, ...popper } = usePopper( + const popper = usePopper( toggleElement, menuElement, mergeOptionsWithPopperConfig({ @@ -89,50 +98,40 @@ export function useDropdownMenu(options: UseDropdownMenuOptions = {}) { enableEvents: show, offset, flip, + fixed, arrowElement, popperConfig, }), ); - let menu: Partial; - - const menuProps = { + const menuProps: UserDropdownMenuProps = { ref: setMenu || noop, 'aria-labelledby': toggleElement?.id, + ...popper.attributes.popper, + style: popper.styles.popper as any, }; - const childArgs = { + const metadata: UseDropdownMenuMetadata = { show, alignEnd, hasShown: hasShownRef.current, - close: handleClose, + toggle: context?.toggle, + popper: shouldUsePopper ? popper : null, + arrowProps: shouldUsePopper + ? { + ref: attachArrowRef, + ...popper.attributes.arrow, + style: popper.styles.arrow as any, + } + : {}, }; - if (!shouldUsePopper) { - menu = { ...childArgs, props: menuProps }; - } else { - menu = { - ...popper, - ...childArgs, - props: { - ...menuProps, - ...attributes.popper, - style: styles.popper as any, - }, - arrowProps: { - ref: attachArrowRef, - ...attributes.arrow, - style: styles.arrow as any, - }, - }; - } - useRootClose(menuElement, handleClose, { clickTrigger: rootCloseEvent, - disabled: !(menu && show), + disabled: !show, }); - return menu as UseDropdownMenuValue; + return [menuProps, metadata] as const; } const propTypes = { @@ -199,7 +198,10 @@ const defaultProps = { }; export interface DropdownMenuProps extends UseDropdownMenuOptions { - children: (args: UseDropdownMenuValue) => React.ReactNode; + children: ( + props: UserDropdownMenuProps, + meta: UseDropdownMenuMetadata, + ) => React.ReactNode; } /** @@ -209,9 +211,9 @@ export interface DropdownMenuProps extends UseDropdownMenuOptions { * @memberOf Dropdown */ function DropdownMenu({ children, ...options }: DropdownMenuProps) { - const args = useDropdownMenu(options); + const [props, meta] = useDropdownMenu(options); - return <>{args.hasShown ? children(args) : null}; + return <>{meta.hasShown ? children(props, meta) : null}; } DropdownMenu.displayName = 'ReactOverlaysDropdownMenu'; diff --git a/src/DropdownToggle.tsx b/src/DropdownToggle.tsx index 92469b70..5793c0ae 100644 --- a/src/DropdownToggle.tsx +++ b/src/DropdownToggle.tsx @@ -1,14 +1,15 @@ import PropTypes from 'prop-types'; -import React, { useContext } from 'react'; +import React, { useContext, useCallback } from 'react'; import DropdownContext, { DropdownContextValue } from './DropdownContext'; export interface UseDropdownToggleProps { ref: DropdownContextValue['setToggle']; + onClick: React.MouseEventHandler; 'aria-haspopup': boolean; 'aria-expanded': boolean; } -export interface UseDropdownToggleHelpers { +export interface UseDropdownToggleMetadata { show: DropdownContextValue['show']; toggle: DropdownContextValue['toggle']; } @@ -23,13 +24,21 @@ const noop = () => {}; */ export function useDropdownToggle(): [ UseDropdownToggleProps, - UseDropdownToggleHelpers, + UseDropdownToggleMetadata, ] { const { show = false, toggle = noop, setToggle } = useContext(DropdownContext) || {}; + const handleClick = useCallback( + (e) => { + toggle(!show, e); + }, + [show, toggle], + ); + return [ { ref: setToggle || noop, + onClick: handleClick, 'aria-haspopup': true, 'aria-expanded': !!show, }, @@ -58,7 +67,8 @@ const propTypes = { export interface DropdownToggleProps { children: ( - args: UseDropdownToggleHelpers & { props: UseDropdownToggleProps }, + props: UseDropdownToggleProps, + meta: UseDropdownToggleMetadata, ) => React.ReactNode; } @@ -69,17 +79,9 @@ export interface DropdownToggleProps { * @memberOf Dropdown */ function DropdownToggle({ children }: DropdownToggleProps) { - const [props, { show, toggle }] = useDropdownToggle(); + const [props, meta] = useDropdownToggle(); - return ( - <> - {children({ - show, - toggle, - props, - })} - - ); + return <>{children(props, meta)}; } DropdownToggle.displayName = 'ReactOverlaysDropdownToggle'; diff --git a/src/mergeOptionsWithPopperConfig.ts b/src/mergeOptionsWithPopperConfig.ts index 4f44a0d7..b9889111 100644 --- a/src/mergeOptionsWithPopperConfig.ts +++ b/src/mergeOptionsWithPopperConfig.ts @@ -2,7 +2,7 @@ import { UsePopperOptions, Offset, Placement, Modifiers } from './usePopper'; export type Config = { flip?: boolean; - show?: boolean; + fixed?: boolean; alignEnd?: boolean; enabled?: boolean; containerPadding?: number; @@ -41,6 +41,7 @@ export default function mergeOptionsWithPopperConfig({ placement, flip, offset, + fixed, containerPadding, arrowElement, popperConfig = {}, @@ -51,6 +52,7 @@ export default function mergeOptionsWithPopperConfig({ ...popperConfig, placement, enabled, + strategy: fixed ? 'fixed' : popperConfig.strategy, modifiers: toModifierArray({ ...modifiers, eventListeners: { diff --git a/test/DropdownSpec.js b/test/DropdownSpec.js index 9340474b..4432d02a 100644 --- a/test/DropdownSpec.js +++ b/test/DropdownSpec.js @@ -15,20 +15,21 @@ describe('', () => { }) => ( - {(args) => { - renderSpy && renderSpy(args); - const { show, close, props: menuProps } = args; + {(menuProps, meta) => { + const { show, toggle } = meta; + renderSpy && renderSpy(meta); + return (
toggle(false)} style={{ display: show ? 'flex' : 'none' }} /> ); @@ -38,14 +39,13 @@ describe('', () => { const Toggle = (props) => ( - {({ toggle, props: toggleProps }) => ( + {(toggleProps) => ( - - - - - - )} -
+ {children || ( + <> + Child Title, + + + + + + + )} ); + let focusableContainer; + + beforeEach(() => { + focusableContainer = document.createElement('div'); + document.body.appendChild(focusableContainer); + }); + + afterEach(() => { + ReactDOM.unmountComponentAtNode(focusableContainer); + document.body.removeChild(focusableContainer); + }); + it('renders toggle with Dropdown.Toggle', () => { const buttonNode = mount() .assertSingle('button.toggle') @@ -84,8 +92,8 @@ describe('', () => { }); it('forwards alignEnd to menu', () => { - const renderSpy = sinon.spy((args) => { - args.alignEnd.should.equal(true); + const renderSpy = sinon.spy((meta) => { + meta.alignEnd.should.equal(true); }); mount( @@ -108,6 +116,7 @@ describe('', () => { wrapper.assertSingle('ReactOverlaysDropdown'); wrapper.assertSingle('div[data-show=true]'); + wrapper.assertSingle('button[aria-expanded=true]').simulate('click'); wrapper.assertNone('.show'); @@ -135,15 +144,13 @@ describe('', () => { const wrapper = mount( - {() => ( -
- Child Title, - - - - -
- )} +
+ Child Title, + + + + +
, ); @@ -160,11 +167,13 @@ describe('', () => { }); it('when focused and closed toggles open when the key "down" is pressed', () => { - const wrapper = mount(); + const wrapper = mount(, { attachTo: focusableContainer }); - wrapper.find('.toggle').simulate('keyDown', { key: 'ArrowDown' }); + simulant.fire(wrapper.find('.toggle').getDOMNode(), 'keydown', { + key: 'ArrowDown', + }); - wrapper.assertSingle('ReactOverlaysDropdownMenu div'); + wrapper.update().assertSingle('ReactOverlaysDropdownMenu div'); }); it('closes when item is clicked', () => { @@ -207,29 +216,15 @@ describe('', () => { }); describe('focusable state', () => { - let focusableContainer; - - beforeEach(() => { - focusableContainer = document.createElement('div'); - document.body.appendChild(focusableContainer); - }); - - afterEach(() => { - ReactDOM.unmountComponentAtNode(focusableContainer); - document.body.removeChild(focusableContainer); - }); - it('when focus should not be moved to first item when focusFirstItemOnShow is `false`', () => { const wrapper = mount( - {({ props }) => ( -
- Child Title, - - - -
- )} +
+ Child Title, + + + +
, { attachTo: focusableContainer }, ); @@ -244,40 +239,39 @@ describe('', () => { it('when focused and closed sets focus on first menu item when the key "down" is pressed for role="menu"', () => { const wrapper = mount( - {({ props }) => ( -
- Child Title, - - - - -
- )} +
+ Child Title, + + + + +
, { attachTo: focusableContainer }, ); - wrapper.find('.toggle').getDOMNode().focus(); + const toggle = wrapper.find('.toggle').getDOMNode(); + toggle.focus(); - wrapper.find('.toggle').simulate('keyDown', { key: 'ArrowDown' }); + simulant.fire(toggle, 'keydown', { + key: 'ArrowDown', + }); document.activeElement.should.equal( - wrapper.find('.menu > button').first().getDOMNode(), + wrapper.update().find('.menu > button').first().getDOMNode(), ); }); it('when focused and closed sets focus on first menu item when the focusFirstItemOnShow is true', () => { const wrapper = mount( - {({ props }) => ( -
- Child Title, - - - - -
- )} +
+ Child Title, + + + + +
, { attachTo: focusableContainer }, ); @@ -296,39 +290,42 @@ describe('', () => { attachTo: focusableContainer, }); - const firstItem = wrapper.find('.menu > button').first(); + const firstItem = wrapper.find('.menu > button').first().getDOMNode(); - firstItem.getDOMNode().focus(); - document.activeElement.should.equal(firstItem.getDOMNode()); + firstItem.focus(); + document.activeElement.should.equal(firstItem); - firstItem.simulate('keyDown', { key: 'Escape' }); + act(() => { + simulant.fire(firstItem, 'keydown', { + key: 'Escape', + }); + }); - document.activeElement.should.equal(wrapper.find('.toggle').getDOMNode()); + console.log(document.activeElement); + document.activeElement.should.equal( + wrapper.update().find('.toggle').getDOMNode(), + ); }); - it('when open and the key "tab" is pressed the menu is closed and focus is progress to the next focusable element', (done) => { + it('when open and the key "tab" is pressed the menu is closed and focus is progress to the next focusable element', () => { const wrapper = mount(
, - focusableContainer, + { attachTo: focusableContainer }, ); - // Need to use Container instead of div above to make instance a composite - // element, to make this call legal. + const toggle = wrapper.find('.toggle').getDOMNode(); - wrapper.find('.toggle').simulate('keyDown', { key: 'Tab' }); + toggle.focus(); - setTimeout(() => { - wrapper - .find('.toggle') - .getDOMNode() - .getAttribute('aria-expanded') - .should.equal('false'); - done(); + simulant.fire(toggle, 'keydown', { + key: 'Tab', }); + toggle.getAttribute('aria-expanded').should.equal('false'); + // simulating a tab event doesn't actually shift focus. // at least that seems to be the case according to SO. // hence no assert on the input having focus. @@ -351,15 +348,13 @@ describe('', () => { mount( - {() => ( -
- Child Title - - - - -
- )} +
+ Child Title + + + + +
, ); diff --git a/www/gatsby-config.js b/www/gatsby-config.js index 96346c40..1ccba857 100644 --- a/www/gatsby-config.js +++ b/www/gatsby-config.js @@ -28,6 +28,7 @@ module.exports = { css: require.resolve('./src/css'), styled: '@emotion/styled', injectCss: require.resolve('./src/injectCss'), + ReactDOM: 'react-dom', }, }, }, diff --git a/www/src/examples/Dropdown.mdx b/www/src/examples/Dropdown.mdx index dbfdf2ad..f3ab5895 100644 --- a/www/src/examples/Dropdown.mdx +++ b/www/src/examples/Dropdown.mdx @@ -7,46 +7,40 @@ built from three base components, you should compose to build your Dropdowns. - `Dropdown.Toggle`: generally a button that triggers the menu opening - `Dropdown.Menu`: the overlaid, menu, positioned to the toggle with PopperJS -```jsx renderAsComponent +```jsx import { useDropdownMenu, useDropdownToggle, Dropdown, } from "react-overlays"; -const MenuContainer = styled("div")` - display: ${(p) => (p.show ? "flex" : "none")}; - min-width: 150px; - position: absolute; - z-index: 1000; - flex-direction: column; - border: 1px solid #e5e5e5; - background-color: white; - box-shadow: 0 5px 15px rgba(0, 0, 0, 0.5); -`; - const Menu = ({ role }) => { - const { show, onClose, props } = useDropdownMenu({ + const [props, { toggle, show }] = useDropdownMenu({ flip: true, offset: [0, 8], }); + const display = show ? "flex" : "none"; return ( - +
- +
); }; @@ -58,7 +52,6 @@ const Toggle = ({ id, children }) => { className="btn" id={id} {...props} - onClick={toggle} > {children} @@ -80,12 +73,10 @@ const DropdownButton = ({ alignEnd={alignEnd} itemSelector="button:not(:disabled)" > - {({ props }) => ( -
- {title} - -
- )} + + {title} + + ); @@ -95,17 +86,60 @@ const ButtonToolbar = styled("div")` } `; -const [show, setShow] = useState(false); +function DropdownExample() { + const [show, setShow] = useState(false); - - setShow(nextShow)} - title={`${show ? "Close" : "Open"} Dropdown`} - /> - + return ( + + setShow(nextShow)} + title={`${show ? "Close" : "Open"} Dropdown`} + /> + + + + + + ); +} - - -; +; +``` + +## Different containers + +Dropdowns use `PopperJS` by default to position Menu's to Toggles. PopperJS is a +powerful positioning library that lets you easily construct Dropdown markup to suit +your app's needs. + +The example here positions the Menu to the `document` body via a Portal. + +```js +import { Dropdown } from "react-overlays"; + + + + {(props) => ( + + )} + + + {(props, { show }) => + ReactDOM.createPortal( +
+

I am rendered into the document body

+
, + document.body + ) + } + +; ```