Handle dynamically adding items in list components: Radio Group, Tabs, Listbox, Menu

Fixes #29

Upstream Headless UI handles this for Radio Group but not for the others. I didn't see any reason why not to implement this for the other ones too.

src/lib/components/listbox/Listbox.svelte

@@ -155,7 +155,31 @@
       searchQuery = "";
     },
     registerOption(id: string, dataRef) {
-      options = [...options, { id, dataRef }];
+      if (!$optionsRef) {
+        // We haven't mounted yet so just append
+        options = [...options, { id, dataRef }];
+        return;
+      }
+      let currentActiveOption =
+        activeOptionIndex !== null ? options[activeOptionIndex] : null;
+
+      let orderMap = Array.from(
+        $optionsRef.querySelectorAll('[id^="headlessui-listbox-option-"]')!
+      ).reduce(
+        (lookup, element, index) =>
+          Object.assign(lookup, { [element.id]: index }),
+        {}
+      ) as Record<string, number>;
+
+      let nextOptions = [...options, { id, dataRef }];
+      nextOptions.sort((a, z) => orderMap[a.id] - orderMap[z.id]);
+      options = nextOptions;
+
+      // Maintain the correct item active
+      activeOptionIndex = (() => {
+        if (currentActiveOption === null) return null;
+        return options.indexOf(currentActiveOption);
+      })();
     },
     unregisterOption(id: string) {
       let nextOptions = options.slice();

src/lib/components/listbox/listbox.test.ts

@@ -6,7 +6,7 @@ import {
   ListboxOptions,
 } from ".";
 import { suppressConsoleLogs } from "$lib/test-utils/suppress-console-logs";
-import { render } from "@testing-library/svelte";
+import { act, render } from "@testing-library/svelte";
 import TestRenderer from "$lib/test-utils/TestRenderer.svelte";
 import {
   assertActiveElement,
@@ -48,6 +48,7 @@ import Button from "$lib/internal/elements/Button.svelte";
 import Div from "$lib/internal/elements/Div.svelte";
 import Span from "$lib/internal/elements/Span.svelte";
 import svelte from "svelte-inline-compile";
+import { writable } from "svelte/store";
 
 let mockId = 0;
 jest.mock('../../hooks/use-id', () => {
@@ -192,7 +193,7 @@ describe('Rendering', () => {
 
   describe('ListboxLabel', () => {
     it(
-      'should be possible to render a ListboxLabel using a render prop',
+      'should be possible to render a ListboxLabel using slot props',
       suppressConsoleLogs(async () => {
         render(svelte`
           <Listbox value={undefined} on:change={console.log}>
@@ -295,7 +296,7 @@ describe('Rendering', () => {
     )
 
     it(
-      'should be possible to render a ListboxButton using a render prop and an `as` prop',
+      'should be possible to render a ListboxButton using slot props and an `as` prop',
       suppressConsoleLogs(async () => {
         render(svelte`
           <Listbox value={undefined} onChange={console.log}>
@@ -510,6 +511,61 @@ describe('Rendering', () => {
         })
       })
     )
+
+    it('should guarantee the listbox option order after a few unmounts', async () => {
+      let showFirst = writable(false);
+      render(svelte`
+      <Listbox value={undefined}>
+        <ListboxButton>Trigger</ListboxButton>
+        <ListboxOptions>
+          {#if $showFirst}
+            <ListboxOption value="a">Option A</ListboxOption>
+          {/if}
+          <ListboxOption value="b">Option B</ListboxOption>
+          <ListboxOption value="c">Option C</ListboxOption>
+        </ListboxOptions>
+      </Listbox>
+    `)
+
+      assertListboxButton({
+        state: ListboxState.InvisibleUnmounted,
+        attributes: { id: 'headlessui-listbox-button-1' },
+      })
+      assertListbox({ state: ListboxState.InvisibleUnmounted })
+
+      // Open Listbox
+      await click(getListboxButton())
+
+      let options = getListboxOptions()
+      expect(options).toHaveLength(2)
+      options.forEach(option => assertListboxOption(option))
+
+      // Make the first option active
+      await press(Keys.ArrowDown)
+
+      // Verify that the first listbox option is active
+      assertActiveListboxOption(options[0])
+
+      // Now add a new option dynamically
+      await act(() => showFirst.set(true));
+
+      // New option should be treated correctly
+      options = getListboxOptions()
+      expect(options).toHaveLength(3)
+      options.forEach(option => assertListboxOption(option))
+
+      // Focused option should now be second
+      assertActiveListboxOption(options[1])
+
+      // We should be able to go to the first option
+      await press(Keys.Home)
+      assertActiveListboxOption(options[0])
+
+      // And the last one
+      await press(Keys.End)
+      assertActiveListboxOption(options[2])
+
+    })
   })
 })
 

src/lib/components/menu/Menu.svelte

@@ -111,7 +111,31 @@
       searchQuery = "";
     },
     registerItem(id: string, data: MenuItemData) {
-      items.push({ id, data });
+      if (!$itemsStore) {
+        // We haven't mounted yet so just append
+        items = [...items, { id, data }];
+        return;
+      }
+      let currentActiveItem =
+        activeItemIndex !== null ? items[activeItemIndex] : null;
+
+      let orderMap = Array.from(
+        $itemsStore.querySelectorAll('[id^="headlessui-menu-item-"]')!
+      ).reduce(
+        (lookup, element, index) =>
+          Object.assign(lookup, { [element.id]: index }),
+        {}
+      ) as Record<string, number>;
+
+      let nextItems = [...items, { id, data }];
+      nextItems.sort((a, z) => orderMap[a.id] - orderMap[z.id]);
+      items = nextItems;
+
+      // Maintain the correct item active
+      activeItemIndex = (() => {
+        if (currentActiveItem === null) return null;
+        return items.indexOf(currentActiveItem);
+      })();
     },
     unregisterItem(id: string) {
       let nextItems = items.slice();
@@ -137,7 +161,7 @@
       ...obj,
       menuState,
       buttonStore,
-      itemsStore: itemsStore,
+      itemsStore,
       items,
       searchQuery,
       activeItemIndex,

src/lib/components/menu/menu.test.ts

@@ -1,5 +1,5 @@
 import { assertActiveElement, assertMenu, assertMenuButton, assertMenuButtonLinkedWithMenu, assertMenuItem, assertMenuLinkedWithMenuItem, assertNoActiveMenuItem, getByText, getMenu, getMenuButton, getMenuButtons, getMenuItems, getMenus, MenuState } from "$lib/test-utils/accessibility-assertions";
-import { render } from "@testing-library/svelte";
+import { act, render } from "@testing-library/svelte";
 import { Menu, MenuButton, MenuItem, MenuItems } from ".";
 import { suppressConsoleLogs } from "$lib/test-utils/suppress-console-logs";
 import TestRenderer from "$lib/test-utils/TestRenderer.svelte";
@@ -11,6 +11,7 @@ import Div from "$lib/internal/elements/Div.svelte";
 import Form from "$lib/internal/elements/Form.svelte";
 import Span from "$lib/internal/elements/Span.svelte";
 import svelte from "svelte-inline-compile";
+import { writable } from "svelte/store";
 
 let mockId = 0;
 jest.mock('../../hooks/use-id', () => {
@@ -327,6 +328,61 @@ describe('Rendering', () => {
         })
       })
     )
+
+    it('should guarantee the menu item order after a few unmounts', async () => {
+      let showFirst = writable(false);
+      render(svelte`
+      <Menu>
+        <MenuButton>Trigger</MenuButton>
+        <MenuItems>
+          {#if $showFirst}
+            <MenuItem as="a">Item A</MenuItem>
+          {/if}
+          <MenuItem as="a">Item B</MenuItem>
+          <MenuItem as="a">Item C</MenuItem>
+        </MenuItems>
+      </Menu>
+    `)
+
+      assertMenuButton({
+        state: MenuState.InvisibleUnmounted,
+        attributes: { id: 'headlessui-menu-button-1' },
+      })
+      assertMenu({ state: MenuState.InvisibleUnmounted })
+
+      // Open Listbox
+      await click(getMenuButton())
+
+      let items = getMenuItems()
+      expect(items).toHaveLength(2)
+      items.forEach(item => assertMenuItem(item))
+
+      // Make the first item active
+      await press(Keys.ArrowDown)
+
+      // Verify that the first menu item is active
+      assertMenuLinkedWithMenuItem(items[0])
+
+      // Now add a new option dynamically
+      await act(() => showFirst.set(true));
+
+      // New option should be treated correctly
+      items = getMenuItems()
+      expect(items).toHaveLength(3)
+      items.forEach(item => assertMenuItem(item))
+
+      // Active item should now be second
+      assertMenuLinkedWithMenuItem(items[1])
+
+      // We should be able to go to the first option
+      await press(Keys.Home)
+      assertMenuLinkedWithMenuItem(items[0])
+
+      // And the last one
+      await press(Keys.End)
+      assertMenuLinkedWithMenuItem(items[2])
+
+    })
   })
 })
 

