Respect selectedIndex for controlled <Tab/> components (#3037)

* ensure controlled `<Tab>` components respect the `selectedIndex` * update changelog * use older syntax in tests * run prettier to fix lint step
tailwindlabs · Mar 15, 2024 · 834dbf4 · 834dbf4
1 parent 8c1c42b
commit 834dbf4
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 5 deletions.
diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Allow setting custom `tabIndex` on the `<Switch />` component ([#2966](https://github.com/tailwindlabs/headlessui/pull/2966))
 - Forward `disabled` state to hidden inputs in form-like components ([#3004](https://github.com/tailwindlabs/headlessui/pull/3004))
 - Prefer incoming `data-*` attributes, over the ones set by Headless UI ([#3035](https://github.com/tailwindlabs/headlessui/pull/3035))
+- Respect `selectedIndex` for controlled `<Tab/>` components ([#3037](https://github.com/tailwindlabs/headlessui/pull/3037))
 
 ### Changed
 

diff --git a/packages/@headlessui-react/src/components/tabs/tabs.test.tsx b/packages/@headlessui-react/src/components/tabs/tabs.test.tsx
@@ -167,6 +167,53 @@ describe('Rendering', () => {
     })
   )
 
+  it(
+    'should use the `selectedIndex` when injecting new tabs dynamically',
+    suppressConsoleLogs(async () => {
+      function Example() {
+        let [tabs, setTabs] = useState<string[]>(['A', 'B', 'C'])
+
+        return (
+          <>
+            <Tab.Group selectedIndex={1}>
+              <Tab.List>
+                {tabs.map((t) => (
+                  <Tab key={t}>Tab {t}</Tab>
+                ))}
+              </Tab.List>
+              <Tab.Panels>
+                {tabs.map((t) => (
+                  <Tab.Panel key={t}>Panel {t}</Tab.Panel>
+                ))}
+              </Tab.Panels>
+            </Tab.Group>
+            <button
+              onClick={() => {
+                setTabs((old) => {
+                  let copy = old.slice()
+                  copy.splice(1, 0, 'D')
+                  return copy
+                })
+              }}
+            >
+              Insert
+            </button>
+          </>
+        )
+      }
+
+      render(<Example />)
+
+      assertTabs({ active: 1, tabContents: 'Tab B', panelContents: 'Panel B' })
+
+      // Add some new tabs
+      await click(getByText('Insert'))
+
+      // We should still be at the same tab position, but the tab itself changed
+      assertTabs({ active: 1, tabContents: 'Tab D', panelContents: 'Panel D' })
+    })
+  )
+
   it(
     'should guarantee the order of DOM nodes when reversing the tabs and panels themselves, then performing actions (controlled component)',
     suppressConsoleLogs(async () => {

diff --git a/packages/@headlessui-react/src/components/tabs/tabs.tsx b/packages/@headlessui-react/src/components/tabs/tabs.tsx
@@ -53,6 +53,7 @@ enum Ordering {
 }
 
 interface StateDefinition {
+  info: MutableRefObject<{ isControlled: boolean }>
   selectedIndex: number
 
   tabs: MutableRefObject<HTMLElement | null>[]
@@ -145,8 +146,18 @@ let reducers: {
     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
+    let selectedIndex = state.selectedIndex
+
+    // When the component is uncontrolled, then we want to maintain the actively
+    // selected tab even if new tabs are inserted or removed before the active
+    // tab.
+    //
+    // When the component is controlled, then we don't want to do this and
+    // instead we want to select the tab based on the `selectedIndex` prop.
+    if (!state.info.current.isControlled) {
+      selectedIndex = adjustedTabs.indexOf(activeTab)
+      if (selectedIndex === -1) selectedIndex = state.selectedIndex
+    }
 
     return { ...state, tabs: adjustedTabs, selectedIndex }
   },
@@ -245,8 +256,11 @@ function GroupFn<TTag extends ElementType = typeof DEFAULT_TABS_TAG>(
 
   let isControlled = selectedIndex !== null
 
+  let info = useLatestValue({ isControlled })
+
   let tabsRef = useSyncRefs(ref)
   let [state, dispatch] = useReducer(stateReducer, {
+    info,
     selectedIndex: selectedIndex ?? defaultIndex,
     tabs: [],
     panels: [],

diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `hidden` attribute to internal `<Hidden />` component when the `Features.Hidden` feature is used ([#2955](https://github.com/tailwindlabs/headlessui/pull/2955))
 - Allow setting custom `tabIndex` on the `<Switch />` component ([#2966](https://github.com/tailwindlabs/headlessui/pull/2966))
 - Forward `disabled` state to hidden inputs in form-like components ([#3004](https://github.com/tailwindlabs/headlessui/pull/3004))
+- Respect `selectedIndex` for controlled `<Tab/>` components ([#3037](https://github.com/tailwindlabs/headlessui/pull/3037))
 
 ## [1.7.19] - 2024-02-07
 

diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.test.ts b/packages/@headlessui-vue/src/components/tabs/tabs.test.ts
@@ -171,6 +171,45 @@ describe('Rendering', () => {
     })
   )
 
+  it(
+    'should use the `selectedIndex` when injecting new tabs dynamically',
+    suppressConsoleLogs(async () => {
+      renderTemplate({
+        template: html`
+          <TabGroup :selectedIndex="1">
+            <TabList>
+              <Tab v-for="t in tabs" :key="t">Tab {{ t }}</Tab>
+            </TabList>
+            <TabPanels>
+              <TabPanel v-for="t in tabs" :key="t">Panel {{ t }}</TabPanel>
+            </TabPanels>
+          </TabGroup>
+          <button @click="add">Insert</button>
+        `,
+        setup() {
+          let tabs = ref<string[]>(['A', 'B', 'C'])
+
+          return {
+            tabs,
+            add() {
+              tabs.value.splice(1, 0, 'D')
+            },
+          }
+        },
+      })
+
+      await new Promise<void>(nextTick)
+
+      assertTabs({ active: 1, tabContents: 'Tab B', panelContents: 'Panel B' })
+
+      // Add some new tabs
+      await click(getByText('Insert'))
+
+      // We should still be at the same tab position, but the tab itself changed
+      assertTabs({ active: 1, tabContents: 'Tab D', panelContents: 'Panel D' })
+    })
+  )
+
   it(
     'should guarantee the order of DOM nodes when reversing the tabs and panels themselves, then performing actions (controlled component)',
     suppressConsoleLogs(async () => {

diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.ts b/packages/@headlessui-vue/src/components/tabs/tabs.ts
@@ -177,9 +177,18 @@ export let TabGroup = defineComponent({
         tabs.value.push(tab)
         tabs.value = sortByDomNode(tabs.value, dom)
 
-        let localSelectedIndex = tabs.value.indexOf(activeTab) ?? selectedIndex.value
-        if (localSelectedIndex !== -1) {
-          selectedIndex.value = localSelectedIndex
+        // When the component is uncontrolled, then we want to maintain the
+        // actively selected tab even if new tabs are inserted or removed before
+        // the active tab.
+        //
+        // When the component is controlled, then we don't want to do this and
+        // instead we want to select the tab based on the `selectedIndex` prop.
+        if (!isControlled.value) {
+          let localSelectedIndex = tabs.value.indexOf(activeTab) ?? selectedIndex.value
+
+          if (localSelectedIndex !== -1) {
+            selectedIndex.value = localSelectedIndex
+          }
         }
       },
       unregisterTab(tab: (typeof tabs)['value'][number]) {