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

When adding new tabs, the selected tab has an incorrect index #368

Closed
weaseldotro opened this issue Oct 26, 2020 · 2 comments
Closed

When adding new tabs, the selected tab has an incorrect index #368

weaseldotro opened this issue Oct 26, 2020 · 2 comments

Comments

@weaseldotro
Copy link
Contributor

The current implementation of Tabs assumes that new tabs are always added to the end of the tabs' list, it doesn't take into consideration that tabs can be inserted in the middle of the list. So the index of the new tab is always incremented based on the last index, it is not calculated based on the tab position.

Example: https://codesandbox.io/s/nervous-meitner-s0pju

Click on toggle green tab button, then click on the tab titled green. The green tab is the second one in the list, so it should have index of 1, however it has an index of 2.

Try playing with the buttons and toggling tabs then clicking on them. The tab index will be incremented each time a new tab is added, so the selected prop will also have an incorrect value.

The correct implementation should recalculate the indexes of the tabs each time a tab is added or removed.

@metonym metonym added the bug Something isn't working label Oct 27, 2020
@metonym metonym mentioned this issue Oct 27, 2020
10 tasks
@metonym
Copy link
Collaborator

metonym commented Oct 27, 2020

Agreed, this is unexpected.

For the case of inserting instead of appending a new tab – is it possible in the current approach of composition?

Current

<Tabs>
  <Tab ... />
  <Tab ... />
  <div slot="content">
    <TabContent .../>
    <TabContent .../>
  </div>
</Tabs>

The API may need to change so that tabs are passed as a single prop, and display is controlled via slots:

<Tabs tabs={[{id: 0, label: '1'}, {id: 1, label: '2'}]}>
  <div slot="label" let:index let:id>
    Label {id} {index}
  </div>
  <div slot="content" let:index let:id>
    ...
  </div>
</Tabs>

@metonym metonym removed the bug Something isn't working label Dec 6, 2020
@metonym
Copy link
Collaborator

metonym commented Dec 6, 2020

If you're reading this from the future, the work around is to use the #key API to re-render the component.

<script>
  import { Button, Tabs, Tab, TabContent } from "carbon-components-svelte";

  let tabs = [
    { title: "blue", visible: true },
    { title: "green", visible: false },
    { title: "red", visible: true },
  ];

  const toggleTab = (title) => {
    tabs = tabs.map((tab) => ({
      ...tab,
      visible: tab.title === title ? !tab.visible : tab.visible,
    }));
  };

  let selected = 0;
</script>

<svelte:options immutable />

{#key tabs}
  <Tabs bind:selected>
    {#each tabs as tab (tab.title)}
      {#if tab.visible}
        <Tab label="{tab.title}" />
      {/if}
    {/each}
    <div slot="content">
      {#each tabs as tab (tab.title)}
        {#if tab.visible}
          <TabContent>Content for {tab.title}</TabContent>
        {/if}
      {/each}
    </div>
  </Tabs>
{/key}

<p>Selected index: {selected}</p>

{#each tabs as { title } (title)}
  <Button on:click="{() => toggleTab(title)}">Toggle {title} tab</Button>
{/each}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants