Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Tab order stays consistent, and the currently active Tab stays active #1837

Merged
merged 3 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<fieldset disabled>` 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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ describe('Rendering', () => {
handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
}}
>
<Combobox name="assignee">
<Combobox<string> name="assignee">
{({ value }) => (
<>
<div data-testid="value">{value}</div>
Expand Down
44 changes: 44 additions & 0 deletions packages/@headlessui-react/src/components/tabs/tabs.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,50 @@ describe('Rendering', () => {
})
)

it(
'should guarantee the order when injecting new tabs dynamically',
suppressConsoleLogs(async () => {
function Example() {
let [tabs, setTabs] = useState<string[]>([])

return (
<Tab.Group>
<Tab.List>
{tabs.map((t, i) => (
<Tab key={t}>Tab {i + 1}</Tab>
))}
<Tab>Insert new</Tab>
</Tab.List>
<Tab.Panels>
{tabs.map((t) => (
<Tab.Panel key={t}>{t}</Tab.Panel>
))}
<Tab.Panel>
<button
onClick={() => {
setTabs((old) => [...old, `Panel ${old.length + 1}`])
}}
>
Insert
</button>
</Tab.Panel>
</Tab.Panels>
</Tab.Group>
)
}

render(<Example />)

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',
Expand Down
28 changes: 18 additions & 10 deletions packages/@headlessui-react/src/components/tabs/tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) }
Expand Down Expand Up @@ -487,7 +491,7 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
let SSRContext = useSSRTabsCounter('Tab.Panel')

let id = `headlessui-tabs-panel-${useId()}`
let internalPanelRef = useRef<HTMLElement>(null)
let internalPanelRef = useRef<HTMLElement | null>(null)
let panelRef = useSyncRefs(internalPanelRef, ref, (element) => {
if (!element) return
requestAnimationFrame(() => actions.forceRerender())
Expand All @@ -514,6 +518,10 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
tabIndex: selected ? 0 : -1,
}

if (!selected && (props.unmount ?? true)) {
return <Hidden as="span" {...ourProps} />
}

return render({
ourProps,
theirProps,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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')
Expand Down Expand Up @@ -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')
}
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
42 changes: 42 additions & 0 deletions packages/@headlessui-vue/src/components/tabs/tabs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,48 @@ describe('Rendering', () => {
assertTabs({ active: 2 })
})

it(
'should guarantee the order when injecting new tabs dynamically',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<TabGroup>
<TabList>
<Tab v-for="(t, i) in tabs" :key="t">Tab {{ i + 1 }}</Tab>
<Tab>Insert new</Tab>
</TabList>
<TabPanels>
<TabPanel v-for="t in tabs" :key="t">{{ t }}</TabPanel>
<TabPanel>
<button @click="add">Insert</button>
</TabPanel>
</TabPanels>
</TabGroup>
`,
setup() {
let tabs = ref<string[]>([])

return {
tabs,
add() {
tabs.value.push(`Panel ${tabs.value.length + 1}`)
},
}
},
})

await new Promise<void>(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(
Expand Down
9 changes: 7 additions & 2 deletions packages/@headlessui-vue/src/components/tabs/tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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')
Expand Down Expand Up @@ -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')
}
Expand Down