diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 207ac6b735..b1217b0ac9 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [internal] add demo mode to `Menu` and `Popover` components ([#2448](https://github.com/tailwindlabs/headlessui/pull/2448)) +### Fixed + +- Ensure `FocusTrap` is only active when the given `enabled` value is `true` ([#2456](https://github.com/tailwindlabs/headlessui/pull/2456)) + ## [1.7.14] - 2023-04-12 ### Fixed diff --git a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx index 0ee2c70ce5..88a2362fd7 100644 --- a/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx +++ b/packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx @@ -1,5 +1,4 @@ import React, { - useEffect, useRef, // Types @@ -25,6 +24,7 @@ import { microTask } from '../../utils/micro-task' import { useWatch } from '../../hooks/use-watch' import { useDisposables } from '../../hooks/use-disposables' import { onDocumentReady } from '../../utils/document-ready' +import { useOnUnmount } from '../../hooks/use-on-unmount' type Containers = // Lazy resolved containers @@ -93,6 +93,7 @@ function FocusTrapFn( { ownerDocument, container, initialFocus }, Boolean(features & Features.InitialFocus) ) + useFocusLock( { ownerDocument, container, containers, previousActiveElement }, Boolean(features & Features.FocusLock) @@ -276,19 +277,11 @@ function useRestoreFocus({ ownerDocument }: { ownerDocument: Document | null }, }, [enabled]) // Restore the focus to the previous element when the component is unmounted - let trulyUnmounted = useRef(false) - useEffect(() => { - trulyUnmounted.current = false - - return () => { - trulyUnmounted.current = true - microTask(() => { - if (!trulyUnmounted.current) return + useOnUnmount(() => { + if (!enabled) return - focusElement(getRestoreElement()) - }) - } - }, []) + focusElement(getRestoreElement()) + }) } function useInitialFocus( diff --git a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx index 3292579309..05ac951891 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx @@ -2,6 +2,7 @@ import React, { createElement, useState } from 'react' import { render } from '@testing-library/react' import { Tab } from './tabs' +import { Dialog } from '../dialog/dialog' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' import { assertTabs, @@ -2881,6 +2882,69 @@ describe('Mouse interactions', () => { ) }) +describe('Composition', () => { + it( + 'should be possible to go to the next item containing a Dialog component', + suppressConsoleLogs(async () => { + render( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + + <> + + + + + Content 3 + + + + ) + + assertActiveElement(document.body) + + await press(Keys.Tab) + assertTabs({ active: 0 }) + + // Navigate to Dialog tab + await press(Keys.ArrowRight) + assertTabs({ active: 1 }) + + // Focus on to the Dialog panel + await press(Keys.Tab) + assertActiveElement(document.querySelector('[data-panel="1"]')) + + // Focus on to the Dialog trigger button + await press(Keys.Tab) + assertActiveElement(getByText('open')) + + // Focus back to the panel + await press(shift(Keys.Tab)) + assertActiveElement(document.querySelector('[data-panel="1"]')) + + // Focus back to tabs + await press(shift(Keys.Tab)) + assertTabs({ active: 1 }) + + // Navigate to the next tab + await press(Keys.ArrowRight) + assertTabs({ active: 2 }) + + // Focus on to the content panel + await press(Keys.Tab) + assertActiveElement(document.querySelector('[data-panel="2"]')) + }) + ) +}) + it( 'should trigger the `onChange` when the tab changes', suppressConsoleLogs(async () => { diff --git a/packages/@headlessui-react/src/hooks/use-on-unmount.ts b/packages/@headlessui-react/src/hooks/use-on-unmount.ts new file mode 100644 index 0000000000..926ffa6c11 --- /dev/null +++ b/packages/@headlessui-react/src/hooks/use-on-unmount.ts @@ -0,0 +1,18 @@ +import { useRef, useEffect } from 'react' +import { microTask } from '../utils/micro-task' + +export function useOnUnmount(cb: () => void) { + let trulyUnmounted = useRef(false) + useEffect(() => { + trulyUnmounted.current = false + + return () => { + trulyUnmounted.current = true + microTask(() => { + if (!trulyUnmounted.current) return + + cb() + }) + } + }, []) +} diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index a96b2fabcd..c5d04a67b5 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix memory leak in `Popover` component ([#2430](https://github.com/tailwindlabs/headlessui/pull/2430)) +- Ensure `FocusTrap` is only active when the given `enabled` value is `true` ([#2456](https://github.com/tailwindlabs/headlessui/pull/2456)) ## [1.7.13] - 2023-04-12 diff --git a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts index 434cfd89ac..bdeca30374 100644 --- a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts +++ b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.ts @@ -284,6 +284,8 @@ function useRestoreFocus( // Restore the focus when we unmount the component onUnmounted(() => { + if (!enabled.value) return + focusElement(getRestoreElement()) }) } diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.test.ts b/packages/@headlessui-vue/src/components/tabs/tabs.test.ts index 638a367c90..846deead76 100644 --- a/packages/@headlessui-vue/src/components/tabs/tabs.test.ts +++ b/packages/@headlessui-vue/src/components/tabs/tabs.test.ts @@ -1,6 +1,7 @@ import { nextTick, ref } from 'vue' import { createRenderTemplate, render } from '../../test-utils/vue-testing-library' import { TabGroup, TabList, Tab, TabPanels, TabPanel } from './tabs' +import { Dialog } from '../dialog/dialog' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' import { assertActiveElement, @@ -20,7 +21,7 @@ beforeAll(() => { afterAll(() => jest.restoreAllMocks()) -const renderTemplate = createRenderTemplate({ TabGroup, TabList, Tab, TabPanels, TabPanel }) +const renderTemplate = createRenderTemplate({ TabGroup, TabList, Tab, TabPanels, TabPanel, Dialog }) describe('safeguards', () => { it.each([ @@ -2716,3 +2717,69 @@ it('should trigger the `change` when the tab changes', async () => { expect(changes).toHaveBeenNthCalledWith(3, 1) expect(changes).toHaveBeenNthCalledWith(4, 0) }) + +describe('Composition', () => { + it( + 'should be possible to go to the next item containing a Dialog component', + suppressConsoleLogs(async () => { + renderTemplate({ + template: ` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + + + + + Content 3 + + + `, + setup: () => ({ + noop: console.log, + }), + }) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + assertTabs({ active: 0 }) + + // Navigate to Dialog tab + await press(Keys.ArrowRight) + assertTabs({ active: 1 }) + + // Focus on to the Dialog panel + await press(Keys.Tab) + assertActiveElement(document.querySelector('[data-panel="1"]')) + + // Focus on to the Dialog trigger button + await press(Keys.Tab) + assertActiveElement(getByText('open')) + + // Focus back to the panel + await press(shift(Keys.Tab)) + assertActiveElement(document.querySelector('[data-panel="1"]')) + + // Focus back to tabs + await press(shift(Keys.Tab)) + assertTabs({ active: 1 }) + + // Navigate to the next tab + await press(Keys.ArrowRight) + assertTabs({ active: 2 }) + + // Focus on to the content panel + await press(Keys.Tab) + assertActiveElement(document.querySelector('[data-panel="2"]')) + }) + ) +})