From 24725216e4e2fb9280bdf3b96583a9fe573410e4 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 20 Oct 2020 15:38:12 +0200 Subject: [PATCH] fix: outside click refocus bug (#114) * add watch script * make interactions in Vue and React consistent * re-work focus restoration When we click outside of the Menu or Listbox, we want to restore the focus to the Button, *unless* we clicked on/in an element that is focusable in itself. For example, when the Menu is open and you click in an input field, the input field should stay focused. We should also close the Menu itself at this point. * add examples with multiple elements * bump dependencies --- package.json | 4 +- packages/@headlessui-react/package.json | 2 +- .../pages/listbox/multiple-elements.tsx | 137 ++++++++++++ .../pages/menu/multiple-elements.tsx | 86 +++++++ .../playground-utils/resolve-all-examples.ts | 4 +- .../src/components/listbox/listbox.test.tsx | 33 +-- .../src/components/listbox/listbox.tsx | 22 +- .../src/components/menu/menu.test.tsx | 33 +-- .../src/components/menu/menu.tsx | 26 +-- .../src/test-utils/interactions.ts | 21 +- packages/@headlessui-vue/examples/src/App.vue | 2 +- .../components/listbox/multiple-elements.vue | 210 ++++++++++++++++++ .../src/components/menu/multiple-elements.vue | 125 +++++++++++ .../@headlessui-vue/examples/src/routes.json | 10 + packages/@headlessui-vue/package.json | 1 + .../src/components/listbox/listbox.test.tsx | 43 +--- .../src/components/listbox/listbox.ts | 44 ++-- .../src/components/menu/menu.test.tsx | 32 +-- .../src/components/menu/menu.ts | 24 +- .../src/test-utils/interactions.ts | 21 +- scripts/watch.sh | 14 ++ yarn.lock | 24 +- 22 files changed, 696 insertions(+), 222 deletions(-) create mode 100644 packages/@headlessui-react/pages/listbox/multiple-elements.tsx create mode 100644 packages/@headlessui-react/pages/menu/multiple-elements.tsx create mode 100644 packages/@headlessui-vue/examples/src/components/listbox/multiple-elements.vue create mode 100644 packages/@headlessui-vue/examples/src/components/menu/multiple-elements.vue create mode 100755 scripts/watch.sh diff --git a/package.json b/package.json index 249cfff..cbc46b8 100644 --- a/package.json +++ b/package.json @@ -34,11 +34,11 @@ "devDependencies": { "@tailwindcss/ui": "^0.6.2", "@testing-library/jest-dom": "^5.11.4", - "@types/node": "^14.11.10", + "@types/node": "^14.14.0", "husky": "^4.3.0", "lint-staged": "^10.4.2", "prismjs": "^1.22.0", - "tailwindcss": "^1.9.4", + "tailwindcss": "^1.9.5", "tsdx": "^0.14.1", "tslib": "^2.0.3", "typescript": "^3.9.7" diff --git a/packages/@headlessui-react/package.json b/packages/@headlessui-react/package.json index 29ab3ea..9d843fc 100644 --- a/packages/@headlessui-react/package.json +++ b/packages/@headlessui-react/package.json @@ -35,7 +35,7 @@ "@types/react-dom": "^16.9.8", "@popperjs/core": "^2.5.3", "@testing-library/react": "^11.1.0", - "framer-motion": "^2.9.1", + "framer-motion": "^2.9.3", "next": "9.5.5", "react": "^16.14.0", "react-dom": "^16.14.0", diff --git a/packages/@headlessui-react/pages/listbox/multiple-elements.tsx b/packages/@headlessui-react/pages/listbox/multiple-elements.tsx new file mode 100644 index 0000000..eaa0468 --- /dev/null +++ b/packages/@headlessui-react/pages/listbox/multiple-elements.tsx @@ -0,0 +1,137 @@ +import * as React from 'react' +import { Listbox } from '@headlessui/react' + +function classNames(...classes) { + return classes.filter(Boolean).join(' ') +} + +const people = [ + 'Wade Cooper', + 'Arlene Mccoy', + 'Devon Webb', + 'Tom Cook', + 'Tanya Fox', + 'Hellen Schmidt', + 'Caroline Schultz', + 'Mason Heaney', + 'Claudie Smitham', + 'Emil Schaefer', +] + +export default function Home() { + return ( +
+ + +
+ +
+ +
+
+ + +
+ ) +} + +function PeopleList() { + const [active, setActivePerson] = React.useState(people[2]) + + // Choose a random person on mount + React.useEffect(() => { + setActivePerson(people[Math.floor(Math.random() * people.length)]) + }, []) + + return ( +
+
+ { + console.log('value:', value) + setActivePerson(value) + }} + > + + Assigned to + + +
+ + + {active} + + + + + + + + +
+ + {people.map(name => ( + { + return classNames( + 'relative py-2 pl-3 cursor-default select-none pr-9 focus:outline-none', + active ? 'text-white bg-indigo-600' : 'text-gray-900' + ) + }} + > + {({ active, selected }) => ( + <> + + {name} + + {selected && ( + + + + + + )} + + )} + + ))} + +
+
+ +
+
+ ) +} diff --git a/packages/@headlessui-react/pages/menu/multiple-elements.tsx b/packages/@headlessui-react/pages/menu/multiple-elements.tsx new file mode 100644 index 0000000..d27b96c --- /dev/null +++ b/packages/@headlessui-react/pages/menu/multiple-elements.tsx @@ -0,0 +1,86 @@ +import * as React from 'react' +import { Menu } from '@headlessui/react' + +function classNames(...classes) { + return classes.filter(Boolean).join(' ') +} + +export default function Home() { + return ( +
+ + +
+
+ +
+
+ + +
+ ) +} + +function Dropdown() { + function resolveClass({ active, disabled }) { + return classNames( + 'block w-full text-left px-4 py-2 text-sm leading-5 text-gray-700', + active && 'bg-gray-100 text-gray-900', + disabled && 'cursor-not-allowed opacity-50' + ) + } + + return ( +
+ + + + Options + + + + + + + +
+

Signed in as

+

tom@example.com

+
+ +
+ + Account settings + + + {data => ( + + Support + + )} + + + New feature (soon) + + + License + +
+ +
+ + Sign out + +
+ + +
+ ) +} diff --git a/packages/@headlessui-react/playground-utils/resolve-all-examples.ts b/packages/@headlessui-react/playground-utils/resolve-all-examples.ts index 8acefca..7f500a6 100644 --- a/packages/@headlessui-react/playground-utils/resolve-all-examples.ts +++ b/packages/@headlessui-react/playground-utils/resolve-all-examples.ts @@ -23,11 +23,11 @@ export async function resolveAllExamples(...paths: string[]) { } const bucket: ExamplesType = { - name: file.name.replace(/-/g, ' ').replace(/.tsx?/g, ''), + name: file.name.replace(/-/g, ' ').replace(/\.tsx?/g, ''), path: [...paths, file.name] .join('/') .replace(/^pages/, '') - .replace(/.tsx?/g, '') + .replace(/\.tsx?/g, '') .replace(/\/+/g, '/'), } diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index 50d19e7..3394ade 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -210,7 +210,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.InvisibleUnmounted, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: false, focused: false }), + textContent: JSON.stringify({ open: false }), }) assertListbox({ state: ListboxState.InvisibleUnmounted }) @@ -219,7 +219,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.Visible, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: true, focused: false }), + textContent: JSON.stringify({ open: true }), }) assertListbox({ state: ListboxState.Visible }) }) @@ -244,7 +244,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.InvisibleUnmounted, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: false, focused: false }), + textContent: JSON.stringify({ open: false }), }) assertListbox({ state: ListboxState.InvisibleUnmounted }) @@ -253,7 +253,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.Visible, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: true, focused: false }), + textContent: JSON.stringify({ open: true }), }) assertListbox({ state: ListboxState.Visible }) }) @@ -2896,31 +2896,6 @@ describe('Mouse interactions', () => { }) ) - it('should focus the listbox when you try to focus the button again (when the listbox is already open)', async () => { - render( - - Trigger - - Option A - Option B - Option C - - - ) - - // Open listbox - await click(getListboxButton()) - - // Verify listbox is focused - assertActiveElement(getListbox()) - - // Try to Re-focus the button - getListboxButton()?.focus() - - // Verify listbox is still focused - assertActiveElement(getListbox()) - }) - it( 'should be a no-op when we click outside of a closed listbox', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 89e63f6..a4be69a 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -169,11 +169,14 @@ export function Listbox< React.useEffect(() => { function handler(event: MouseEvent) { const target = event.target as HTMLElement + const active = document.activeElement + if (listboxState !== ListboxStates.Open) return if (buttonRef.current?.contains(target)) return if (!optionsRef.current?.contains(target)) dispatch({ type: ActionTypes.CloseListbox }) - if (!event.defaultPrevented) d.nextFrame(() => buttonRef.current?.focus()) + if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element + if (!event.defaultPrevented) buttonRef.current?.focus() } window.addEventListener('click', handler) @@ -195,7 +198,7 @@ export function Listbox< // --- const DEFAULT_BUTTON_TAG = 'button' -type ButtonRenderPropArg = { open: boolean; focused: boolean } +type ButtonRenderPropArg = { open: boolean } type ButtonPropsWeControl = | 'ref' | 'id' @@ -205,8 +208,6 @@ type ButtonPropsWeControl = | 'aria-expanded' | 'aria-labelledby' | 'onKeyDown' - | 'onFocus' - | 'onBlur' | 'onPointerUp' const Button = forwardRefWithAs(function Button< @@ -217,7 +218,6 @@ const Button = forwardRefWithAs(function Button< ) { const [state, dispatch] = useListboxContext([Listbox.name, Button.name].join('.')) const buttonRef = useSyncRefs(state.buttonRef, ref) - const [focused, setFocused] = React.useState(false) const id = `headlessui-listbox-button-${useId()}` const d = useDisposables() @@ -268,20 +268,14 @@ const Button = forwardRefWithAs(function Button< [dispatch, d, state, props.disabled] ) - const handleFocus = React.useCallback(() => { - if (state.listboxState === ListboxStates.Open) return state.optionsRef.current?.focus() - setFocused(true) - }, [state, setFocused]) - - const handleBlur = React.useCallback(() => setFocused(false), [setFocused]) const labelledby = useComputed(() => { if (!state.labelRef.current) return undefined return [state.labelRef.current.id, id].join(' ') }, [state.labelRef.current, id]) const propsBag = React.useMemo( - () => ({ open: state.listboxState === ListboxStates.Open, focused }), - [state, focused] + () => ({ open: state.listboxState === ListboxStates.Open }), + [state] ) const passthroughProps = props const propsWeControl = { @@ -293,8 +287,6 @@ const Button = forwardRefWithAs(function Button< 'aria-expanded': state.listboxState === ListboxStates.Open ? true : undefined, 'aria-labelledby': labelledby, onKeyDown: handleKeyDown, - onFocus: handleFocus, - onBlur: handleBlur, onPointerUp: handlePointerUp, } diff --git a/packages/@headlessui-react/src/components/menu/menu.test.tsx b/packages/@headlessui-react/src/components/menu/menu.test.tsx index d0fb46f..a63662e 100644 --- a/packages/@headlessui-react/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.test.tsx @@ -133,7 +133,7 @@ describe('Rendering', () => { assertMenuButton({ state: MenuState.InvisibleUnmounted, attributes: { id: 'headlessui-menu-button-1' }, - textContent: JSON.stringify({ open: false, focused: false }), + textContent: JSON.stringify({ open: false }), }) assertMenu({ state: MenuState.InvisibleUnmounted }) @@ -142,7 +142,7 @@ describe('Rendering', () => { assertMenuButton({ state: MenuState.Visible, attributes: { id: 'headlessui-menu-button-1' }, - textContent: JSON.stringify({ open: true, focused: false }), + textContent: JSON.stringify({ open: true }), }) assertMenu({ state: MenuState.Visible }) }) @@ -167,7 +167,7 @@ describe('Rendering', () => { assertMenuButton({ state: MenuState.InvisibleUnmounted, attributes: { id: 'headlessui-menu-button-1' }, - textContent: JSON.stringify({ open: false, focused: false }), + textContent: JSON.stringify({ open: false }), }) assertMenu({ state: MenuState.InvisibleUnmounted }) @@ -176,7 +176,7 @@ describe('Rendering', () => { assertMenuButton({ state: MenuState.Visible, attributes: { id: 'headlessui-menu-button-1' }, - textContent: JSON.stringify({ open: true, focused: false }), + textContent: JSON.stringify({ open: true }), }) assertMenu({ state: MenuState.Visible }) }) @@ -2464,31 +2464,6 @@ describe('Mouse interactions', () => { }) ) - it('should focus the menu when you try to focus the button again (when the menu is already open)', async () => { - render( - - Trigger - - Item A - Item B - Item C - - - ) - - // Open menu - await click(getMenuButton()) - - // Verify menu is focused - assertActiveElement(getMenu()) - - // Try to Re-focus the button - getMenuButton()?.focus() - - // Verify menu is still focused - assertActiveElement(getMenu()) - }) - it( 'should be a no-op when we click outside of a closed menu', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 74f0e05..b7d186a 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -151,11 +151,14 @@ export function Menu( React.useEffect(() => { function handler(event: MouseEvent) { const target = event.target as HTMLElement + const active = document.activeElement + if (menuState !== MenuStates.Open) return if (buttonRef.current?.contains(target)) return if (!itemsRef.current?.contains(target)) dispatch({ type: ActionTypes.CloseMenu }) - if (!event.defaultPrevented) d.nextFrame(() => buttonRef.current?.focus()) + if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element + if (!event.defaultPrevented) buttonRef.current?.focus() } window.addEventListener('click', handler) @@ -174,7 +177,7 @@ export function Menu( // --- const DEFAULT_BUTTON_TAG = 'button' -type ButtonRenderPropArg = { open: boolean; focused: boolean } +type ButtonRenderPropArg = { open: boolean } type ButtonPropsWeControl = | 'ref' | 'id' @@ -183,8 +186,6 @@ type ButtonPropsWeControl = | 'aria-controls' | 'aria-expanded' | 'onKeyDown' - | 'onFocus' - | 'onBlur' | 'onPointerUp' const Button = forwardRefWithAs(function Button< @@ -195,7 +196,6 @@ const Button = forwardRefWithAs(function Button< ) { const [state, dispatch] = useMenuContext([Menu.name, Button.name].join('.')) const buttonRef = useSyncRefs(state.buttonRef, ref) - const [focused, setFocused] = React.useState(false) const id = `headlessui-menu-button-${useId()}` const d = useDisposables() @@ -244,17 +244,7 @@ const Button = forwardRefWithAs(function Button< [dispatch, d, state, props.disabled] ) - const handleFocus = React.useCallback(() => { - if (state.menuState === MenuStates.Open) state.itemsRef.current?.focus() - setFocused(true) - }, [state, setFocused]) - - const handleBlur = React.useCallback(() => setFocused(false), [setFocused]) - - const propsBag = React.useMemo(() => ({ open: state.menuState === MenuStates.Open, focused }), [ - state, - focused, - ]) + const propsBag = React.useMemo(() => ({ open: state.menuState === MenuStates.Open }), [state]) const passthroughProps = props const propsWeControl = { ref: buttonRef, @@ -264,8 +254,6 @@ const Button = forwardRefWithAs(function Button< 'aria-controls': state.itemsRef.current?.id, 'aria-expanded': state.menuState === MenuStates.Open ? true : undefined, onKeyDown: handleKeyDown, - onFocus: handleFocus, - onBlur: handleBlur, onPointerUp: handlePointerUp, } @@ -431,7 +419,7 @@ function Item( (event: { preventDefault: Function }) => { if (disabled) return event.preventDefault() dispatch({ type: ActionTypes.CloseMenu }) - d.nextFrame(() => state.buttonRef.current?.focus()) + disposables().nextFrame(() => state.buttonRef.current?.focus()) if (onClick) return onClick(event) }, [d, dispatch, state.buttonRef, disabled, onClick] diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index 4475c4f..1fd0c30 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -1,7 +1,12 @@ import { fireEvent } from '@testing-library/react' -import { disposables } from '../utils/disposables' -const d = disposables() +function nextFrame(cb: Function): void { + setImmediate(() => + setImmediate(() => { + cb() + }) + ) +} export const Keys: Record> = { Space: { key: ' ', keyCode: 32 }, @@ -57,7 +62,7 @@ export async function type(events: Partial[]) { // We don't want to actually wait in our tests, so let's advance jest.runAllTimers() - await new Promise(d.nextFrame) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, type) throw err @@ -80,7 +85,7 @@ export async function click(element: Document | Element | Window | Node | null) fireEvent.mouseUp(element) fireEvent.click(element) - await new Promise(d.nextFrame) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, click) throw err @@ -93,7 +98,7 @@ export async function focus(element: Document | Element | Window | Node | null) fireEvent.focus(element) - await new Promise(d.nextFrame) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, focus) throw err @@ -107,7 +112,7 @@ export async function mouseEnter(element: Document | Element | Window | null) { fireEvent.pointerEnter(element) fireEvent.mouseOver(element) - await new Promise(d.nextFrame) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, mouseEnter) throw err @@ -121,7 +126,7 @@ export async function mouseMove(element: Document | Element | Window | null) { fireEvent.pointerMove(element) fireEvent.mouseMove(element) - await new Promise(d.nextFrame) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, mouseMove) throw err @@ -137,7 +142,7 @@ export async function mouseLeave(element: Document | Element | Window | null) { fireEvent.mouseOut(element) fireEvent.mouseLeave(element) - await new Promise(d.nextFrame) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, mouseLeave) throw err diff --git a/packages/@headlessui-vue/examples/src/App.vue b/packages/@headlessui-vue/examples/src/App.vue index b3362a5..9eec170 100644 --- a/packages/@headlessui-vue/examples/src/App.vue +++ b/packages/@headlessui-vue/examples/src/App.vue @@ -67,7 +67,7 @@
diff --git a/packages/@headlessui-vue/examples/src/components/listbox/multiple-elements.vue b/packages/@headlessui-vue/examples/src/components/listbox/multiple-elements.vue new file mode 100644 index 0000000..77b4892 --- /dev/null +++ b/packages/@headlessui-vue/examples/src/components/listbox/multiple-elements.vue @@ -0,0 +1,210 @@ + + + diff --git a/packages/@headlessui-vue/examples/src/components/menu/multiple-elements.vue b/packages/@headlessui-vue/examples/src/components/menu/multiple-elements.vue new file mode 100644 index 0000000..3b037a4 --- /dev/null +++ b/packages/@headlessui-vue/examples/src/components/menu/multiple-elements.vue @@ -0,0 +1,125 @@ + + + diff --git a/packages/@headlessui-vue/examples/src/routes.json b/packages/@headlessui-vue/examples/src/routes.json index 6e81dd6..4fe95c0 100644 --- a/packages/@headlessui-vue/examples/src/routes.json +++ b/packages/@headlessui-vue/examples/src/routes.json @@ -26,6 +26,11 @@ "name": "Menu with Popper + Transition", "path": "/menu/menu-with-transition-and-popper", "component": "./components/menu/menu-with-transition-and-popper.vue" + }, + { + "name": "Menu multiple elements", + "path": "/menu/multiple-elements", + "component": "./components/menu/multiple-elements.vue" } ] }, @@ -37,6 +42,11 @@ "name": "Listbox (basic)", "path": "/listbox/listbox", "component": "./components/listbox/listbox.vue" + }, + { + "name": "Listbox multiple elements", + "path": "/listbox/multiple-elements", + "component": "./components/listbox/multiple-elements.vue" } ] }, diff --git a/packages/@headlessui-vue/package.json b/packages/@headlessui-vue/package.json index d635a1b..da6baff 100644 --- a/packages/@headlessui-vue/package.json +++ b/packages/@headlessui-vue/package.json @@ -24,6 +24,7 @@ "playground:build": "NODE_ENV=production vite build examples", "prepublishOnly": "npm run build", "build": "../../scripts/build.sh", + "watch": "../../scripts/watch.sh", "test": "../../scripts/test.sh", "lint": "../../scripts/lint.sh" }, diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index 4417cde..ec8c4e7 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -36,6 +36,13 @@ import { jest.mock('../../hooks/use-id') +beforeAll(() => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any) + jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(clearImmediate as any) +}) + +afterAll(() => jest.restoreAllMocks()) + function renderTemplate(input: string | Partial[0]>) { const defaultComponents = { Listbox, ListboxLabel, ListboxButton, ListboxOptions, ListboxOption } @@ -233,7 +240,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.InvisibleUnmounted, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: false, focused: false }), + textContent: JSON.stringify({ open: false }), }) assertListbox({ state: ListboxState.InvisibleUnmounted }) @@ -242,7 +249,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.Visible, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: true, focused: false }), + textContent: JSON.stringify({ open: true }), }) assertListbox({ state: ListboxState.Visible }) }) @@ -268,7 +275,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.InvisibleUnmounted, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: false, focused: false }), + textContent: JSON.stringify({ open: false }), }) assertListbox({ state: ListboxState.InvisibleUnmounted }) @@ -277,7 +284,7 @@ describe('Rendering', () => { assertListboxButton({ state: ListboxState.Visible, attributes: { id: 'headlessui-listbox-button-1' }, - textContent: JSON.stringify({ open: true, focused: false }), + textContent: JSON.stringify({ open: true }), }) assertListbox({ state: ListboxState.Visible }) }) @@ -3104,34 +3111,6 @@ describe('Mouse interactions', () => { }) ) - it('should focus the listbox when you try to focus the button again (when the listbox is already open)', async () => { - renderTemplate({ - template: ` - - Trigger - - Option A - Option B - Option C - - - `, - setup: () => ({ value: ref(null) }), - }) - - // Open listbox - await click(getListboxButton()) - - // Verify listbox is focused - assertActiveElement(getListbox()) - - // Try to Re-focus the button - getListboxButton()?.focus() - - // Verify listbox is still focused - assertActiveElement(getListbox()) - }) - it( 'should be a no-op when we click outside of a closed listbox', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index ac43857..52d8a0b 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -26,6 +26,10 @@ enum ListboxStates { Closed, } +function nextFrame(cb: () => void) { + requestAnimationFrame(() => requestAnimationFrame(cb)) +} + type ListboxOptionDataRef = Ref<{ textValue: string; disabled: boolean; value: unknown }> type StateDefinition = { // State @@ -155,13 +159,15 @@ export const Listbox = defineComponent({ onMounted(() => { function handler(event: MouseEvent) { - if (listboxState.value !== ListboxStates.Open) return - if (buttonRef.value?.contains(event.target as HTMLElement)) return + const target = event.target as HTMLElement + const active = document.activeElement - if (!optionsRef.value?.contains(event.target as HTMLElement)) { - api.closeListbox() - } - if (!event.defaultPrevented) nextTick(() => buttonRef.value?.focus()) + if (listboxState.value !== ListboxStates.Open) return + if (buttonRef.value?.contains(target)) return + + if (!optionsRef.value?.contains(target)) api.closeListbox() + if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element + if (!event.defaultPrevented) buttonRef.value?.focus() } window.addEventListener('click', handler) @@ -221,7 +227,7 @@ export const ListboxButton = defineComponent({ render() { const api = useListboxContext('ListboxButton') - const slot = { open: api.listboxState.value === ListboxStates.Open, focused: this.focused } + const slot = { open: api.listboxState.value === ListboxStates.Open } const propsWeControl = { ref: 'el', id: this.id, @@ -233,8 +239,6 @@ export const ListboxButton = defineComponent({ ? [api.labelRef.value.id, this.id].join(' ') : undefined, onKeyDown: this.handleKeyDown, - onFocus: this.handleFocus, - onBlur: this.handleBlur, onPointerUp: this.handlePointerUp, } @@ -248,7 +252,6 @@ export const ListboxButton = defineComponent({ setup(props) { const api = useListboxContext('ListboxButton') const id = `headlessui-listbox-button-${useId()}` - const focused = ref(false) function handleKeyDown(event: KeyboardEvent) { switch (event.key) { @@ -284,28 +287,11 @@ export const ListboxButton = defineComponent({ } else { event.preventDefault() api.openListbox() - nextTick(() => api.optionsRef.value?.focus()) + nextFrame(() => api.optionsRef.value?.focus()) } } - function handleFocus() { - if (api.listboxState.value === ListboxStates.Open) return api.optionsRef.value?.focus() - focused.value = true - } - - function handleBlur() { - focused.value = false - } - - return { - id, - el: api.buttonRef, - focused, - handleKeyDown, - handlePointerUp, - handleFocus, - handleBlur, - } + return { id, el: api.buttonRef, handleKeyDown, handlePointerUp } }, }) diff --git a/packages/@headlessui-vue/src/components/menu/menu.test.tsx b/packages/@headlessui-vue/src/components/menu/menu.test.tsx index bd7d041..e951073 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-vue/src/components/menu/menu.test.tsx @@ -31,6 +31,13 @@ import { jest.mock('../../hooks/use-id') +beforeAll(() => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any) + jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(clearImmediate as any) +}) + +afterAll(() => jest.restoreAllMocks()) + function renderTemplate(input: string | Partial[0]>) { const defaultComponents = { Menu, MenuButton, MenuItems, MenuItem } @@ -2381,31 +2388,6 @@ describe('Mouse interactions', () => { assertMenu({ state: MenuState.InvisibleUnmounted }) }) - it('should focus the menu when you try to focus the button again (when the menu is already open)', async () => { - renderTemplate(` - - Trigger - - Item A - Item B - Item C - - - `) - - // Open menu - await click(getMenuButton()) - - // Verify menu is focused - assertActiveElement(getMenu()) - - // Try to Re-focus the button - getMenuButton()?.focus() - - // Verify menu is still focused - assertActiveElement(getMenu()) - }) - it('should be a no-op when we click outside of a closed menu', async () => { renderTemplate(` diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts index 606ce72..2cf3d5d 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.ts +++ b/packages/@headlessui-vue/src/components/menu/menu.ts @@ -21,6 +21,10 @@ enum MenuStates { Closed, } +function nextFrame(cb: () => void) { + requestAnimationFrame(() => requestAnimationFrame(cb)) +} + type MenuItemDataRef = Ref<{ textValue: string; disabled: boolean }> type StateDefinition = { // State @@ -132,11 +136,15 @@ export const Menu = defineComponent({ onMounted(() => { function handler(event: MouseEvent) { - if (menuState.value !== MenuStates.Open) return - if (buttonRef.value?.contains(event.target as HTMLElement)) return + const target = event.target as HTMLElement + const active = document.activeElement - if (!itemsRef.value?.contains(event.target as HTMLElement)) api.closeMenu() - if (!event.defaultPrevented) nextTick(() => buttonRef.value?.focus()) + if (menuState.value !== MenuStates.Open) return + if (buttonRef.value?.contains(target)) return + + if (!itemsRef.value?.contains(target)) api.closeMenu() + if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element + if (!event.defaultPrevented) buttonRef.value?.focus() } window.addEventListener('click', handler) @@ -170,7 +178,6 @@ export const MenuButton = defineComponent({ 'aria-controls': api.itemsRef.value?.id, 'aria-expanded': api.menuState.value === MenuStates.Open ? true : undefined, onKeyDown: this.handleKeyDown, - onFocus: this.handleFocus, onPointerUp: this.handlePointerUp, } @@ -219,20 +226,15 @@ export const MenuButton = defineComponent({ } else { event.preventDefault() api.openMenu() - nextTick(() => api.itemsRef.value?.focus()) + nextFrame(() => api.itemsRef.value?.focus()) } } - function handleFocus() { - if (api.menuState.value === MenuStates.Open) api.itemsRef.value?.focus() - } - return { id, el: api.buttonRef, handleKeyDown, handlePointerUp, - handleFocus, } }, }) diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index 235c6a7..f449d35 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -1,6 +1,13 @@ -import { nextTick } from 'vue' import { fireEvent } from '@testing-library/dom' +function nextFrame(cb: Function): void { + setImmediate(() => + setImmediate(() => { + cb() + }) + ) +} + export const Keys: Record> = { Space: { key: ' ', keyCode: 32 }, Enter: { key: 'Enter', keyCode: 13 }, @@ -55,7 +62,7 @@ export async function type(events: Partial[]) { // We don't want to actually wait in our tests, so let's advance jest.runAllTimers() - await new Promise(nextTick) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, type) throw err @@ -78,7 +85,7 @@ export async function click(element: Document | Element | Window | null) { fireEvent.mouseUp(element) fireEvent.click(element) - await new Promise(nextTick) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, click) throw err @@ -91,7 +98,7 @@ export async function focus(element: Document | Element | Window | null) { fireEvent.focus(element) - await new Promise(nextTick) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, focus) throw err @@ -106,7 +113,7 @@ export async function mouseEnter(element: Document | Element | Window | null) { fireEvent.pointerEnter(element) fireEvent.mouseOver(element) - await new Promise(nextTick) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, mouseEnter) throw err @@ -120,7 +127,7 @@ export async function mouseMove(element: Document | Element | Window | null) { fireEvent.pointerMove(element) fireEvent.mouseMove(element) - await new Promise(nextTick) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, mouseMove) throw err @@ -136,7 +143,7 @@ export async function mouseLeave(element: Document | Element | Window | null) { fireEvent.mouseOut(element) fireEvent.mouseLeave(element) - await new Promise(nextTick) + await new Promise(nextFrame) } catch (err) { Error.captureStackTrace(err, mouseLeave) throw err diff --git a/scripts/watch.sh b/scripts/watch.sh new file mode 100755 index 0000000..6a3b621 --- /dev/null +++ b/scripts/watch.sh @@ -0,0 +1,14 @@ +#!/bin/bash +set -e + +node="yarn node" +tsdxArgs=() + +# Add script name +tsdxArgs+=("watch" "--name" "headlessui" "--format" "cjs,esm,umd" "--tsconfig" "./tsconfig.tsdx.json") + +# Passthrough arguments and flags +tsdxArgs+=($@) + +# Execute +$node "$(yarn bin tsdx)" "${tsdxArgs[@]}" diff --git a/yarn.lock b/yarn.lock index cb8078c..9380c02 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1807,10 +1807,10 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-14.11.1.tgz#56af902ad157e763f9ba63d671c39cda3193c835" integrity sha512-oTQgnd0hblfLsJ6BvJzzSL+Inogp3lq9fGgqRkMB/ziKMgEUaFl801OncOzUmalfzt14N0oPHMK47ipl+wbTIw== -"@types/node@^14.11.10": - version "14.11.10" - resolved "https://registry.yarnpkg.com/@types/node/-/node-14.11.10.tgz#8c102aba13bf5253f35146affbf8b26275069bef" - integrity sha512-yV1nWZPlMFpoXyoknm4S56y2nlTAuFYaJuQtYRAOU7xA/FJ9RY0Xm7QOkaYMMmr8ESdHIuUb6oQgR/0+2NqlyA== +"@types/node@^14.14.0": + version "14.14.0" + resolved "https://registry.yarnpkg.com/@types/node/-/node-14.14.0.tgz#f1091b6ad5de18e8e91bdbd43ec63f13de372538" + integrity sha512-BfbIHP9IapdupGhq/hc+jT5dyiBVZ2DdeC5WwJWQWDb0GijQlzUFAeIQn/2GtvZcd2HVUU7An8felIICFTC2qg== "@types/normalize-package-data@^2.4.0": version "2.4.0" @@ -5000,10 +5000,10 @@ fragment-cache@^0.2.1: dependencies: map-cache "^0.2.2" -framer-motion@^2.9.1: - version "2.9.1" - resolved "https://registry.yarnpkg.com/framer-motion/-/framer-motion-2.9.1.tgz#86713730e9503922eedcf5e1bb91a5acc30076f5" - integrity sha512-NjEF5u1FkCTS+zRsDWOPPCxWBUWk255WXy4d1FDCC3j6ETJzDXx0V/NUPvwhzyATHbahfX5JdsHWdRe1whNHJg== +framer-motion@^2.9.3: + version "2.9.3" + resolved "https://registry.yarnpkg.com/framer-motion/-/framer-motion-2.9.3.tgz#dcdf62cb7f8a55cd8f536d2f3be5373ae785fd4d" + integrity sha512-4uoPCaSf4/TzOttJVlneKnO/A9PGdcN0YZaw372m9ejpMu8083m5AWXXENEqOBjbhOSjX2L0Vl+AYGinjcwgBg== dependencies: framesync "^4.1.0" hey-listen "^1.0.8" @@ -9797,10 +9797,10 @@ table@^5.2.3: slice-ansi "^2.1.0" string-width "^3.0.0" -tailwindcss@^1.9.4: - version "1.9.4" - resolved "https://registry.yarnpkg.com/tailwindcss/-/tailwindcss-1.9.4.tgz#5ae8ff84bc8234df22ba5f2c7feafb64bb14da55" - integrity sha512-CVeP4J1pDluBM/AF11JPku9Cx+VwQ6MbOcnlobnWVVZnq+xku8sa+XXmYzy/GvE08qD8w+OmpSdN21ZFPoVDRg== +tailwindcss@^1.9.5: + version "1.9.5" + resolved "https://registry.yarnpkg.com/tailwindcss/-/tailwindcss-1.9.5.tgz#3339b790a68bc1f09a8efd8eb94cb05aed5235c2" + integrity sha512-Je5t1fAfyW333YTpSxF+8uJwbnrkpyBskDtZYgSMMKQbNp6QUhEKJ4g/JIevZjD2Zidz9VxLraEUq/yWOx6nQg== dependencies: "@fullhuman/postcss-purgecss" "^2.1.2" autoprefixer "^9.4.5"