From 3f14839c3343f627779bc6285cb2e074e64d6462 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 30 Aug 2021 13:48:49 +0200 Subject: [PATCH] Warn instead of error when there are no focusable elements (#775) * warn instead of error when there are no focusable elements * update changelog Co-authored-by: Krystof Rehacek --- CHANGELOG.md | 2 ++ .../components/focus-trap/focus-trap.test.tsx | 25 +++++++------- .../src/hooks/use-focus-trap.ts | 2 +- .../components/focus-trap/focus-trap.test.ts | 34 ++++++++----------- .../src/hooks/use-focus-trap.ts | 2 +- 5 files changed, 30 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 296b954..b804164 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Only add `type=button` to real buttons ([#709](https://github.com/tailwindlabs/headlessui/pull/709)) - Fix `escape` bug not closing Dialog after clicking in Dialog ([#754](https://github.com/tailwindlabs/headlessui/pull/754)) +- Use `console.warn` instead of throwing an error when there are no focusable elements ([#775](https://github.com/tailwindlabs/headlessui/pull/775)) ## [Unreleased - Vue] @@ -19,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Only add `type=button` to real buttons ([#709](https://github.com/tailwindlabs/headlessui/pull/709)) - Add Vue emit types ([#679](https://github.com/tailwindlabs/headlessui/pull/679), [#712](https://github.com/tailwindlabs/headlessui/pull/712)) - Fix `escape` bug not closing Dialog after clicking in Dialog ([#754](https://github.com/tailwindlabs/headlessui/pull/754)) +- Use `console.warn` instead of throwing an error when there are no focusable elements ([#775](https://github.com/tailwindlabs/headlessui/pull/775)) ## [@headlessui/react@v1.4.0] - 2021-07-29 diff --git a/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx b/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx index f0de0f3..58c2c52 100644 --- a/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx +++ b/packages/@headlessui-react/src/components/focus-trap/focus-trap.test.tsx @@ -62,20 +62,19 @@ it('should focus the initialFocus element inside the FocusTrap even if another e assertActiveElement(document.getElementById('c')) }) -it( - 'should error when there is no focusable element inside the FocusTrap', - suppressConsoleLogs(() => { - expect(() => { - render( - - Nothing to see here... - - ) - }).toThrowErrorMatchingInlineSnapshot( - `"There are no focusable elements inside the "` +it('should warn when there is no focusable element inside the FocusTrap', () => { + let spy = jest.spyOn(console, 'warn').mockImplementation(jest.fn()) + + function Example() { + return ( + + Nothing to see here... + ) - }) -) + } + render() + expect(spy.mock.calls[0][0]).toBe('There are no focusable elements inside the ') +}) it( 'should not be possible to programmatically escape the focus trap', diff --git a/packages/@headlessui-react/src/hooks/use-focus-trap.ts b/packages/@headlessui-react/src/hooks/use-focus-trap.ts index 2b7eb90..68b4c28 100644 --- a/packages/@headlessui-react/src/hooks/use-focus-trap.ts +++ b/packages/@headlessui-react/src/hooks/use-focus-trap.ts @@ -89,7 +89,7 @@ export function useFocusTrap( focusElement(initialFocus.current) } else { if (focusIn(container.current, Focus.First) === FocusResult.Error) { - throw new Error('There are no focusable elements inside the ') + console.warn('There are no focusable elements inside the ') } } diff --git a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts index ee70eed..8243a82 100644 --- a/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts +++ b/packages/@headlessui-vue/src/components/focus-trap/focus-trap.test.ts @@ -109,28 +109,22 @@ it('should focus the initialFocus element inside the FocusTrap even if another e assertActiveElement(document.getElementById('c')) }) -it( - 'should error when there is no focusable element inside the FocusTrap', - suppressConsoleLogs(async () => { - expect.assertions(1) +it('should warn when there is no focusable element inside the FocusTrap', async () => { + expect.assertions(1) + let spy = jest.spyOn(console, 'warn').mockImplementation(jest.fn()) - renderTemplate({ - template: html` - - Nothing to see here... - - `, - errorCaptured(err: unknown) { - expect((err as Error).message).toMatchInlineSnapshot( - `"There are no focusable elements inside the "` - ) - return false - }, - }) + renderTemplate( + html` + + Nothing to see here... + + ` + ) - await new Promise(nextTick) - }) -) + await new Promise(nextTick) + + expect(spy.mock.calls[0][0]).toBe('There are no focusable elements inside the ') +}) it( 'should not be possible to programmatically escape the focus trap', diff --git a/packages/@headlessui-vue/src/hooks/use-focus-trap.ts b/packages/@headlessui-vue/src/hooks/use-focus-trap.ts index 3236a14..e5a070e 100644 --- a/packages/@headlessui-vue/src/hooks/use-focus-trap.ts +++ b/packages/@headlessui-vue/src/hooks/use-focus-trap.ts @@ -53,7 +53,7 @@ export function useFocusTrap( } } - if (!couldFocus) throw new Error('There are no focusable elements inside the ') + if (!couldFocus) console.warn('There are no focusable elements inside the ') } previousActiveElement.value = document.activeElement as HTMLElement