diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c631d366e..b804164bd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,20 @@ 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)) +- 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] -- Nothing yet! +### Fixes + +- 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/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index 6fa61270ef..fcdc88f0f2 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -430,6 +430,90 @@ describe('Keyboard interactions', () => { assertDialog({ state: DialogState.InvisibleUnmounted }) }) ) + + it( + 'should be possible to close the dialog with Escape, when a field is focused', + suppressConsoleLogs(async () => { + function Example() { + let [isOpen, setIsOpen] = useState(false) + return ( + <> + + + Contents + + + + + ) + } + render() + + assertDialog({ state: DialogState.InvisibleUnmounted }) + + // Open dialog + await click(document.getElementById('trigger')) + + // Verify it is open + assertDialog({ + state: DialogState.Visible, + attributes: { id: 'headlessui-dialog-1' }, + }) + + // Close dialog + await press(Keys.Escape) + + // Verify it is close + assertDialog({ state: DialogState.InvisibleUnmounted }) + }) + ) + + it( + 'should not be possible to close the dialog with Escape, when a field is focused but cancels the event', + suppressConsoleLogs(async () => { + function Example() { + let [isOpen, setIsOpen] = useState(false) + return ( + <> + + + Contents + { + event.preventDefault() + event.stopPropagation() + }} + /> + + + + ) + } + render() + + assertDialog({ state: DialogState.InvisibleUnmounted }) + + // Open dialog + await click(document.getElementById('trigger')) + + // Verify it is open + assertDialog({ + state: DialogState.Visible, + attributes: { id: 'headlessui-dialog-1' }, + }) + + // Try to close the dialog + await press(Keys.Escape) + + // Verify it is still open + assertDialog({ state: DialogState.Visible }) + }) + ) }) }) diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 2055bd4ac6..839ca69087 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -7,15 +7,14 @@ import React, { useMemo, useReducer, useRef, + useState, // Types ContextType, ElementType, MouseEvent as ReactMouseEvent, - KeyboardEvent as ReactKeyboardEvent, MutableRefObject, Ref, - useState, } from 'react' import { Props } from '../../types' @@ -217,6 +216,16 @@ let DialogRoot = forwardRefWithAs(function Dialog< close() }) + // Handle `Escape` to close + useWindowEvent('keydown', event => { + if (event.key !== Keys.Escape) return + if (dialogState !== DialogStates.Open) return + if (hasNestedDialogs) return + event.preventDefault() + event.stopPropagation() + close() + }) + // Scroll lock useEffect(() => { if (dialogState !== DialogStates.Open) return @@ -282,16 +291,6 @@ let DialogRoot = forwardRefWithAs(function Dialog< onClick(event: ReactMouseEvent) { event.stopPropagation() }, - - // Handle `Escape` to close - onKeyDown(event: ReactKeyboardEvent) { - if (event.key !== Keys.Escape) return - if (dialogState !== DialogStates.Open) return - if (hasNestedDialogs) return - event.preventDefault() - event.stopPropagation() - close() - }, } let passthroughProps = rest diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx index a6528a2d6d..6c72cff4e1 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) => ( + + + Contents + + + + + `, + setup() { + let isOpen = ref(false) + return { + isOpen, + setIsOpen(value: boolean) { + isOpen.value = value + }, + toggleOpen() { + isOpen.value = !isOpen.value + }, + } + }, + }) + + assertDialog({ state: DialogState.InvisibleUnmounted }) + + // Open dialog + await click(document.getElementById('trigger')) + + // Verify it is open + assertDialog({ + state: DialogState.Visible, + attributes: { id: 'headlessui-dialog-1' }, + }) + + // Close dialog + await press(Keys.Escape) + + // Verify it is close + assertDialog({ state: DialogState.InvisibleUnmounted }) + }) + ) + + it( + 'should not be possible to close the dialog with Escape, when a field is focused but cancels the event', + suppressConsoleLogs(async () => { + renderTemplate({ + template: ` +
+ + + Contents + + + +
+ `, + setup() { + let isOpen = ref(false) + return { + isOpen, + setIsOpen(value: boolean) { + isOpen.value = value + }, + toggleOpen() { + isOpen.value = !isOpen.value + }, + cancel(event: KeyboardEvent) { + event.preventDefault() + event.stopPropagation() + }, + } + }, + }) + + assertDialog({ state: DialogState.InvisibleUnmounted }) + + // Open dialog + await click(document.getElementById('trigger')) + + // Verify it is open + assertDialog({ + state: DialogState.Visible, + attributes: { id: 'headlessui-dialog-1' }, + }) + + // Try to close the dialog + await press(Keys.Escape) + + // Verify it is still open + assertDialog({ state: DialogState.Visible }) + }) + ) }) }) diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.ts b/packages/@headlessui-vue/src/components/dialog/dialog.ts index 04103fac8b..d2a7f84ad3 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.ts @@ -74,7 +74,7 @@ export let Dialog = defineComponent({ open: { type: [Boolean, String], default: Missing }, initialFocus: { type: Object as PropType, default: null }, }, - emits: ['close'], + emits: { close: (_close: boolean) => true }, render() { let propsWeControl = { // Manually passthrough the attributes, because Vue can't automatically pass @@ -87,7 +87,6 @@ export let Dialog = defineComponent({ 'aria-labelledby': this.titleId, 'aria-describedby': this.describedby, onClick: this.handleClick, - onKeydown: this.handleKeyDown, } let { open: _, initialFocus, ...passThroughProps } = this.$props @@ -205,6 +204,16 @@ export let Dialog = defineComponent({ nextTick(() => target?.focus()) }) + // Handle `Escape` to close + useWindowEvent('keydown', event => { + if (event.key !== Keys.Escape) return + if (dialogState.value !== DialogStates.Open) return + if (containers.value.size > 1) return // 1 is myself, otherwise other elements in the Stack + event.preventDefault() + event.stopPropagation() + api.close() + }) + // Scroll lock watchEffect(onInvalidate => { if (dialogState.value !== DialogStates.Open) return @@ -260,16 +269,6 @@ export let Dialog = defineComponent({ handleClick(event: MouseEvent) { event.stopPropagation() }, - - // Handle `Escape` to close - handleKeyDown(event: KeyboardEvent) { - if (event.key !== Keys.Escape) return - if (dialogState.value !== DialogStates.Open) return - if (containers.value.size > 1) return // 1 is myself, otherwise other elements in the Stack - event.preventDefault() - event.stopPropagation() - api.close() - }, } }, }) diff --git a/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts b/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts index fc3df40a25..415513357b 100644 --- a/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts +++ b/packages/@headlessui-vue/src/components/disclosure/disclosure.test.ts @@ -1,4 +1,4 @@ -import { defineComponent, nextTick, ref, watch } from 'vue' +import { defineComponent, nextTick, ref, watch, h } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Disclosure, DisclosureButton, DisclosurePanel } from './disclosure' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' @@ -291,6 +291,98 @@ describe('Rendering', () => { assertDisclosurePanel({ state: DisclosureState.Visible }) }) ) + + describe('`type` attribute', () => { + it('should set the `type` to "button" by default', async () => { + renderTemplate( + html` + + + Trigger + + + ` + ) + + expect(getDisclosureButton()).toHaveAttribute('type', 'button') + }) + + it('should not set the `type` to "button" if it already contains a `type`', async () => { + renderTemplate( + html` + + + Trigger + + + ` + ) + + expect(getDisclosureButton()).toHaveAttribute('type', 'submit') + }) + + it( + 'should set the `type` to "button" when using the `as` prop which resolves to a "button"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ + CustomButton: defineComponent({ + setup: props => () => h('button', { ...props }), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getDisclosureButton()).toHaveAttribute('type', 'button') + }) + ) + + it('should not set the type if the "as" prop is not a "button"', async () => { + renderTemplate( + html` + + + Trigger + + + ` + ) + + expect(getDisclosureButton()).not.toHaveAttribute('type') + }) + + it( + 'should not set the `type` to "button" when using the `as` prop which resolves to a "div"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ + CustomButton: defineComponent({ + setup: props => () => h('div', props), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getDisclosureButton()).not.toHaveAttribute('type') + }) + ) + }) }) describe('DisclosurePanel', () => { diff --git a/packages/@headlessui-vue/src/components/disclosure/disclosure.ts b/packages/@headlessui-vue/src/components/disclosure/disclosure.ts index e8cc87da5f..23fe0a3f18 100644 --- a/packages/@headlessui-vue/src/components/disclosure/disclosure.ts +++ b/packages/@headlessui-vue/src/components/disclosure/disclosure.ts @@ -1,5 +1,14 @@ // WAI-ARIA: https://www.w3.org/TR/wai-aria-practices-1.2/#disclosure -import { defineComponent, ref, provide, inject, InjectionKey, Ref, computed } from 'vue' +import { + defineComponent, + ref, + provide, + inject, + InjectionKey, + Ref, + computed, + watchEffect, +} from 'vue' import { Keys } from '../../keyboard' import { match } from '../../utils/match' @@ -7,6 +16,7 @@ import { render, Features } from '../../utils/render' import { useId } from '../../hooks/use-id' import { dom } from '../../utils/dom' import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' +import { useResolveButtonType } from '../../hooks/use-resolve-button-type' enum DisclosureStates { Open, @@ -129,14 +139,15 @@ export let DisclosureButton = defineComponent({ let slot = { open: api.disclosureState.value === DisclosureStates.Open } let propsWeControl = this.isWithinPanel ? { - type: 'button', + ref: 'el', + type: this.type, onClick: this.handleClick, onKeydown: this.handleKeyDown, } : { id: this.id, ref: 'el', - type: 'button', + type: this.type, 'aria-expanded': this.$props.disabled ? undefined : api.disclosureState.value === DisclosureStates.Open, @@ -155,16 +166,28 @@ export let DisclosureButton = defineComponent({ name: 'DisclosureButton', }) }, - setup(props) { + setup(props, { attrs }) { let api = useDisclosureContext('DisclosureButton') let panelContext = useDisclosurePanelContext() let isWithinPanel = panelContext === null ? false : panelContext === api.panelId + let elementRef = ref(null) + + if (!isWithinPanel) { + watchEffect(() => { + api.button.value = elementRef.value + }) + } + return { isWithinPanel, id: api.buttonId, - el: isWithinPanel ? undefined : api.button, + el: elementRef, + type: useResolveButtonType( + computed(() => ({ as: props.as, type: attrs.type })), + elementRef + ), handleClick() { if (props.disabled) return 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 ee70eedf46..8243a82ff4 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/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index a67db2630f..1feee6bbf5 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -1,4 +1,4 @@ -import { defineComponent, nextTick, ref, watch } from 'vue' +import { defineComponent, nextTick, ref, watch, h } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Listbox, ListboxLabel, ListboxButton, ListboxOptions, ListboxOption } from './listbox' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' @@ -358,6 +358,101 @@ describe('Rendering', () => { assertListboxButtonLinkedWithListboxLabel() }) ) + + describe('`type` attribute', () => { + it('should set the `type` to "button" by default', async () => { + renderTemplate({ + template: html` + + Trigger + + `, + setup: () => ({ value: ref(null) }), + }) + + expect(getListboxButton()).toHaveAttribute('type', 'button') + }) + + it('should not set the `type` to "button" if it already contains a `type`', async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ value: ref(null) }), + }) + + expect(getListboxButton()).toHaveAttribute('type', 'submit') + }) + + it( + 'should set the `type` to "button" when using the `as` prop which resolves to a "button"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ + value: ref(null), + CustomButton: defineComponent({ + setup: props => () => h('button', { ...props }), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getListboxButton()).toHaveAttribute('type', 'button') + }) + ) + + it('should not set the type if the "as" prop is not a "button"', async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ value: ref(null) }), + }) + + expect(getListboxButton()).not.toHaveAttribute('type') + }) + + it( + 'should not set the `type` to "button" when using the `as` prop which resolves to a "div"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ + value: ref(null), + CustomButton: defineComponent({ + setup: props => () => h('div', props), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getListboxButton()).not.toHaveAttribute('type') + }) + ) + }) }) describe('ListboxOptions', () => { diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index 49b5d28343..d431f1b664 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -23,6 +23,7 @@ import { dom } from '../../utils/dom' import { useWindowEvent } from '../../hooks/use-window-event' import { useOpenClosed, State, useOpenClosedProvider } from '../../internal/open-closed' import { match } from '../../utils/match' +import { useResolveButtonType } from '../../hooks/use-resolve-button-type' enum ListboxStates { Open, @@ -78,7 +79,7 @@ function useListboxContext(component: string) { export let Listbox = defineComponent({ name: 'Listbox', - emits: ['update:modelValue'], + emits: { 'update:modelValue': (_value: any) => true }, props: { as: { type: [Object, String], default: 'template' }, disabled: { type: [Boolean], default: false }, @@ -268,7 +269,7 @@ export let ListboxButton = defineComponent({ let propsWeControl = { ref: 'el', id: this.id, - type: 'button', + type: this.type, 'aria-haspopup': true, 'aria-controls': dom(api.optionsRef)?.id, 'aria-expanded': api.disabled.value @@ -291,7 +292,7 @@ export let ListboxButton = defineComponent({ name: 'ListboxButton', }) }, - setup() { + setup(props, { attrs }) { let api = useListboxContext('ListboxButton') let id = `headlessui-listbox-button-${useId()}` @@ -344,7 +345,17 @@ export let ListboxButton = defineComponent({ } } - return { id, el: api.buttonRef, handleKeyDown, handleKeyUp, handleClick } + return { + id, + el: api.buttonRef, + type: useResolveButtonType( + computed(() => ({ as: props.as, type: attrs.type })), + api.buttonRef + ), + handleKeyDown, + handleKeyUp, + handleClick, + } }, }) diff --git a/packages/@headlessui-vue/src/components/menu/menu.test.tsx b/packages/@headlessui-vue/src/components/menu/menu.test.tsx index 9bdc3c272b..30bbf07a90 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-vue/src/components/menu/menu.test.tsx @@ -353,6 +353,96 @@ describe('Rendering', () => { }) }) ) + + describe('`type` attribute', () => { + it('should set the `type` to "button" by default', async () => { + renderTemplate( + jsx` + + Trigger + + ` + ) + + expect(getMenuButton()).toHaveAttribute('type', 'button') + }) + + it('should not set the `type` to "button" if it already contains a `type`', async () => { + renderTemplate( + jsx` + + + Trigger + + + ` + ) + + expect(getMenuButton()).toHaveAttribute('type', 'submit') + }) + + it( + 'should set the `type` to "button" when using the `as` prop which resolves to a "button"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: jsx` + + + Trigger + + + `, + setup: () => ({ + CustomButton: defineComponent({ + setup: props => () => h('button', { ...props }), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getMenuButton()).toHaveAttribute('type', 'button') + }) + ) + + it('should not set the type if the "as" prop is not a "button"', async () => { + renderTemplate( + jsx` + + + Trigger + + + ` + ) + + expect(getMenuButton()).not.toHaveAttribute('type') + }) + + it( + 'should not set the `type` to "button" when using the `as` prop which resolves to a "div"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: jsx` + + + Trigger + + + `, + setup: () => ({ + CustomButton: defineComponent({ + setup: props => () => h('div', props), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getMenuButton()).not.toHaveAttribute('type') + }) + ) + }) }) describe('MenuItems', () => { diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts index 837d67dbf6..3a252c68d5 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.ts +++ b/packages/@headlessui-vue/src/components/menu/menu.ts @@ -20,6 +20,7 @@ import { useWindowEvent } from '../../hooks/use-window-event' import { useTreeWalker } from '../../hooks/use-tree-walker' import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' import { match } from '../../utils/match' +import { useResolveButtonType } from '../../hooks/use-resolve-button-type' enum MenuStates { Open, @@ -183,7 +184,7 @@ export let MenuButton = defineComponent({ let propsWeControl = { ref: 'el', id: this.id, - type: 'button', + type: this.type, 'aria-haspopup': true, 'aria-controls': dom(api.itemsRef)?.id, 'aria-expanded': this.$props.disabled ? undefined : api.menuState.value === MenuStates.Open, @@ -200,7 +201,7 @@ export let MenuButton = defineComponent({ name: 'MenuButton', }) }, - setup(props) { + setup(props, { attrs }) { let api = useMenuContext('MenuButton') let id = `headlessui-menu-button-${useId()}` @@ -256,7 +257,17 @@ export let MenuButton = defineComponent({ } } - return { id, el: api.buttonRef, handleKeyDown, handleKeyUp, handleClick } + return { + id, + el: api.buttonRef, + type: useResolveButtonType( + computed(() => ({ as: props.as, type: attrs.type })), + api.buttonRef + ), + handleKeyDown, + handleKeyUp, + handleClick, + } }, }) diff --git a/packages/@headlessui-vue/src/components/popover/popover.test.ts b/packages/@headlessui-vue/src/components/popover/popover.test.ts index 758b732cb1..bc872cc825 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.test.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.test.ts @@ -1,4 +1,4 @@ -import { defineComponent, nextTick, ref, watch } from 'vue' +import { defineComponent, nextTick, ref, watch, h } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Popover, PopoverGroup, PopoverButton, PopoverPanel, PopoverOverlay } from './popover' @@ -327,6 +327,96 @@ describe('Rendering', () => { assertPopoverPanel({ state: PopoverState.Visible }) }) ) + + describe('`type` attribute', () => { + it('should set the `type` to "button" by default', async () => { + renderTemplate( + html` + + Trigger + + ` + ) + + expect(getPopoverButton()).toHaveAttribute('type', 'button') + }) + + it('should not set the `type` to "button" if it already contains a `type`', async () => { + renderTemplate( + html` + + + Trigger + + + ` + ) + + expect(getPopoverButton()).toHaveAttribute('type', 'submit') + }) + + it( + 'should set the `type` to "button" when using the `as` prop which resolves to a "button"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ + CustomButton: defineComponent({ + setup: props => () => h('button', { ...props }), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getPopoverButton()).toHaveAttribute('type', 'button') + }) + ) + + it('should not set the type if the "as" prop is not a "button"', async () => { + renderTemplate( + html` + + + Trigger + + + ` + ) + + expect(getPopoverButton()).not.toHaveAttribute('type') + }) + + it( + 'should not set the `type` to "button" when using the `as` prop which resolves to a "div"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + + `, + setup: () => ({ + CustomButton: defineComponent({ + setup: props => () => h('div', props), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getPopoverButton()).not.toHaveAttribute('type') + }) + ) + }) }) describe('PopoverPanel', () => { diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index ef6fdd3912..5b8de111ae 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -27,6 +27,7 @@ import { import { dom } from '../../utils/dom' import { useWindowEvent } from '../../hooks/use-window-event' import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' +import { useResolveButtonType } from '../../hooks/use-resolve-button-type' enum PopoverStates { Open, @@ -211,14 +212,15 @@ export let PopoverButton = defineComponent({ let slot = { open: api.popoverState.value === PopoverStates.Open } let propsWeControl = this.isWithinPanel ? { - type: 'button', + ref: 'el', + type: this.type, onKeydown: this.handleKeyDown, onClick: this.handleClick, } : { ref: 'el', id: api.buttonId, - type: 'button', + type: this.type, 'aria-expanded': this.$props.disabled ? undefined : api.popoverState.value === PopoverStates.Open, @@ -237,7 +239,7 @@ export let PopoverButton = defineComponent({ name: 'PopoverButton', }) }, - setup(props) { + setup(props, { attrs }) { let api = usePopoverContext('PopoverButton') let groupContext = usePopoverGroupContext() @@ -261,9 +263,21 @@ export let PopoverButton = defineComponent({ true ) + let elementRef = ref(null) + + if (!isWithinPanel) { + watchEffect(() => { + api.button.value = elementRef.value + }) + } + return { isWithinPanel, - el: isWithinPanel ? null : api.button, + el: elementRef, + type: useResolveButtonType( + computed(() => ({ as: props.as, type: attrs.type })), + elementRef + ), handleKeyDown(event: KeyboardEvent) { if (isWithinPanel) { if (api.popoverState.value === PopoverStates.Closed) return @@ -373,7 +387,6 @@ export let PopoverButton = defineComponent({ api.togglePopover() } }, - handleFocus() {}, } }, }) diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts index e332b8e940..dba087a802 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts @@ -60,7 +60,7 @@ function useRadioGroupContext(component: string) { export let RadioGroup = defineComponent({ name: 'RadioGroup', - emits: ['update:modelValue'], + emits: { 'update:modelValue': (_value: any) => true }, props: { as: { type: [Object, String], default: 'div' }, disabled: { type: [Boolean], default: false }, diff --git a/packages/@headlessui-vue/src/components/switch/switch.test.tsx b/packages/@headlessui-vue/src/components/switch/switch.test.tsx index 6c15b77343..7cc092f559 100644 --- a/packages/@headlessui-vue/src/components/switch/switch.test.tsx +++ b/packages/@headlessui-vue/src/components/switch/switch.test.tsx @@ -1,4 +1,4 @@ -import { defineComponent, ref, watch } from 'vue' +import { defineComponent, ref, watch, h } from 'vue' import { render } from '../../test-utils/vue-testing-library' import { Switch, SwitchLabel, SwitchDescription, SwitchGroup } from './switch' @@ -12,6 +12,7 @@ import { } from '../../test-utils/accessibility-assertions' import { press, click, Keys } from '../../test-utils/interactions' import { html } from '../../test-utils/html' +import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' jest.mock('../../hooks/use-id') @@ -101,6 +102,93 @@ describe('Rendering', () => { assertSwitch({ state: SwitchState.Off, label: 'Enable notifications' }) }) + + describe('`type` attribute', () => { + it('should set the `type` to "button" by default', async () => { + renderTemplate({ + template: html` + + Trigger + + `, + setup: () => ({ checked: ref(false) }), + }) + + expect(getSwitch()).toHaveAttribute('type', 'button') + }) + + it('should not set the `type` to "button" if it already contains a `type`', async () => { + renderTemplate({ + template: html` + + Trigger + + `, + setup: () => ({ checked: ref(false) }), + }) + + expect(getSwitch()).toHaveAttribute('type', 'submit') + }) + + it( + 'should set the `type` to "button" when using the `as` prop which resolves to a "button"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + Trigger + + `, + setup: () => ({ + checked: ref(false), + CustomButton: defineComponent({ + setup: props => () => h('button', { ...props }), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getSwitch()).toHaveAttribute('type', 'button') + }) + ) + + it('should not set the type if the "as" prop is not a "button"', async () => { + renderTemplate({ + template: html` + + Trigger + + `, + setup: () => ({ checked: ref(false) }), + }) + + expect(getSwitch()).not.toHaveAttribute('type') + }) + + it( + 'should not set the `type` to "button" when using the `as` prop which resolves to a "div"', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + Trigger + + `, + setup: () => ({ + checked: ref(false), + CustomButton: defineComponent({ + setup: props => () => h('div', props), + }), + }), + }) + + await new Promise(requestAnimationFrame) + + expect(getSwitch()).not.toHaveAttribute('type') + }) + ) + }) }) describe('Render composition', () => { diff --git a/packages/@headlessui-vue/src/components/switch/switch.ts b/packages/@headlessui-vue/src/components/switch/switch.ts index d0ef859afb..ae202f8786 100644 --- a/packages/@headlessui-vue/src/components/switch/switch.ts +++ b/packages/@headlessui-vue/src/components/switch/switch.ts @@ -7,6 +7,7 @@ import { // Types InjectionKey, Ref, + computed, } from 'vue' import { render } from '../../utils/render' @@ -14,6 +15,7 @@ import { useId } from '../../hooks/use-id' import { Keys } from '../../keyboard' import { Label, useLabels } from '../label/label' import { Description, useDescriptions } from '../description/description' +import { useResolveButtonType } from '../../hooks/use-resolve-button-type' type StateDefinition = { // State @@ -57,19 +59,18 @@ export let SwitchGroup = defineComponent({ export let Switch = defineComponent({ name: 'Switch', - emits: ['update:modelValue'], + emits: { 'update:modelValue': (_value: boolean) => true }, props: { as: { type: [Object, String], default: 'button' }, modelValue: { type: Boolean, default: false }, }, render() { - let api = inject(GroupContext, null) - let slot = { checked: this.$props.modelValue } let propsWeControl = { id: this.id, - ref: api === null ? undefined : api.switchRef, + ref: 'el', role: 'switch', + type: this.type, tabIndex: 0, 'aria-checked': this.$props.modelValue, 'aria-labelledby': this.labelledby, @@ -79,10 +80,6 @@ export let Switch = defineComponent({ onKeypress: this.handleKeyPress, } - if (this.$props.as === 'button') { - Object.assign(propsWeControl, { type: 'button' }) - } - return render({ props: { ...this.$props, ...propsWeControl }, slot, @@ -91,7 +88,7 @@ export let Switch = defineComponent({ name: 'Switch', }) }, - setup(props, { emit }) { + setup(props, { emit, attrs }) { let api = inject(GroupContext, null) let id = `headlessui-switch-${useId()}` @@ -99,9 +96,16 @@ export let Switch = defineComponent({ emit('update:modelValue', !props.modelValue) } + let internalSwitchRef = ref(null) + let switchRef = api === null ? internalSwitchRef : api.switchRef + return { id, - el: api?.switchRef, + el: switchRef, + type: useResolveButtonType( + computed(() => ({ as: props.as, type: attrs.type })), + switchRef + ), labelledby: api?.labelledby, describedby: api?.describedby, handleClick(event: MouseEvent) { diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.ts b/packages/@headlessui-vue/src/components/tabs/tabs.ts index b13361e9b7..7c50ac0145 100644 --- a/packages/@headlessui-vue/src/components/tabs/tabs.ts +++ b/packages/@headlessui-vue/src/components/tabs/tabs.ts @@ -52,7 +52,9 @@ function useTabsContext(component: string) { export let TabGroup = defineComponent({ name: 'TabGroup', - emits: ['change'], + emits: { + change: (_index: number) => true, + }, props: { as: { type: [Object, String], default: 'template' }, defaultIndex: { type: [Number], default: 0 }, diff --git a/packages/@headlessui-vue/src/components/transitions/transition.ts b/packages/@headlessui-vue/src/components/transitions/transition.ts index 02ba8b8cb5..baed2a359f 100644 --- a/packages/@headlessui-vue/src/components/transitions/transition.ts +++ b/packages/@headlessui-vue/src/components/transitions/transition.ts @@ -145,7 +145,12 @@ export let TransitionChild = defineComponent({ leaveFrom: { type: [String], default: '' }, leaveTo: { type: [String], default: '' }, }, - emits: ['beforeEnter', 'afterEnter', 'beforeLeave', 'afterLeave'], + emits: { + beforeEnter: () => true, + afterEnter: () => true, + beforeLeave: () => true, + afterLeave: () => true, + }, render() { if (this.renderAsRoot) { return h( @@ -357,7 +362,12 @@ export let TransitionRoot = defineComponent({ leaveFrom: { type: [String], default: '' }, leaveTo: { type: [String], default: '' }, }, - emits: ['beforeEnter', 'afterEnter', 'beforeLeave', 'afterLeave'], + emits: { + beforeEnter: () => true, + afterEnter: () => true, + beforeLeave: () => true, + afterLeave: () => true, + }, render() { let { show, appear, unmount, ...passThroughProps } = this.$props let sharedProps = { unmount } diff --git a/packages/@headlessui-vue/src/hooks/use-focus-trap.ts b/packages/@headlessui-vue/src/hooks/use-focus-trap.ts index 3236a14b69..e5a070e721 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 diff --git a/packages/@headlessui-vue/src/hooks/use-resolve-button-type.ts b/packages/@headlessui-vue/src/hooks/use-resolve-button-type.ts new file mode 100644 index 0000000000..507fbd741d --- /dev/null +++ b/packages/@headlessui-vue/src/hooks/use-resolve-button-type.ts @@ -0,0 +1,33 @@ +import { ref, onMounted, watchEffect, Ref } from 'vue' +import { dom } from '../utils/dom' + +function resolveType(type: unknown, as: string | object) { + if (type) return type + + let tag = as ?? 'button' + if (typeof tag === 'string' && tag.toLowerCase() === 'button') return 'button' + + return undefined +} + +export function useResolveButtonType( + data: Ref<{ as: string | object; type?: unknown }>, + refElement: Ref +) { + let type = ref(resolveType(data.value.type, data.value.as)) + + onMounted(() => { + type.value = resolveType(data.value.type, data.value.as) + }) + + watchEffect(() => { + if (type.value) return + if (!dom(refElement)) return + + if (dom(refElement) instanceof HTMLButtonElement && !dom(refElement)?.hasAttribute('type')) { + type.value = 'button' + } + }) + + return type +} diff --git a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts index 5a6fb38a87..6e434b3469 100644 --- a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts @@ -7,7 +7,7 @@ function assertNever(x: never): never { // --- export function getMenuButton(): HTMLElement | null { - return document.querySelector('button,[role="button"]') + return document.querySelector('button,[role="button"],[id^="headlessui-menu-button-"]') } export function getMenuButtons(): HTMLElement[] { @@ -226,7 +226,7 @@ export function getListboxLabel(): HTMLElement | null { } export function getListboxButton(): HTMLElement | null { - return document.querySelector('button,[role="button"]') + return document.querySelector('button,[role="button"],[id^="headlessui-listbox-button-"]') } export function getListboxButtons(): HTMLElement[] {