kobaltedev / kobalte

A UI toolkit for building accessible web apps and design systems with SolidJS.
https://kobalte.dev
MIT License
1.22k stars 62 forks source link

[Tabs] First tab gets selected when adding a new tab and setting it as selected simultaneously #220

Closed jakst closed 1 year ago

jakst commented 1 year ago

Describe the bug With controlled values for the Tabs component, it is not possible to add a new tab dynamically and select it at the same time. Instead the first tab is selected.

To Reproduce Steps to reproduce the behavior:

  1. Open this stacbklitz: https://stackblitz.com/edit/github-wkw3ra?file=src%2FApp.tsx
  2. Click on the "Add tab" button
  3. The first tab is selected instead of the last
import { batch, Component, createSignal, For } from 'solid-js';

import styles from './App.module.css';
import { Tabs } from '@kobalte/core';

const App: Component = () => {
  const [selectedTab, setSelectedTab] = createSignal('Banana');

  const [tabs, setTabs] = createSignal(['Orange', 'Banana', 'Pear']);

  // BUG:
  // If we add a tab and set it as selected, the Tab component
  // resets the selected tab to the first tab in the list.
  // Expected: The newly added tab to be selected.
  function addTab() {
    batch(() => {
      const newTab = `Apple${tabs().length - 2}`;
      setTabs((v) => [...v, newTab]);
      setSelectedTab(newTab);
    });
  }

  return (
    <div class={styles.App}>
      <button onClick={addTab}>Add tab</button>

      <Tabs.Root
        activationMode="manual"
        value={selectedTab()}
        onChange={setSelectedTab}
      >
        <Tabs.List>
          <For each={tabs()}>
            {(tab) => (
              <Tabs.Trigger class="tabs__trigger" value={tab}>
                {tab}
              </Tabs.Trigger>
            )}
          </For>
        </Tabs.List>
      </Tabs.Root>
    </div>
  );
};

export default App;
bram209 commented 1 year ago

Related #218?

fabien-ml commented 1 year ago

Hi, thanks for reporting the issue.

The reason this doesn't works when using batch is because the selectedTab is updated before the new list of tabs is rendered and registered to Tabs.Root. So when the internal effect run to set the selected tab it doesn't find the new tab (at the moment) and fallback to the first one.

I don't have a solution for this currently except not using batch.

jakst commented 1 year ago

Ok, I guess I can work around using batch. I see why that would be hard to solve on your end. Thanks for looking into it!