From fad81c03eb09bd9ee54fdf8c5261ac26b5de456d Mon Sep 17 00:00:00 2001 From: Andy Edwards Date: Wed, 28 Aug 2024 22:13:06 -0500 Subject: [PATCH] feat: accept popupId: null in types --- package.json | 3 +- pnpm-lock.yaml | 24 ++- src/hooks.ts | 6 +- src/useEvent.ts | 1 + test/hooks.test.tsx | 357 +++++++++++++++++++++++++++++++++++++++++++- test/index.tsx | 1 + 6 files changed, 381 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 7cbc084..ddc575b 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "@types/glob": "^8.0.0", "@types/mocha": "^10.0.6", "@types/node": "^18.11.13", + "@types/sinon": "^17.0.3", "@typescript-eslint/eslint-plugin": "^7.6.0", "@typescript-eslint/parser": "^7.6.0", "@typescript-eslint/typescript-estree": "^5.46.0", @@ -81,7 +82,7 @@ "react": "^18.2.0", "react-dom": "^18.2.0", "rimraf": "^3.0.2", - "sinon": "^6.1.4", + "sinon": "^6.3.5", "typescript": "^5.1.0", "webpack": "^5.75.0", "webpack-cli": "^5.0.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 64192dc..00c3013 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -94,6 +94,9 @@ devDependencies: '@types/node': specifier: ^18.11.13 version: 18.11.13 + '@types/sinon': + specifier: ^17.0.3 + version: 17.0.3 '@typescript-eslint/eslint-plugin': specifier: ^7.6.0 version: 7.9.0(@typescript-eslint/parser@7.9.0)(eslint@8.57.0)(typescript@5.4.2) @@ -167,7 +170,7 @@ devDependencies: specifier: ^3.0.2 version: 3.0.2 sinon: - specifier: ^6.1.4 + specifier: ^6.3.5 version: 6.3.5 typescript: specifier: ^5.1.0 @@ -200,6 +203,7 @@ packages: /@babel/cli@7.24.8(@babel/core@7.25.2): resolution: {integrity: sha512-isdp+G6DpRyKc+3Gqxy2rjzgF7Zj9K0mzLNnxz+E/fgeag8qT3vVulX4gY9dGO1q0y+0lUv6V3a+uhUzMzrwXg==} engines: {node: '>=6.9.0'} + hasBin: true peerDependencies: '@babel/core': ^7.0.0-0 dependencies: @@ -569,6 +573,7 @@ packages: /@babel/node@7.20.5(@babel/core@7.25.2): resolution: {integrity: sha512-ElSr40UtumWE4fYYF1xfLP2C6b9nfS/rswK7YYpLo9HDGEXGXEAWZaGCxIirxGIDyoV0rbl6jV1LyFMQ6ZOQSA==} engines: {node: '>=6.9.0'} + hasBin: true peerDependencies: '@babel/core': ^7.0.0-0 dependencies: @@ -2678,6 +2683,16 @@ packages: '@types/node': 18.11.13 dev: true + /@types/sinon@17.0.3: + resolution: {integrity: sha512-j3uovdn8ewky9kRBG19bOwaZbexJu/XjtkHyjvUgt4xfPFz18dcORIMqnYh66Fx3Powhcr85NT5+er3+oViapw==} + dependencies: + '@types/sinonjs__fake-timers': 8.1.5 + dev: true + + /@types/sinonjs__fake-timers@8.1.5: + resolution: {integrity: sha512-mQkU2jY8jJEF7YHjHvsQO8+3ughTL1mcnn96igfhONmR+fUPSKIkefQYpSe8bsly2Ep7oQbn/6VG5/9/0qcArQ==} + dev: true + /@types/sockjs@0.3.33: resolution: {integrity: sha512-f0KEEe05NvUnat+boPTZ0dgaLZ4SfSouXUgv5noUiefG2ajgKjmETo9ZJyuqsl7dfl2aHlLJUiki6B4ZYldiiw==} dependencies: @@ -4632,6 +4647,7 @@ packages: /eslint-config-prettier@7.2.0(eslint@8.57.0): resolution: {integrity: sha512-rV4Qu0C3nfJKPOAhFujFxB7RMP+URFyQqqOZW9DMRD7ZDTFyjaIlETU3xzHELt++4ugC0+Jm084HQYkkJe+Ivg==} + hasBin: true peerDependencies: eslint: '>=7.0.0' dependencies: @@ -4640,6 +4656,7 @@ packages: /eslint-config-prettier@8.10.0(eslint@8.57.0): resolution: {integrity: sha512-SM8AMJdeQqRYT9O9zguiruQZaN7+z+E4eAP9oiLNGKMtomwaB1E9dcgUD6ZAn/eQAb52USbvezbiljfZUhbJcg==} + hasBin: true peerDependencies: eslint: '>=7.0.0' dependencies: @@ -8826,6 +8843,7 @@ packages: /update-browserslist-db@1.0.10(browserslist@4.21.4): resolution: {integrity: sha512-OztqDenkfFkbSG+tRxBeAnCVPckDBcvibKd35yDONx6OU8N7sqgwc7rCbkJ/WcYtVRZ4ba68d6byhC21GFh7sQ==} + hasBin: true peerDependencies: browserslist: '>= 4.21.0' dependencies: @@ -8836,6 +8854,7 @@ packages: /update-browserslist-db@1.1.0(browserslist@4.23.3): resolution: {integrity: sha512-EdRAaAyk2cUE1wOf2DkEhzxqOQvFOoRJFNS6NeyJ01Gp2beMRpBAINjM2iDXE3KCuKhwnvHIQCJm6ThL2Z+HzQ==} + hasBin: true peerDependencies: browserslist: '>= 4.21.0' dependencies: @@ -8927,6 +8946,7 @@ packages: /webpack-cli@5.0.1(webpack-dev-server@4.11.1)(webpack@5.75.0): resolution: {integrity: sha512-S3KVAyfwUqr0Mo/ur3NzIp6jnerNpo7GUO6so51mxLi1spqsA17YcMXy0WOIJtBSnj748lthxC6XLbNKh/ZC+A==} engines: {node: '>=14.15.0'} + hasBin: true peerDependencies: '@webpack-cli/generators': '*' webpack: 5.x.x @@ -8974,6 +8994,7 @@ packages: /webpack-dev-server@4.11.1(webpack-cli@5.0.1)(webpack@5.75.0): resolution: {integrity: sha512-lILVz9tAUy1zGFwieuaQtYiadImb5M3d+H+L1zDYalYoDl0cksAB1UNyuE5MMWJrG6zR1tXkCP2fitl7yoUJiw==} engines: {node: '>= 12.13.0'} + hasBin: true peerDependencies: webpack: ^4.37.0 || ^5.0.0 webpack-cli: '*' @@ -9035,6 +9056,7 @@ packages: /webpack@5.75.0(webpack-cli@5.0.1): resolution: {integrity: sha512-piaIaoVJlqMsPtX/+3KTTO6jfvrSYgauFVdt8cr9LTHKmcq/AMd4mhzsiP7ZF/PGRNPGA8336jldh9l2Kt2ogQ==} engines: {node: '>=10.13.0'} + hasBin: true peerDependencies: webpack-cli: '*' peerDependenciesMeta: diff --git a/src/hooks.ts b/src/hooks.ts index fa27c47..432e8d4 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -26,7 +26,7 @@ export type Variant = 'popover' | 'popper' | 'dialog' export type PopupState = { open: (eventOrAnchorEl?: SyntheticEvent | Element | null) => void - close: () => void + close: (eventOrAnchorEl?: SyntheticEvent | Element | null) => void toggle: (eventOrAnchorEl?: SyntheticEvent | Element | null) => void onBlur: (event: FocusEvent) => void onMouseLeave: (event: MouseEvent) => void @@ -82,7 +82,7 @@ export function usePopupState({ disableAutoFocus, }: { parentPopupState?: PopupState | null | undefined - popupId?: string + popupId?: string | null variant: Variant disableAutoFocus?: boolean | null | undefined }): PopupState { @@ -281,7 +281,7 @@ export function usePopupState({ const popupState: PopupState = { ...state, setAnchorEl, - popupId, + popupId: popupId ?? undefined, variant, open, close, diff --git a/src/useEvent.ts b/src/useEvent.ts index faa16be..3b370fc 100644 --- a/src/useEvent.ts +++ b/src/useEvent.ts @@ -1,6 +1,7 @@ import * as React from 'react' export function useEvent any>(handler: Fn): Fn { + // istanbul ignore next if (typeof window === 'undefined') { // useLayoutEffect doesn't work on the server side, don't bother // trying to make callback functions stable diff --git a/test/hooks.test.tsx b/test/hooks.test.tsx index e692856..197499f 100644 --- a/test/hooks.test.tsx +++ b/test/hooks.test.tsx @@ -9,7 +9,16 @@ import { screen, waitForOptions, } from '@testing-library/react' -import { Button, Input, Popper, Popover, Menu, MenuItem } from '@mui/material' +import { + Button, + Input, + Popper, + Popover, + Menu, + MenuItem, + Dialog, + DialogTitle, +} from '@mui/material' import { usePopupState, anchorRef, @@ -22,8 +31,12 @@ import { bindHover, bindContextMenu, PopupState, + bindDoubleClick, + bindDialog, } from '../src/hooks' import { afterEach, beforeEach, describe, it } from 'mocha' +import sinon from 'sinon' +import HoverMenu from '../src/HoverMenu' const waitForTruthy = (cb: () => any, opts?: waitForOptions) => waitFor(() => { @@ -33,7 +46,22 @@ const waitForTruthy = (cb: () => any, opts?: waitForOptions) => /* eslint-disable react/jsx-handler-names */ -afterEach(cleanup) +let consoleError: sinon.SinonSpy< + Parameters<(typeof console)['error']>, + void + // eslint-disable-next-line no-console +> = console.error as any + +beforeEach(() => { + sinon.spy(console, 'error') + // eslint-disable-next-line no-console + consoleError = console.error as any +}) + +afterEach(() => { + cleanup() + consoleError.restore() +}) describe('usePopupState', () => { describe('bindMenu/bindTrigger', () => { @@ -45,8 +73,12 @@ describe('usePopupState', () => { beforeEach(() => (popupStates.length = 0)) - const MenuTest = (): React.ReactElement => { - const popupState = usePopupState({ popupId: 'menu', variant: 'popover' }) + const MenuTest = ({ + popupId = null, + }: { + popupId?: string | null + }): React.ReactElement => { + const popupState = usePopupState({ popupId, variant: 'popover' }) popupStates.push(popupState) return ( @@ -54,7 +86,11 @@ describe('usePopupState', () => { Open Menu - + Test @@ -63,7 +99,7 @@ describe('usePopupState', () => { } it('passes correct props to bindTrigger/bindMenu', async () => { - render() + render() assert.strictEqual(popupStates[0].isOpen, false) button = screen.getByRole('button') menu = screen.queryByTestId('menu') @@ -87,6 +123,36 @@ describe('usePopupState', () => { assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') }) + it('open/close with touch events works', async () => { + render() + assert.strictEqual(popupStates[0].isOpen, false) + button = screen.getByRole('button') + menu = screen.queryByTestId('menu') + assert.strictEqual(button.getAttribute('aria-controls'), null) + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + assert.strictEqual(menu, null) + + fireEvent.touchStart(button) + fireEvent.click(button) + await waitForTruthy(() => screen.queryByTestId('menu')) + menu = screen.getByTestId('menu') + assert.strictEqual(popupStates[popupStates.length - 1].isOpen, true) + assert.strictEqual(button.getAttribute('aria-controls'), 'menu') + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + assert.strictEqual(menu.getAttribute('id'), 'menu') + + await Promise.all([ + waitForElementToBeRemoved(menu), + (() => { + fireEvent.touchStart(screen.getByTestId('menuitem')) + fireEvent.click(screen.getByTestId('menuitem')) + })(), + ]) + assert.strictEqual(popupStates[popupStates.length - 1].isOpen, false) + assert.strictEqual(button.getAttribute('aria-controls'), null) + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + }) + it('open/close works', async () => { render() @@ -99,6 +165,21 @@ describe('usePopupState', () => { await waitForTruthy(() => popupStates[2]) assert.strictEqual(popupStates[2].isOpen, false) }) + it('open warns if not passed an anchor element', async () => { + render() + + await waitForTruthy(() => popupStates[0]) + popupStates[0].open() + popupStates[popupStates.length - 1].close() + popupStates[popupStates.length - 1].open() + + assert.deepEqual(consoleError.args, [ + [ + '[material-ui-popup-state] WARNING', + 'eventOrAnchorEl should be defined if setAnchorEl is not used', + ], + ]) + }) it('toggle works', async () => { render() @@ -174,6 +255,91 @@ describe('usePopupState', () => { assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') }) }) + describe('bindMenu/bindDoubleClick', () => { + let button + let menu + + const MenuTest = (): React.ReactElement => { + const popupState = usePopupState({ popupId: 'menu', variant: 'popover' }) + return ( + + + + + Test + + + + ) + } + + it('passes correct props to bindDoubleClick/bindMenu', async () => { + render() + button = screen.getByRole('button') + menu = screen.queryByTestId('menu') + assert.strictEqual(button.getAttribute('aria-controls'), null) + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + assert.strictEqual(menu, null) + + fireEvent.click(button) + assert.strictEqual(screen.queryByTestId('menu'), null) + fireEvent.doubleClick(button) + menu = screen.getByTestId('menu') + assert.strictEqual(button.getAttribute('aria-controls'), 'menu') + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + assert.strictEqual(menu.getAttribute('id'), 'menu') + + await Promise.all([ + waitForElementToBeRemoved(menu), + fireEvent.click(screen.getByTestId('menuitem')), + ]) + assert.strictEqual(button.getAttribute('aria-controls'), null) + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + }) + }) + + describe('bindDialog/bindTrigger', () => { + let button + let dialog + + const popupStates: PopupState[] = [] + + beforeEach(() => (popupStates.length = 0)) + + const DialogTest = (): React.ReactElement => { + const popupState = usePopupState({ variant: 'dialog' }) + popupStates.push(popupState) + return ( + + + + Test + + + ) + } + + it('passes correct props to bindTrigger/bindDialog', async () => { + render() + assert.strictEqual(popupStates[0].isOpen, false) + button = screen.getByRole('button') + dialog = screen.queryByTestId('dialog') + assert.strictEqual(dialog, null) + + fireEvent.click(button) + dialog = screen.getByTestId('dialog') + assert.strictEqual(popupStates[1].isOpen, true) + const backdrop = document.querySelector('.MuiBackdrop-root') + if (!backdrop) throw new Error(`failed to find backdrop element`) + + await Promise.all([ + waitForElementToBeRemoved(dialog), + fireEvent.click(backdrop), + ]) + assert.strictEqual(popupStates[2].isOpen, false) + }) + }) + describe('bindPopover/bindFocus', () => { let input let popover @@ -421,4 +587,183 @@ describe('usePopupState', () => { ]) }) }) + describe('cascading hover menus', function () { + const CascadingContext = React.createContext<{ + parentPopupState: PopupState | null + rootPopupState: PopupState | null + }>({ + parentPopupState: null, + rootPopupState: null, + }) + + function CascadingMenuItem({ + onClick, + ...props + }: React.ComponentProps) { + const { rootPopupState } = React.useContext(CascadingContext) + if (!rootPopupState) + throw new Error('must be used inside a CascadingMenu') + const handleClick = React.useCallback< + NonNullable['onClick']> + >( + (event) => { + rootPopupState.close(event) + if (onClick) onClick(event) + }, + [rootPopupState, onClick] + ) + + return ( + + ) + } + + function CascadingSubmenu({ + title, + popupId, + ...props + }: Omit, 'popupState'> & { + title: React.ReactNode + popupId?: string + }) { + const { parentPopupState } = React.useContext(CascadingContext) + const popupState = usePopupState({ + popupId, + variant: 'popover', + parentPopupState, + }) + return ( + + + {title} + + + + ) + } + + function CascadingMenu({ + popupState, + ...props + }: Omit, 'open'> & { + popupState: PopupState + }) { + const { rootPopupState } = React.useContext(CascadingContext) + const context = React.useMemo( + () => ({ + rootPopupState: rootPopupState || popupState, + parentPopupState: popupState, + }), + [rootPopupState, popupState] + ) + + return ( + + + + ) + } + + const CascadingHoverMenus = () => { + const popupState = usePopupState({ + popupId: 'rootMenu', + variant: 'popover', + }) + return ( +
+ + + Tea + Cake + Death + + Cheesecake + Cheesedeath + + Cake (the band) + Death Metal + + + Salad + Lobotomy + + + +
+ ) + } + + it('passes correct props to bindTrigger/bindMenu', async () => { + let menu + + render() + const button = screen.getByRole('button') + menu = document.getElementById('rootMenu') + assert.strictEqual(button.getAttribute('aria-controls'), null) + assert.strictEqual(button.getAttribute('aria-haspopup'), 'true') + assert.strictEqual(menu, null) + + fireEvent.mouseOver(button) + menu = document.getElementById('rootMenu') + assert.exists(menu) + assert.exists(screen.queryByTestId('Tea')) + assert.exists(screen.queryByTestId('Cake')) + assert.exists(screen.queryByTestId('Death')) + assert.exists(screen.queryByTestId('More Choices')) + assert.notExists(screen.queryByTestId('Cheesecake')) + + fireEvent.mouseOver(screen.getByTestId('More Choices')) + assert.exists(screen.queryByTestId('Cheesecake')) + assert.exists(screen.queryByTestId('Cheesedeath')) + assert.exists(screen.queryByTestId('Even More Choices')) + + fireEvent.mouseOver(screen.getByTestId('Even More Choices')) + assert.exists(screen.getByTestId('Cake (the band)')) + assert.exists(screen.getByTestId('Death Metal')) + + fireEvent.mouseLeave( + screen.getByTestId('Death Metal').closest('.MuiMenu-root')! + ) + await new Promise((r) => setTimeout(r, 30)) + assert.notExists(screen.queryByTestId('Cake (the band)')) + assert.notExists(screen.queryByTestId('Death Metal')) + assert.notExists(screen.queryByTestId('Cheesecake')) + assert.notExists(screen.queryByTestId('Cheesedeath')) + assert.notExists(screen.queryByTestId('Tea')) + assert.notExists(screen.queryByTestId('Cake')) + assert.notExists(screen.queryByTestId('Death')) + }) + }) }) diff --git a/test/index.tsx b/test/index.tsx index 086e923..b45c97c 100644 --- a/test/index.tsx +++ b/test/index.tsx @@ -126,6 +126,7 @@ describe('usePopupState', () => { assert.strictEqual(popupStates[2].isOpen, false) }) }) + describe('bindMenu/bindContextMenu', () => { let button let menu