Improve Tabs wrapping around when controlling the component and overflowing the selectedIndex (#2213)

* ensure chaning the `selectedIndex` tabs properly wraps around

We never want to use and index that doesn't map to a proper tab.

This commit also makes the implementation similar for both React and
Vue.

* add tests to prove the underflow and overflow wrapping

* drop updating the index manually

This is already adjusted when tabs change internally. You can still
manually change it of course, but for these tests that doesn't matter
and cause different results.

* update changelog
This commit is contained in:
Robin Malfait
2023-01-26 15:55:55 +01:00
committed by GitHub
parent dbcfb23bc3
commit e2294f5280
6 changed files with 312 additions and 47 deletions
+1
View File
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve `Combobox` accessibility ([#2153](https://github.com/tailwindlabs/headlessui/pull/2153))
- Fix crash when reading `headlessuiFocusGuard` of `relatedTarget` in the `FocusTrap` component ([#2203](https://github.com/tailwindlabs/headlessui/pull/2203))
- Fix `FocusTrap` in `Dialog` when there is only 1 focusable element ([#2172](https://github.com/tailwindlabs/headlessui/pull/2172))
- Improve `Tabs` wrapping around when controlling the component and overflowing the `selectedIndex` ([#2213](https://github.com/tailwindlabs/headlessui/pull/2213))
## [1.7.7] - 2022-12-16
@@ -179,7 +179,6 @@ describe('Rendering', () => {
<button
onClick={() => {
setTabs((tabs) => tabs.slice().reverse())
setSelectedIndex((idx) => tabs.length - 1 - idx)
}}
>
reverse
@@ -1083,6 +1082,94 @@ describe('Rendering', () => {
assertActiveElement(getByText('Tab 1'))
})
)
it(
'should wrap around when overflowing the index when using a controlled component',
suppressConsoleLogs(async () => {
function Example() {
let [selectedIndex, setSelectedIndex] = useState(0)
return (
<Tab.Group selectedIndex={selectedIndex} onChange={setSelectedIndex}>
{({ selectedIndex }) => (
<>
<Tab.List>
<Tab>Tab 1</Tab>
<Tab>Tab 2</Tab>
<Tab>Tab 3</Tab>
</Tab.List>
<Tab.Panels>
<Tab.Panel>Content 1</Tab.Panel>
<Tab.Panel>Content 2</Tab.Panel>
<Tab.Panel>Content 3</Tab.Panel>
</Tab.Panels>
<button onClick={() => setSelectedIndex(selectedIndex + 1)}>Next</button>
</>
)}
</Tab.Group>
)
}
render(<Example />)
assertActiveElement(document.body)
await click(getByText('Next'))
assertTabs({ active: 1 })
await click(getByText('Next'))
assertTabs({ active: 2 })
await click(getByText('Next'))
assertTabs({ active: 0 })
await click(getByText('Next'))
assertTabs({ active: 1 })
})
)
it(
'should wrap around when underflowing the index when using a controlled component',
suppressConsoleLogs(async () => {
function Example() {
let [selectedIndex, setSelectedIndex] = useState(0)
return (
<Tab.Group selectedIndex={selectedIndex} onChange={setSelectedIndex}>
{({ selectedIndex }) => (
<>
<Tab.List>
<Tab>Tab 1</Tab>
<Tab>Tab 2</Tab>
<Tab>Tab 3</Tab>
</Tab.List>
<Tab.Panels>
<Tab.Panel>Content 1</Tab.Panel>
<Tab.Panel>Content 2</Tab.Panel>
<Tab.Panel>Content 3</Tab.Panel>
</Tab.Panels>
<button onClick={() => setSelectedIndex(selectedIndex - 1)}>Previous</button>
</>
)}
</Tab.Group>
)
}
render(<Example />)
assertActiveElement(document.body)
await click(getByText('Previous'))
assertTabs({ active: 2 })
await click(getByText('Previous'))
assertTabs({ active: 1 })
await click(getByText('Previous'))
assertTabs({ active: 0 })
await click(getByText('Previous'))
assertTabs({ active: 2 })
})
)
})
describe(`'Tab'`, () => {
@@ -30,6 +30,17 @@ import { microTask } from '../../utils/micro-task'
import { Hidden } from '../../internal/hidden'
import { getOwnerDocument } from '../../utils/owner'
enum Direction {
Forwards,
Backwards,
}
enum Ordering {
Less = -1,
Equal = 0,
Greater = 1,
}
interface StateDefinition {
selectedIndex: number
@@ -68,16 +79,30 @@ let reducers: {
let nextState = { ...state, tabs, panels }
// Underflow
if (action.index < 0) {
return { ...nextState, selectedIndex: tabs.indexOf(focusableTabs[0]) }
}
if (
// Underflow
action.index < 0 ||
// Overflow
action.index > tabs.length - 1
) {
let direction = match(Math.sign(action.index - state.selectedIndex), {
[Ordering.Less]: () => Direction.Backwards,
[Ordering.Equal]: () => {
return match(Math.sign(action.index), {
[Ordering.Less]: () => Direction.Forwards,
[Ordering.Equal]: () => Direction.Forwards,
[Ordering.Greater]: () => Direction.Backwards,
})
},
[Ordering.Greater]: () => Direction.Forwards,
})
// Overflow
else if (action.index > tabs.length) {
return {
...nextState,
selectedIndex: tabs.indexOf(focusableTabs[focusableTabs.length - 1]),
selectedIndex: match(direction, {
[Direction.Forwards]: () => tabs.indexOf(focusableTabs[0]),
[Direction.Backwards]: () => tabs.indexOf(focusableTabs[focusableTabs.length - 1]),
}),
}
}
+1
View File
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve `Combobox` accessibility ([#2153](https://github.com/tailwindlabs/headlessui/pull/2153))
- Fix crash when reading `headlessuiFocusGuard` of `relatedTarget` in the `FocusTrap` component ([#2203](https://github.com/tailwindlabs/headlessui/pull/2203))
- Fix `FocusTrap` in `Dialog` when there is only 1 focusable element ([#2172](https://github.com/tailwindlabs/headlessui/pull/2172))
- Improve `Tabs` wrapping around when controlling the component and overflowing the `selectedIndex` ([#2213](https://github.com/tailwindlabs/headlessui/pull/2213))
## [1.7.7] - 2022-12-16
@@ -199,7 +199,6 @@ describe('Rendering', () => {
selectedIndex,
reverse() {
tabs.value = tabs.value.slice().reverse()
selectedIndex.value = tabs.value.length - 1 - selectedIndex.value
},
handleChange(value: number) {
selectedIndex.value = value
@@ -999,6 +998,106 @@ describe('`selectedIndex`', () => {
assertTabs({ active: 0 })
assertActiveElement(getByText('Tab 1'))
})
it(
'should wrap around when overflowing the index when using a controlled component',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<TabGroup :selectedIndex="value" @change="set" v-slot="{ selectedIndex }">
<TabList>
<Tab>Tab 1</Tab>
<Tab>Tab 2</Tab>
<Tab>Tab 3</Tab>
</TabList>
<TabPanels>
<TabPanel>Content 1</TabPanel>
<TabPanel>Content 2</TabPanel>
<TabPanel>Content 3</TabPanel>
</TabPanels>
<button @click="set(selectedIndex + 1)">Next</button>
</TabGroup>
`,
setup() {
let value = ref(0)
return {
value,
set(v: number) {
value.value = v
},
}
},
})
await new Promise<void>(nextTick)
assertActiveElement(document.body)
await click(getByText('Next'))
assertTabs({ active: 1 })
await click(getByText('Next'))
assertTabs({ active: 2 })
await click(getByText('Next'))
assertTabs({ active: 0 })
await click(getByText('Next'))
assertTabs({ active: 1 })
})
)
it(
'should wrap around when underflowing the index when using a controlled component',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<TabGroup :selectedIndex="value" @change="set" v-slot="{ selectedIndex }">
<TabList>
<Tab>Tab 1</Tab>
<Tab>Tab 2</Tab>
<Tab>Tab 3</Tab>
</TabList>
<TabPanels>
<TabPanel>Content 1</TabPanel>
<TabPanel>Content 2</TabPanel>
<TabPanel>Content 3</TabPanel>
</TabPanels>
<button @click="set(selectedIndex - 1)">Previous</button>
</TabGroup>
`,
setup() {
let value = ref(0)
return {
value,
set(v: number) {
value.value = v
},
}
},
})
await new Promise<void>(nextTick)
assertActiveElement(document.body)
await click(getByText('Previous'))
assertTabs({ active: 2 })
await click(getByText('Previous'))
assertTabs({ active: 1 })
await click(getByText('Previous'))
assertTabs({ active: 0 })
await click(getByText('Previous'))
assertTabs({ active: 2 })
})
)
})
describe('Keyboard interactions', () => {
@@ -13,6 +13,7 @@ import {
// Types
InjectionKey,
Ref,
watch,
} from 'vue'
import { Features, render, omit } from '../../utils/render'
@@ -27,6 +28,17 @@ import { microTask } from '../../utils/micro-task'
import { Hidden } from '../../internal/hidden'
import { getOwnerDocument } from '../../utils/owner'
enum Direction {
Forwards,
Backwards,
}
enum Ordering {
Less = -1,
Equal = 0,
Greater = 1,
}
type StateDefinition = {
// State
selectedIndex: Ref<number | null>
@@ -78,7 +90,9 @@ export let TabGroup = defineComponent({
},
inheritAttrs: false,
setup(props, { slots, attrs, emit }) {
let selectedIndex = ref<StateDefinition['selectedIndex']['value']>(null)
let selectedIndex = ref<StateDefinition['selectedIndex']['value']>(
props.selectedIndex ?? props.defaultIndex
)
let tabs = ref<StateDefinition['tabs']['value']>([])
let panels = ref<StateDefinition['panels']['value']>([])
@@ -87,6 +101,60 @@ export let TabGroup = defineComponent({
isControlled.value ? props.selectedIndex : selectedIndex.value
)
function setSelectedIndex(indexToSet: number) {
let tabs = sortByDomNode(api.tabs.value, dom)
let panels = sortByDomNode(api.panels.value, dom)
let focusableTabs = tabs.filter((tab) => !dom(tab)?.hasAttribute('disabled'))
if (
// Underflow
indexToSet < 0 ||
// Overflow
indexToSet > tabs.length - 1
) {
let direction = match(
selectedIndex.value === null // Not set yet
? Ordering.Equal
: Math.sign(indexToSet - selectedIndex.value!),
{
[Ordering.Less]: () => Direction.Backwards,
[Ordering.Equal]: () => {
return match(Math.sign(indexToSet), {
[Ordering.Less]: () => Direction.Forwards,
[Ordering.Equal]: () => Direction.Forwards,
[Ordering.Greater]: () => Direction.Backwards,
})
},
[Ordering.Greater]: () => Direction.Forwards,
}
)
selectedIndex.value = match(direction, {
[Direction.Forwards]: () => tabs.indexOf(focusableTabs[0]),
[Direction.Backwards]: () => tabs.indexOf(focusableTabs[focusableTabs.length - 1]),
})
api.tabs.value = tabs
api.panels.value = panels
}
// Middle
else {
let before = tabs.slice(0, indexToSet)
let after = tabs.slice(indexToSet)
let next = [...after, ...before].find((tab) => focusableTabs.includes(tab))
if (!next) return
let localSelectedIndex = tabs.indexOf(next) ?? api.selectedIndex.value
if (localSelectedIndex === -1) localSelectedIndex = api.selectedIndex.value
selectedIndex.value = localSelectedIndex
api.tabs.value = tabs
api.panels.value = panels
}
}
let api = {
selectedIndex: computed(() => selectedIndex.value ?? props.defaultIndex ?? null),
orientation: computed(() => (props.vertical ? 'vertical' : 'horizontal')),
@@ -99,18 +167,29 @@ export let TabGroup = defineComponent({
}
if (!isControlled.value) {
selectedIndex.value = index
setSelectedIndex(index)
}
},
registerTab(tab: typeof tabs['value'][number]) {
if (!tabs.value.includes(tab)) tabs.value.push(tab)
if (tabs.value.includes(tab)) return
let activeTab = tabs.value[selectedIndex.value!]
tabs.value.push(tab)
tabs.value = sortByDomNode(tabs.value, dom)
let localSelectedIndex = tabs.value.indexOf(activeTab) ?? selectedIndex.value
if (localSelectedIndex !== -1) {
selectedIndex.value = localSelectedIndex
}
},
unregisterTab(tab: typeof tabs['value'][number]) {
let idx = tabs.value.indexOf(tab)
if (idx !== -1) tabs.value.splice(idx, 1)
},
registerPanel(panel: typeof panels['value'][number]) {
if (!panels.value.includes(panel)) panels.value.push(panel)
if (panels.value.includes(panel)) return
panels.value.push(panel)
panels.value = sortByDomNode(panels.value, dom)
},
unregisterPanel(panel: typeof panels['value'][number]) {
let idx = panels.value.indexOf(panel)
@@ -130,41 +209,14 @@ export let TabGroup = defineComponent({
computed(() => (mounted.value ? null : SSRCounter.value))
)
watchEffect(() => {
if (api.tabs.value.length <= 0) return
if (props.selectedIndex === null && selectedIndex.value !== null) return
let incomingSelectedIndex = computed(() => props.selectedIndex)
api.tabs.value = sortByDomNode(api.tabs.value, dom)
api.panels.value = sortByDomNode(api.panels.value, dom)
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 (indexToSet < 0) {
selectedIndex.value = tabs.indexOf(focusableTabs[0])
}
// Overflow
else if (indexToSet > api.tabs.value.length) {
selectedIndex.value = tabs.indexOf(focusableTabs[focusableTabs.length - 1])
}
// Middle
else {
let before = tabs.slice(0, indexToSet)
let after = tabs.slice(indexToSet)
let next = [...after, ...before].find((tab) => focusableTabs.includes(tab))
if (!next) return
let localSelectedIndex = tabs.indexOf(next) ?? api.selectedIndex.value
if (localSelectedIndex === -1) localSelectedIndex = api.selectedIndex.value
selectedIndex.value = localSelectedIndex
}
onMounted(() => {
watch(
[incomingSelectedIndex /* Deliberately skipping defaultIndex */],
() => setSelectedIndex(props.selectedIndex ?? props.defaultIndex),
{ immediate: true }
)
})
watchEffect(() => {