Skip to content

fix(tabs): not able to stop tab switcing when controlled#4635

Merged
mimarz merged 6 commits intomainfrom
tabs/react-controlled
Mar 20, 2026
Merged

fix(tabs): not able to stop tab switcing when controlled#4635
mimarz merged 6 commits intomainfrom
tabs/react-controlled

Conversation

@Barsnes
Copy link
Member

@Barsnes Barsnes commented Mar 17, 2026

Summary

resolves #4617

I added a storybook story as well. We can remove this, but I think it's nice to have, In that story you can never select tab 2

Checks

@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2026

🦋 Changeset detected

Latest commit: 4f6a79d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@digdir/designsystemet-react Patch
@digdir/designsystemet Patch
@digdir/designsystemet-css Patch
@digdir/designsystemet-types Patch
@digdir/designsystemet-web Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2026

Preview deployments for this pull request:

storybook - 19. Mar 2026 - 09:04

@Barsnes Barsnes marked this pull request as ready for review March 17, 2026 13:15
Copy link
Collaborator

@mimarz mimarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onChange is triggered twice with the same value. This happens when opening story and when clicking on "the other" tab.

export function TabsTest(): ReactElement {
  const tabName1 = 'tab1';
  const tabName2 = 'tab2';
  const [selectedTab, setSelectedTab] = useState<string>(tabName1);

  const handleChange = (tabName: string) => {
    if (
      confirm('Are you sure you want to select another tab?')
    ) {
      setSelectedTab(tabName);
    }
  };

  return (
    <Tabs value={selectedTab} onChange={handleChange}>
      <Tabs.List>
        <Tabs.Tab value={tabName1}>Tab 1</Tabs.Tab>
        <Tabs.Tab value={tabName2}>Tab 2</Tabs.Tab>
      </Tabs.List>
      <Tabs.Panel value={tabName1}>Content of tab 1</Tabs.Panel>
      <Tabs.Panel value={tabName2}>Content of tab 2</Tabs.Panel>
    </Tabs>
  );
}

Looks like onClick on ds-tab is triggering twice in both use-cases.
Adding the isTrusted check back seemed to resolve the problem. 😅
Any reason why we only check isTrusted for onChange before? 🤔
Anyhow, rolling back that change seemed to fix it.

onClick={(e: MouseEvent<DSTabElement>) => {
  if (e.isTrusted) {
    onChange?.(value); // Only call onChange is user actually clicked, not when programmatically clicked/controlled
  }
  onClick?.(e);
}}


await user.click(screen.getByRole('tab', { name: 'Tab 2' }));

expect(onChange).toHaveBeenCalledWith('value2');
Copy link
Collaborator

@mimarz mimarz Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(onChange).toHaveBeenCalledWith('value2');
expect(onChange).toHaveBeenCalledWith('value2');
expect(onChange).toHaveBeenCalledOnce();

@Barsnes
Copy link
Member Author

Barsnes commented Mar 19, 2026

onChange is triggered twice with the same value. This happens when opening story and when clicking on "the other" tab.

export function TabsTest(): ReactElement {
  const tabName1 = 'tab1';
  const tabName2 = 'tab2';
  const [selectedTab, setSelectedTab] = useState<string>(tabName1);

  const handleChange = (tabName: string) => {
    if (
      confirm('Are you sure you want to select another tab?')
    ) {
      setSelectedTab(tabName);
    }
  };

  return (
    <Tabs value={selectedTab} onChange={handleChange}>
      <Tabs.List>
        <Tabs.Tab value={tabName1}>Tab 1</Tabs.Tab>
        <Tabs.Tab value={tabName2}>Tab 2</Tabs.Tab>
      </Tabs.List>
      <Tabs.Panel value={tabName1}>Content of tab 1</Tabs.Panel>
      <Tabs.Panel value={tabName2}>Content of tab 2</Tabs.Panel>
    </Tabs>
  );
}

Looks like onClick on ds-tab is triggering twice in both use-cases. Adding the isTrusted check back seemed to resolve the problem. 😅 Any reason why we only check isTrusted for onChange before? 🤔 Anyhow, rolling back that change seemed to fix it.

onClick={(e: MouseEvent<DSTabElement>) => {
  if (e.isTrusted) {
    onChange?.(value); // Only call onChange is user actually clicked, not when programmatically clicked/controlled
  }
  onClick?.(e);
}}

I rolled it back, but changed it sightly

@Barsnes Barsnes requested a review from mimarz March 19, 2026 08:12
Copy link
Collaborator

@mimarz mimarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Suggest adding the suggestion for test as to guard ourselves (or other developers) from removing the isTrusted in the future :)

@Barsnes
Copy link
Member Author

Barsnes commented Mar 20, 2026

LGTM 👍

Suggest adding the suggestion for test as to guard ourselves (or other developers) from removing the isTrusted in the future :)

It has been added
image

@mimarz
Copy link
Collaborator

mimarz commented Mar 20, 2026

LGTM 👍
Suggest adding the suggestion for test as to guard ourselves (or other developers) from removing the isTrusted in the future :)

It has been added image

ahh, didn't catch that as the suggestion was still up. Superduper!

@mimarz mimarz merged commit 191fa54 into main Mar 20, 2026
16 checks passed
@mimarz mimarz deleted the tabs/react-controlled branch March 20, 2026 08:15
@github-actions github-actions bot mentioned this pull request Mar 19, 2026
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

Successfully merging this pull request may close these issues.

The Tabs component enforces tab switching when controlled

3 participants