From c1117840fda78cd088ff28ac26305f081ce33fae Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 2 Aug 2021 13:57:58 +0200 Subject: [PATCH] Only add `type=button` for real buttons (#709) * add `{type:'button'}` only for buttons We will try and infer the type based on the passed in `props.as` prop or the default tag. However, when somebody uses `as={CustomComponent}` then we don't know what it will render. Therefore we have to pass it a ref and check if the final result is a button or not. If it is, and it doesn't have a `type` yet, then we can set the `type` correctly. * update changelog --- CHANGELOG.md | 8 +- .../components/disclosure/disclosure.test.tsx | 62 +++++++++++- .../src/components/disclosure/disclosure.tsx | 10 +- .../src/components/listbox/listbox.test.tsx | 60 ++++++++++++ .../src/components/listbox/listbox.tsx | 3 +- .../src/components/menu/menu.test.tsx | 59 +++++++++++ .../src/components/menu/menu.tsx | 3 +- .../src/components/popover/popover.test.tsx | 62 +++++++++++- .../src/components/popover/popover.tsx | 8 +- .../src/components/switch/switch.test.tsx | 60 ++++++++++++ .../src/components/switch/switch.tsx | 15 ++- .../src/components/tabs/tabs.test.tsx | 72 ++++++++++++++ .../src/components/tabs/tabs.tsx | 5 +- .../src/hooks/use-resolve-button-type.ts | 34 +++++++ .../src/hooks/use-sync-refs.ts | 2 +- .../test-utils/accessibility-assertions.ts | 4 +- .../components/disclosure/disclosure.test.ts | 94 +++++++++++++++++- .../src/components/disclosure/disclosure.ts | 33 ++++++- .../src/components/listbox/listbox.test.tsx | 97 ++++++++++++++++++- .../src/components/listbox/listbox.ts | 17 +++- .../src/components/menu/menu.test.tsx | 90 +++++++++++++++++ .../src/components/menu/menu.ts | 17 +++- .../src/components/popover/popover.test.ts | 92 +++++++++++++++++- .../src/components/popover/popover.ts | 23 ++++- .../src/components/switch/switch.test.tsx | 90 ++++++++++++++++- .../src/components/switch/switch.ts | 22 +++-- .../src/hooks/use-resolve-button-type.ts | 33 +++++++ .../test-utils/accessibility-assertions.ts | 4 +- 28 files changed, 1026 insertions(+), 53 deletions(-) create mode 100644 packages/@headlessui-react/src/hooks/use-resolve-button-type.ts create mode 100644 packages/@headlessui-vue/src/hooks/use-resolve-button-type.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c631d3..a34b450 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased - React] -- Nothing yet! +### Fixes + +- Only add `type=button` to real buttons ([#709](https://github.com/tailwindlabs/headlessui/pull/709)) ## [Unreleased - Vue] -- Nothing yet! +### Fixes + +- Only add `type=button` to real buttons ([#709](https://github.com/tailwindlabs/headlessui/pull/709)) ## [@headlessui/react@v1.4.0] - 2021-07-29 diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx index a6528a2..6c72cff 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx @@ -20,7 +20,7 @@ jest.mock('../../hooks/use-id') afterAll(() => jest.restoreAllMocks()) function nextFrame() { - return new Promise(resolve => { + return new Promise(resolve => { requestAnimationFrame(() => { requestAnimationFrame(() => { resolve() @@ -296,6 +296,66 @@ describe('Rendering', () => { assertDisclosurePanel({ state: DisclosureState.Visible }) }) ) + + describe('`type` attribute', () => { + it('should set the `type` to "button" by default', async () => { + render( + + Trigger + + ) + + expect(getDisclosureButton()).toHaveAttribute('type', 'button') + }) + + it('should not set the `type` to "button" if it already contains a `type`', async () => { + render( + + Trigger + + ) + + expect(getDisclosureButton()).toHaveAttribute('type', 'submit') + }) + + it('should set the `type` to "button" when using the `as` prop which resolves to a "button"', async () => { + let CustomButton = React.forwardRef((props, ref) => ( +