src/lib/components/radio-group/RadioGroup.svelte

@@ -86,7 +86,22 @@
       return true;
     },
     registerOption(action: Option) {
-      options = [...options, action];
+      if (!radioGroupRef) {
+        // We haven't mounted yet so just append
+        options = [...options, action];
+        return;
+      }
+      let orderMap = Array.from(
+        radioGroupRef.querySelectorAll('[id^="headlessui-radiogroup-option-"]')!
+      ).reduce(
+        (lookup, element, index) =>
+          Object.assign(lookup, { [element.id]: index }),
+        {}
+      ) as Record<string, number>;
+
+      let newOptions = [...options, action];
+      newOptions.sort((a, z) => orderMap[a.id] - orderMap[z.id]);
+      options = newOptions;
     },
     unregisterOption(id: Option["id"]) {
       options = options.filter((radio) => radio.id !== id);

src/lib/components/radio-group/radio-group.test.ts

@@ -124,8 +124,7 @@ describe('Rendering', () => {
     assertNotFocusable(getByText('Dine in'))
   })
 
-  // TODO: fix this test!
+  it('should guarantee the radio option order after a few unmounts', async () => {
-  it.skip('should guarantee the radio option order after a few unmounts', async () => {
     render(svelte`
       <script>
         let showFirst = false;

src/lib/components/tabs/Tab.svelte

@@ -27,7 +27,7 @@
     return () => $api.unregisterTab(tabRef);
   });
 
-  $: myIndex = $api.tabs.indexOf(tabRef);
+  $: myIndex = tabRef ? $api.tabs.indexOf(tabRef) : -1;
   $: selected = myIndex === $api.selectedIndex;
 
   function handleKeyDown(e: CustomEvent) {

src/lib/components/tabs/TabGroup.svelte

@@ -9,9 +9,11 @@
     orientation: "vertical" | "horizontal";
     activation: "auto" | "manual";
 
-    tabs: (HTMLElement | null)[];
+    tabs: HTMLElement[];
     panels: PanelData[];
 
+    listRef: Writable<HTMLElement | null>;
+
     // State mutators
     setSelectedIndex(index: number): void;
     registerTab(tab: HTMLElement | null): void;
@@ -63,6 +65,7 @@
   let selectedIndex: StateDefinition["selectedIndex"] = null;
   let tabs: StateDefinition["tabs"] = [];
   let panels: StateDefinition["panels"] = [];
+  let listRef: StateDefinition["listRef"] = writable(null);
 
   const dispatch = createEventDispatcher();
 
@@ -72,13 +75,39 @@
     activation: manual ? "manual" : "auto",
     tabs,
     panels,
+    listRef,
     setSelectedIndex(index: number) {
       if (selectedIndex === index) return;
       selectedIndex = index;
       dispatch("change", index);
     },
     registerTab(tab: typeof tabs[number]) {
-      if (!tabs.includes(tab)) tabs = [...tabs, tab];
+      if (tabs.includes(tab)) return;
+      if (!$listRef) {
+        // We haven't mounted yet so just append
+        tabs = [...tabs, tab];
+        return;
+      }
+      let currentSelectedTab =
+        selectedIndex !== null ? tabs[selectedIndex] : null;
+
+      let orderMap = Array.from(
+        $listRef.querySelectorAll('[id^="headlessui-tabs-tab-"]')!
+      ).reduce(
+        (lookup, element, index) =>
+          Object.assign(lookup, { [element.id]: index }),
+        {}
+      ) as Record<string, number>;
+
+      let nextTabs = [...tabs, tab];
+      nextTabs.sort((a, z) => orderMap[a.id] - orderMap[z.id]);
+      tabs = nextTabs;
+
+      // Maintain the correct item active
+      selectedIndex = (() => {
+        if (currentSelectedTab === null) return null;
+        return tabs.indexOf(currentSelectedTab);
+      })();
     },
     unregisterTab(tab: typeof tabs[number]) {
       tabs = tabs.filter((t) => t !== tab);

src/lib/components/tabs/TabList.svelte

@@ -11,6 +11,8 @@
   export let use: HTMLActionArray = [];
 
   let api = useTabsContext("TabList");
+  let listRef = $api.listRef;
+
   $: propsWeControl = {
     role: "tablist",
     "aria-orientation": $api.orientation,
@@ -23,6 +25,7 @@
   {...{ ...$$restProps, ...propsWeControl }}
   {as}
   {slotProps}
+  bind:el={$listRef}
   use={[...use, forwardEvents]}
   name={"TabList"}
 >

src/lib/components/tabs/tabs.test.ts

@@ -1,4 +1,4 @@
-import { render } from "@testing-library/svelte";
+import { act, render } from "@testing-library/svelte";
 import { suppressConsoleLogs } from "$lib/test-utils/suppress-console-logs";
 import TestRenderer from "$lib/test-utils/TestRenderer.svelte";
 import { Tab, TabGroup, TabList, TabPanel, TabPanels } from ".";
@@ -6,6 +6,7 @@ import { assertActiveElement, assertTabs, getByText, getTabs } from "$lib/test-u
 import { click, Keys, press, shift } from "$lib/test-utils/interactions";
 import Button from "$lib/internal/elements/Button.svelte";
 import svelte from "svelte-inline-compile";
+import { writable } from "svelte/store";
 
 let mockId = 0;
 jest.mock('../../hooks/use-id', () => {
@@ -431,6 +432,48 @@ describe('Rendering', () => {
       })
 
     })
+
+    it('should guarantee the tab order after a few unmounts', async () => {
+      let showFirst = writable(false);
+      render(svelte`
+      <TabGroup>
+        <TabList>
+          {#if $showFirst}
+            <Tab>Tab 1</Tab>
+          {/if}
+          <Tab>Tab 2</Tab>
+          <Tab>Tab 3</Tab>
+        </TabList>
+      </TabGroup>
+    `)
+
+      let tabs = getTabs()
+      expect(tabs).toHaveLength(2)
+
+      // Make the first tab active
+      await press(Keys.Tab)
+
+      // Verify that the first tab is active
+      assertTabs({ active: 0 })
+
+      // Now add a new tab dynamically
+      await act(() => showFirst.set(true));
+
+      // New tab should be treated correctly
+      tabs = getTabs()
+      expect(tabs).toHaveLength(3)
+
+      // Active tab should now be second
+      assertTabs({ active: 1 })
+
+      // We should be able to go to the first tab
+      await press(Keys.Home)
+      assertTabs({ active: 0 })
+
+      // And the last one
+      await press(Keys.End)
+      assertTabs({ active: 2 })
+    })
   })
 })