From b296b7378397fdee5be8650747fbef50ecb1c68e Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 8 Sep 2022 19:14:42 +0200 Subject: [PATCH] Ensure `Tab` order stays consistent, and the currently active `Tab` stays active (#1837) * ensure tabs order stays consistent This ensures that whenever you insert or delete tabs before the current tab, that the current tab stays active with the proper panel. To do this we had to start rendering the non-visible panels as well, but we used the `Hidden` component already which is position fixed and completely hidden so this should not break layouts where using flexbox or grid. * update changelog * fix TypeScript issue --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/combobox/combobox.test.tsx | 2 +- .../src/components/tabs/tabs.test.tsx | 44 +++++++++++++++++++ .../src/components/tabs/tabs.tsx | 28 +++++++----- .../test-utils/accessibility-assertions.ts | 10 +++++ packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/tabs/tabs.test.ts | 42 ++++++++++++++++++ .../src/components/tabs/tabs.ts | 9 +++- .../test-utils/accessibility-assertions.ts | 10 +++++ 9 files changed, 134 insertions(+), 13 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 0088bad..48339ad 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Improve iOS scroll locking ([#1830](https://github.com/tailwindlabs/headlessui/pull/1830)) - Add `
` check to radio group options in React ([#1835](https://github.com/tailwindlabs/headlessui/pull/1835)) +- Ensure `Tab` order stays consistent, and the currently active `Tab` stays active ([#1837](https://github.com/tailwindlabs/headlessui/pull/1837)) ## [1.7.0] - 2022-09-06 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index cc67bc7..0ddb12b 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -1046,7 +1046,7 @@ describe('Rendering', () => { handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) }} > - + name="assignee"> {({ value }) => ( <>
{value}
diff --git a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx index f2d635f..20e16bf 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx @@ -123,6 +123,50 @@ describe('Rendering', () => { }) ) + it( + 'should guarantee the order when injecting new tabs dynamically', + suppressConsoleLogs(async () => { + function Example() { + let [tabs, setTabs] = useState([]) + + return ( + + + {tabs.map((t, i) => ( + Tab {i + 1} + ))} + Insert new + + + {tabs.map((t) => ( + {t} + ))} + + + + + + ) + } + + render() + + assertTabs({ active: 0, tabContents: 'Insert new', panelContents: 'Insert' }) + + // Add some new tabs + await click(getByText('Insert')) + + // We should still be on the tab we were on + assertTabs({ active: 1, tabContents: 'Insert new', panelContents: 'Insert' }) + }) + ) + describe('`renderProps`', () => { it( 'should expose the `selectedIndex` on the `Tab.Group` component', diff --git a/packages/@headlessui-react/src/components/tabs/tabs.tsx b/packages/@headlessui-react/src/components/tabs/tabs.tsx index 62197eb..a34ce3d 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.tsx @@ -27,6 +27,7 @@ import { useLatestValue } from '../../hooks/use-latest-value' import { FocusSentinel } from '../../internal/focus-sentinel' import { useEvent } from '../../hooks/use-event' import { microTask } from '../../utils/micro-task' +import { Hidden } from '../../internal/hidden' interface StateDefinition { selectedIndex: number @@ -88,20 +89,23 @@ let reducers: { }, [ActionTypes.RegisterTab](state, action) { if (state.tabs.includes(action.tab)) return state - return { ...state, tabs: sortByDomNode([...state.tabs, action.tab], (tab) => tab.current) } + let activeTab = state.tabs[state.selectedIndex] + + let adjustedTabs = sortByDomNode([...state.tabs, action.tab], (tab) => tab.current) + let selectedIndex = adjustedTabs.indexOf(activeTab) ?? state.selectedIndex + if (selectedIndex === -1) selectedIndex = state.selectedIndex + + return { ...state, tabs: adjustedTabs, selectedIndex } }, [ActionTypes.UnregisterTab](state, action) { - return { - ...state, - tabs: sortByDomNode( - state.tabs.filter((tab) => tab !== action.tab), - (tab) => tab.current - ), - } + return { ...state, tabs: state.tabs.filter((tab) => tab !== action.tab) } }, [ActionTypes.RegisterPanel](state, action) { if (state.panels.includes(action.panel)) return state - return { ...state, panels: [...state.panels, action.panel] } + return { + ...state, + panels: sortByDomNode([...state.panels, action.panel], (panel) => panel.current), + } }, [ActionTypes.UnregisterPanel](state, action) { return { ...state, panels: state.panels.filter((panel) => panel !== action.panel) } @@ -487,7 +491,7 @@ let Panel = forwardRefWithAs(function Panel(null) + let internalPanelRef = useRef(null) let panelRef = useSyncRefs(internalPanelRef, ref, (element) => { if (!element) return requestAnimationFrame(() => actions.forceRerender()) @@ -514,6 +518,10 @@ let Panel = forwardRefWithAs(function Panel + } + return render({ ourProps, theirProps, diff --git a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts index 6ba9263..89d201e 100644 --- a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts @@ -1664,9 +1664,13 @@ export function assertTabs( { active, orientation = 'horizontal', + tabContents = null, + panelContents = null, }: { active: number orientation?: 'vertical' | 'horizontal' + tabContents?: string | null + panelContents?: string | null }, list = getTabList(), tabs = getTabs(), @@ -1689,6 +1693,9 @@ export function assertTabs( if (tab === activeTab) { expect(tab).toHaveAttribute('aria-selected', 'true') expect(tab).toHaveAttribute('tabindex', '0') + if (tabContents !== null) { + expect(tab.textContent).toBe(tabContents) + } } else { expect(tab).toHaveAttribute('aria-selected', 'false') expect(tab).toHaveAttribute('tabindex', '-1') @@ -1716,6 +1723,9 @@ export function assertTabs( if (panel === activePanel) { expect(panel).toHaveAttribute('tabindex', '0') + if (tabContents !== null) { + expect(panel.textContent).toBe(panelContents) + } } else { expect(panel).toHaveAttribute('tabindex', '-1') } diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 38cc81f..15b9050 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 - Improve iOS scroll locking ([#1830](https://github.com/tailwindlabs/headlessui/pull/1830)) +- Ensure `Tab` order stays consistent, and the currently active `Tab` stays active ([#1837](https://github.com/tailwindlabs/headlessui/pull/1837)) ## [1.7.0] - 2022-09-06 diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.test.ts b/packages/@headlessui-vue/src/components/tabs/tabs.test.ts index 29a6cb9..f0a3aa6 100644 --- a/packages/@headlessui-vue/src/components/tabs/tabs.test.ts +++ b/packages/@headlessui-vue/src/components/tabs/tabs.test.ts @@ -132,6 +132,48 @@ describe('Rendering', () => { assertTabs({ active: 2 }) }) + it( + 'should guarantee the order when injecting new tabs dynamically', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Tab {{ i + 1 }} + Insert new + + + {{ t }} + + + + + + `, + setup() { + let tabs = ref([]) + + return { + tabs, + add() { + tabs.value.push(`Panel ${tabs.value.length + 1}`) + }, + } + }, + }) + + await new Promise(nextTick) + + assertTabs({ active: 0, tabContents: 'Insert new', panelContents: 'Insert' }) + + // Add some new tabs + await click(getByText('Insert')) + + // We should still be on the tab we were on + assertTabs({ active: 1, tabContents: 'Insert new', panelContents: 'Insert' }) + }) + ) + describe('`renderProps`', () => { it('should expose the `selectedIndex` on the `Tabs` component', async () => { renderTemplate( diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.ts b/packages/@headlessui-vue/src/components/tabs/tabs.ts index a9fb146..a60236b 100644 --- a/packages/@headlessui-vue/src/components/tabs/tabs.ts +++ b/packages/@headlessui-vue/src/components/tabs/tabs.ts @@ -24,6 +24,7 @@ import { focusIn, Focus } from '../../utils/focus-management' import { useResolveButtonType } from '../../hooks/use-resolve-button-type' import { FocusSentinel } from '../../internal/focus-sentinel' import { microTask } from '../../utils/micro-task' +import { Hidden } from '../../internal/hidden' type StateDefinition = { // State @@ -320,7 +321,7 @@ export let Tab = defineComponent({ id, role: 'tab', type: type.value, - 'aria-controls': api.panels.value[myIndex.value]?.value?.id, + 'aria-controls': dom(api.panels.value[myIndex.value])?.id, 'aria-selected': selected.value, tabIndex: selected.value ? 0 : -1, disabled: props.disabled ? true : undefined, @@ -390,10 +391,14 @@ export let TabPanel = defineComponent({ ref: internalPanelRef, id, role: 'tabpanel', - 'aria-labelledby': api.tabs.value[myIndex.value]?.value?.id, + 'aria-labelledby': dom(api.tabs.value[myIndex.value])?.id, tabIndex: selected.value ? 0 : -1, } + if (!selected.value && props.unmount) { + return h(Hidden, { as: 'span', ...ourProps }) + } + return render({ ourProps, theirProps: props, diff --git a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts index 6ba9263..89d201e 100644 --- a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts @@ -1664,9 +1664,13 @@ export function assertTabs( { active, orientation = 'horizontal', + tabContents = null, + panelContents = null, }: { active: number orientation?: 'vertical' | 'horizontal' + tabContents?: string | null + panelContents?: string | null }, list = getTabList(), tabs = getTabs(), @@ -1689,6 +1693,9 @@ export function assertTabs( if (tab === activeTab) { expect(tab).toHaveAttribute('aria-selected', 'true') expect(tab).toHaveAttribute('tabindex', '0') + if (tabContents !== null) { + expect(tab.textContent).toBe(tabContents) + } } else { expect(tab).toHaveAttribute('aria-selected', 'false') expect(tab).toHaveAttribute('tabindex', '-1') @@ -1716,6 +1723,9 @@ export function assertTabs( if (panel === activePanel) { expect(panel).toHaveAttribute('tabindex', '0') + if (tabContents !== null) { + expect(panel.textContent).toBe(panelContents) + } } else { expect(panel).toHaveAttribute('tabindex', '-1') }