diff --git a/CHANGELOG.md b/CHANGELOG.md index 37963e5a0c..db60073e5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased - React] -- Nothing yet! +### Fixes + +- Ensure portal root exists in the DOM ([#950](https://github.com/tailwindlabs/headlessui/pull/950)) +- Ensure correct DOM node order when performing focus actions ([#1038](https://github.com/tailwindlabs/headlessui/pull/1038)) + +### Added + +- Allow for `Tab.Group` to be controllable ([#909](https://github.com/tailwindlabs/headlessui/pull/909), [#970](https://github.com/tailwindlabs/headlessui/pull/970)) ## [Unreleased - Vue] -- Nothing yet! +### Fixes + +- Fix missing key binding in examples ([#1036](https://github.com/tailwindlabs/headlessui/pull/1036), [#1006](https://github.com/tailwindlabs/headlessui/pull/1006)) +- Fix slice => splice typo in `Tabs` component ([#1037](https://github.com/tailwindlabs/headlessui/pull/1037), [#986](https://github.com/tailwindlabs/headlessui/pull/986)) +- Ensure correct DOM node order when performing focus actions ([#1038](https://github.com/tailwindlabs/headlessui/pull/1038)) + +### Added + +- Allow for `TabGroup` to be controllable ([#909](https://github.com/tailwindlabs/headlessui/pull/909), [#970](https://github.com/tailwindlabs/headlessui/pull/970)) ## [@headlessui/react@v1.4.2] - 2021-11-08 diff --git a/packages/@headlessui-react/src/components/portal/portal.tsx b/packages/@headlessui-react/src/components/portal/portal.tsx index 24cd3148ad..1c46f51e7a 100644 --- a/packages/@headlessui-react/src/components/portal/portal.tsx +++ b/packages/@headlessui-react/src/components/portal/portal.tsx @@ -34,6 +34,15 @@ function usePortalTarget(): HTMLElement | null { return document.body.appendChild(root) }) + // Ensure the portal root is always in the DOM + useEffect(() => { + if (target === null) return + + if (!document.body.contains(target)) { + document.body.appendChild(target) + } + }, [target]) + useEffect(() => { if (forceInRoot) return if (groupTarget === null) return diff --git a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx index 2125583c00..420eb78b54 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx @@ -1,4 +1,4 @@ -import React, { createElement } from 'react' +import React, { createElement, useState } from 'react' import { render } from '@testing-library/react' import { Tab } from './tabs' @@ -75,6 +75,45 @@ describe('Rendering', () => { assertTabs({ active: 0 }) }) + it('should guarantee the order of DOM nodes when performing actions', async () => { + function Example() { + let [hide, setHide] = useState(false) + + return ( + <> + + + + Tab 1 + {!hide && Tab 2} + Tab 3 + + + + Content 1 + {!hide && Content 2} + Content 3 + + + + ) + } + + render() + + await click(getByText('toggle')) // Remove Tab 2 + await click(getByText('toggle')) // Re-add Tab 2 + + await press(Keys.Tab) + assertTabs({ active: 0 }) + + await press(Keys.ArrowRight) + assertTabs({ active: 1 }) + + await press(Keys.ArrowRight) + assertTabs({ active: 2 }) + }) + describe('`renderProps`', () => { it('should expose the `selectedIndex` on the `Tab.Group` component', async () => { render( @@ -415,6 +454,249 @@ describe('Rendering', () => { assertTabs({ active: 0 }) assertActiveElement(getByText('Tab 1')) }) + + it('should not change the Tab if the defaultIndex changes', async () => { + function Example() { + let [defaultIndex, setDefaultIndex] = useState(1) + + return ( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + + ) + } + + render() + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 1 }) + assertActiveElement(getByText('Tab 2')) + + await click(getByText('Tab 3')) + + assertTabs({ active: 2 }) + assertActiveElement(getByText('Tab 3')) + + // Change default index + await click(getByText('change')) + + // Nothing should change... + assertTabs({ active: 2 }) + }) + }) + + describe('`selectedIndex`', () => { + it('should be possible to change active tab controlled and uncontrolled', async () => { + let handleChange = jest.fn() + + function ControlledTabs() { + let [selectedIndex, setSelectedIndex] = useState(0) + + return ( + <> + { + setSelectedIndex(value) + handleChange(value) + }} + > + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + + ) + } + + render() + + assertActiveElement(document.body) + + // test uncontrolled behaviour + await click(getByText('Tab 2')) + expect(handleChange).toHaveBeenCalledTimes(1) + expect(handleChange).toHaveBeenNthCalledWith(1, 1) + assertTabs({ active: 1 }) + + // test controlled behaviour + await click(getByText('setSelectedIndex')) + assertTabs({ active: 2 }) + }) + + it('should jump to the nearest tab when the selectedIndex is out of bounds (-2)', async () => { + render( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + ) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 0 }) + assertActiveElement(getByText('Tab 1')) + }) + + it('should jump to the nearest tab when the selectedIndex is out of bounds (+5)', async () => { + render( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + ) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 2 }) + assertActiveElement(getByText('Tab 3')) + }) + + it('should jump to the next available tab when the selectedIndex is a disabled tab', async () => { + render( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + ) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 1 }) + assertActiveElement(getByText('Tab 2')) + }) + + it('should jump to the next available tab when the selectedIndex is a disabled tab and wrap around', async () => { + render( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + ) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 0 }) + assertActiveElement(getByText('Tab 1')) + }) + + it('should prefer selectedIndex over defaultIndex', async () => { + render( + <> + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + ) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 0 }) + assertActiveElement(getByText('Tab 1')) + }) }) describe(`'Tab'`, () => { diff --git a/packages/@headlessui-react/src/components/tabs/tabs.tsx b/packages/@headlessui-react/src/components/tabs/tabs.tsx index d90cfe385a..0c0e585879 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.tsx @@ -127,11 +127,19 @@ function Tabs( props: Props & { defaultIndex?: number onChange?: (index: number) => void + selectedIndex?: number vertical?: boolean manual?: boolean } ) { - let { defaultIndex = 0, vertical = false, manual = false, onChange, ...passThroughProps } = props + let { + defaultIndex = 0, + vertical = false, + manual = false, + onChange, + selectedIndex = null, + ...passThroughProps + } = props const orientation = vertical ? 'vertical' : 'horizontal' const activation = manual ? 'manual' : 'auto' @@ -161,18 +169,20 @@ function Tabs( useEffect(() => { if (state.tabs.length <= 0) return - if (state.selectedIndex !== null) return + if (selectedIndex === null && state.selectedIndex !== null) return let tabs = state.tabs.map(tab => tab.current).filter(Boolean) as HTMLElement[] let focusableTabs = tabs.filter(tab => !tab.hasAttribute('disabled')) + let indexToSet = selectedIndex ?? defaultIndex + // Underflow - if (defaultIndex < 0) { + if (indexToSet < 0) { dispatch({ type: ActionTypes.SetSelectedIndex, index: tabs.indexOf(focusableTabs[0]) }) } // Overflow - else if (defaultIndex > state.tabs.length) { + else if (indexToSet > state.tabs.length) { dispatch({ type: ActionTypes.SetSelectedIndex, index: tabs.indexOf(focusableTabs[focusableTabs.length - 1]), @@ -181,15 +191,15 @@ function Tabs( // Middle else { - let before = tabs.slice(0, defaultIndex) - let after = tabs.slice(defaultIndex) + let before = tabs.slice(0, indexToSet) + let after = tabs.slice(indexToSet) let next = [...after, ...before].find(tab => focusableTabs.includes(tab)) if (!next) return dispatch({ type: ActionTypes.SetSelectedIndex, index: tabs.indexOf(next) }) } - }, [defaultIndex, state.tabs, state.selectedIndex]) + }, [defaultIndex, selectedIndex, state.tabs, state.selectedIndex]) let lastChangedIndex = useRef(state.selectedIndex) let providerBag = useMemo>( @@ -348,10 +358,6 @@ export function Tab( } let passThroughProps = props - if (process.env.NODE_ENV === 'test') { - Object.assign(propsWeControl, { ['data-headlessui-index']: myIndex }) - } - return render({ props: { ...passThroughProps, ...propsWeControl }, slot, @@ -423,10 +429,6 @@ function Panel( tabIndex: selected ? 0 : -1, } - if (process.env.NODE_ENV === 'test') { - Object.assign(propsWeControl, { ['data-headlessui-index']: myIndex }) - } - let passThroughProps = props return render({ diff --git a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts index 6e434b3469..1fb40a1f39 100644 --- a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts @@ -1224,8 +1224,8 @@ export function assertTabs( expect(list).toHaveAttribute('role', 'tablist') expect(list).toHaveAttribute('aria-orientation', orientation) - let activeTab = tabs.find(tab => tab.dataset.headlessuiIndex === '' + active) - let activePanel = panels.find(panel => panel.dataset.headlessuiIndex === '' + active) + let activeTab = Array.from(list.querySelectorAll('[id^="headlessui-tabs-tab-"]'))[active] + let activePanel = panels.find(panel => panel.id === activeTab.getAttribute('aria-controls')) for (let tab of tabs) { expect(tab).toHaveAttribute('id') diff --git a/packages/@headlessui-react/src/utils/focus-management.ts b/packages/@headlessui-react/src/utils/focus-management.ts index b3d1104d91..a51b1cde03 100644 --- a/packages/@headlessui-react/src/utils/focus-management.ts +++ b/packages/@headlessui-react/src/utils/focus-management.ts @@ -103,7 +103,15 @@ export function focusElement(element: HTMLElement | null) { } export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus) { - let elements = Array.isArray(container) ? container : getFocusableElements(container) + let elements = Array.isArray(container) + ? container.slice().sort((a, b) => { + let position = a.compareDocumentPosition(b) + + if (position & Node.DOCUMENT_POSITION_FOLLOWING) return -1 + if (position & Node.DOCUMENT_POSITION_PRECEDING) return 1 + return 0 + }) + : getFocusableElements(container) let active = document.activeElement as HTMLElement let direction = (() => { diff --git a/packages/@headlessui-vue/examples/src/components/tabs/tabs.vue b/packages/@headlessui-vue/examples/src/components/tabs/tabs.vue index f8499b13dc..3353754838 100644 --- a/packages/@headlessui-vue/examples/src/components/tabs/tabs.vue +++ b/packages/@headlessui-vue/examples/src/components/tabs/tabs.vue @@ -15,7 +15,7 @@ toggle + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + `, + setup() { + let hide = ref(false) + + return { + hide, + toggle() { + hide.value = !hide.value + }, + } + }, + }) + + await new Promise(nextTick) + + await click(getByText('toggle')) // Remove Tab 2 + await click(getByText('toggle')) // Re-add Tab 2 + + await press(Keys.Tab) + assertTabs({ active: 0 }) + + await press(Keys.ArrowRight) + assertTabs({ active: 1 }) + + await press(Keys.ArrowRight) + assertTabs({ active: 2 }) + }) + describe('`renderProps`', () => { it('should expose the `selectedIndex` on the `Tabs` component', async () => { renderTemplate( @@ -433,6 +478,262 @@ describe('Rendering', () => { assertTabs({ active: 0 }) assertActiveElement(getByText('Tab 1')) }) + + it('should not change the Tab if the defaultIndex changes', async () => { + renderTemplate({ + template: html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + + `, + setup() { + let defaultIndex = ref(1) + return { defaultIndex } + }, + }) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 1 }) + assertActiveElement(getByText('Tab 2')) + + await click(getByText('Tab 3')) + + assertTabs({ active: 2 }) + assertActiveElement(getByText('Tab 3')) + + // Change default index + await click(getByText('change')) + + // Nothing should change... + assertTabs({ active: 2 }) + }) + }) +}) + +describe('`selectedIndex`', () => { + it('should be possible to change active tab controlled and uncontrolled', async () => { + let handleChange = jest.fn() + + renderTemplate({ + template: html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + `, + setup() { + let selectedIndex = ref(0) + + return { + selectedIndex, + handleChange(value: number) { + selectedIndex.value = value + handleChange(value) + }, + next() { + selectedIndex.value += 1 + }, + } + }, + }) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + // test uncontrolled behaviour + await click(getByText('Tab 2')) + expect(handleChange).toHaveBeenCalledTimes(1) + expect(handleChange).toHaveBeenNthCalledWith(1, 1) + assertTabs({ active: 1 }) + + // test controlled behaviour + await click(getByText('setSelectedIndex')) + assertTabs({ active: 2 }) + }) + + it('should jump to the nearest tab when the selectedIndex is out of bounds (-2)', async () => { + renderTemplate( + html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + ` + ) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 0 }) + assertActiveElement(getByText('Tab 1')) + }) + + it('should jump to the nearest tab when the selectedIndex is out of bounds (+5)', async () => { + renderTemplate( + html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + ` + ) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 2 }) + assertActiveElement(getByText('Tab 3')) + }) + + it('should jump to the next available tab when the selectedIndex is a disabled tab', async () => { + renderTemplate( + html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + ` + ) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 1 }) + assertActiveElement(getByText('Tab 2')) + }) + + it('should jump to the next available tab when the selectedIndex is a disabled tab and wrap around', async () => { + renderTemplate( + html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + ` + ) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 0 }) + assertActiveElement(getByText('Tab 1')) + }) + + it('should prefer selectedIndex over defaultIndex', async () => { + renderTemplate( + html` + + + Tab 1 + Tab 2 + Tab 3 + + + + Content 1 + Content 2 + Content 3 + + + + + ` + ) + + await new Promise(nextTick) + + assertActiveElement(document.body) + + await press(Keys.Tab) + + assertTabs({ active: 0 }) + assertActiveElement(getByText('Tab 1')) }) }) @@ -1880,7 +2181,7 @@ describe('Mouse interactions', () => { }) }) -it('should trigger the `onChange` when the tab changes', async () => { +it('should trigger the `change` when the tab changes', async () => { let changes = jest.fn() renderTemplate({ diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.ts b/packages/@headlessui-vue/src/components/tabs/tabs.ts index 3705e8145a..d7f8a1888c 100644 --- a/packages/@headlessui-vue/src/components/tabs/tabs.ts +++ b/packages/@headlessui-vue/src/components/tabs/tabs.ts @@ -8,6 +8,7 @@ import { computed, InjectionKey, Ref, + watchEffect, } from 'vue' import { Features, render, omit } from '../../utils/render' @@ -58,6 +59,7 @@ export let TabGroup = defineComponent({ }, props: { as: { type: [Object, String], default: 'template' }, + selectedIndex: { type: [Number], default: null }, defaultIndex: { type: [Number], default: 0 }, vertical: { type: [Boolean], default: false }, manual: { type: [Boolean], default: false }, @@ -83,40 +85,42 @@ export let TabGroup = defineComponent({ }, unregisterTab(tab: typeof tabs['value'][number]) { let idx = tabs.value.indexOf(tab) - if (idx !== -1) tabs.value.slice(idx, 1) + if (idx !== -1) tabs.value.splice(idx, 1) }, registerPanel(panel: typeof panels['value'][number]) { if (!panels.value.includes(panel)) panels.value.push(panel) }, unregisterPanel(panel: typeof panels['value'][number]) { let idx = panels.value.indexOf(panel) - if (idx !== -1) panels.value.slice(idx, 1) + if (idx !== -1) panels.value.splice(idx, 1) }, } provide(TabsContext, api) - onMounted(() => { - if (api.tabs.value.length <= 0) return console.log('bail') - if (selectedIndex.value !== null) return console.log('bail 2') + watchEffect(() => { + if (api.tabs.value.length <= 0) return + if (props.selectedIndex === null && selectedIndex.value !== null) return let tabs = api.tabs.value.map(tab => dom(tab)).filter(Boolean) as HTMLElement[] let focusableTabs = tabs.filter(tab => !tab.hasAttribute('disabled')) + let indexToSet = props.selectedIndex ?? props.defaultIndex + // Underflow - if (props.defaultIndex < 0) { + if (indexToSet < 0) { selectedIndex.value = tabs.indexOf(focusableTabs[0]) } // Overflow - else if (props.defaultIndex > api.tabs.value.length) { + else if (indexToSet > api.tabs.value.length) { selectedIndex.value = tabs.indexOf(focusableTabs[focusableTabs.length - 1]) } // Middle else { - let before = tabs.slice(0, props.defaultIndex) - let after = tabs.slice(props.defaultIndex) + let before = tabs.slice(0, indexToSet) + let after = tabs.slice(indexToSet) let next = [...after, ...before].find(tab => focusableTabs.includes(tab)) if (!next) return @@ -129,7 +133,7 @@ export let TabGroup = defineComponent({ let slot = { selectedIndex: selectedIndex.value } return render({ - props: omit(props, ['defaultIndex', 'manual', 'vertical']), + props: omit(props, ['selectedIndex', 'defaultIndex', 'manual', 'vertical', 'onChange']), slot, slots, attrs, @@ -195,10 +199,6 @@ export let Tab = defineComponent({ disabled: this.$props.disabled ? true : undefined, } - if (process.env.NODE_ENV === 'test') { - Object.assign(propsWeControl, { ['data-headlessui-index']: this.myIndex }) - } - return render({ props: { ...this.$props, ...propsWeControl }, slot, @@ -330,10 +330,6 @@ export let TabPanel = defineComponent({ tabIndex: this.selected ? 0 : -1, } - if (process.env.NODE_ENV === 'test') { - Object.assign(propsWeControl, { ['data-headlessui-index']: this.myIndex }) - } - return render({ props: { ...this.$props, ...propsWeControl }, slot, diff --git a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts index 6e434b3469..1fb40a1f39 100644 --- a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts @@ -1224,8 +1224,8 @@ export function assertTabs( expect(list).toHaveAttribute('role', 'tablist') expect(list).toHaveAttribute('aria-orientation', orientation) - let activeTab = tabs.find(tab => tab.dataset.headlessuiIndex === '' + active) - let activePanel = panels.find(panel => panel.dataset.headlessuiIndex === '' + active) + let activeTab = Array.from(list.querySelectorAll('[id^="headlessui-tabs-tab-"]'))[active] + let activePanel = panels.find(panel => panel.id === activeTab.getAttribute('aria-controls')) for (let tab of tabs) { expect(tab).toHaveAttribute('id') diff --git a/packages/@headlessui-vue/src/utils/focus-management.ts b/packages/@headlessui-vue/src/utils/focus-management.ts index f873373c7e..f446e84e3b 100644 --- a/packages/@headlessui-vue/src/utils/focus-management.ts +++ b/packages/@headlessui-vue/src/utils/focus-management.ts @@ -96,7 +96,15 @@ export function focusElement(element: HTMLElement | null) { } export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus) { - let elements = Array.isArray(container) ? container : getFocusableElements(container) + let elements = Array.isArray(container) + ? container.slice().sort((a, b) => { + let position = a.compareDocumentPosition(b) + + if (position & Node.DOCUMENT_POSITION_FOLLOWING) return -1 + if (position & Node.DOCUMENT_POSITION_PRECEDING) return 1 + return 0 + }) + : getFocusableElements(container) let active = document.activeElement as HTMLElement let direction = (() => {