Skip to content

Commit

Permalink
[Tabs] Fix tabs activating incorrectly on non-primary button clicks (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
mj12albert authored Jan 16, 2025
1 parent 3b4f1ca commit 8256095
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 21 deletions.
32 changes: 32 additions & 0 deletions packages/react/src/tabs/root/TabsRoot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,23 @@ describe('<Tabs.Root />', () => {
});

describe('prop: onValueChange', () => {
it('should call onValueChange on pointerdown', async () => {
const handleChange = spy();
const handlePointerDown = spy();
const { getAllByRole, user } = await render(
<Tabs.Root value={0} onValueChange={handleChange}>
<Tabs.List>
<Tabs.Tab value={0} />
<Tabs.Tab value={1} onPointerDown={handlePointerDown} />
</Tabs.List>
</Tabs.Root>,
);

await user.pointer({ keys: '[MouseLeft>]', target: getAllByRole('tab')[1] });
expect(handleChange.callCount).to.equal(1);
expect(handlePointerDown.callCount).to.equal(1);
});

it('should call onValueChange when clicking', async () => {
const handleChange = spy();
const { getAllByRole } = await render(
Expand All @@ -184,6 +201,21 @@ describe('<Tabs.Root />', () => {
expect(handleChange.firstCall.args[0]).to.equal(1);
});

it('should not call onValueChange on non-main button clicks', async () => {
const handleChange = spy();
const { getAllByRole } = await render(
<Tabs.Root value={0} onValueChange={handleChange}>
<Tabs.List>
<Tabs.Tab value={0} />
<Tabs.Tab value={1} />
</Tabs.List>
</Tabs.Root>,
);

fireEvent.click(getAllByRole('tab')[1], { button: 2 });
expect(handleChange.callCount).to.equal(0);
});

it('should not call onValueChange when already selected', async () => {
const handleChange = spy();
const { getAllByRole } = await render(
Expand Down
66 changes: 45 additions & 21 deletions packages/react/src/tabs/tab/useTabsTab.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use client';
import * as React from 'react';
import { mergeReactProps } from '../../utils/mergeReactProps';
import { ownerDocument } from '../../utils/owner';
import { useEnhancedEffect } from '../../utils/useEnhancedEffect';
import { useForkRef } from '../../utils/useForkRef';
import { useBaseUiId } from '../../utils/useBaseUiId';
Expand Down Expand Up @@ -75,32 +76,56 @@ function useTabsTab(parameters: useTabsTab.Parameters): useTabsTab.ReturnValue {

const tabPanelId = index > -1 ? getTabPanelIdByTabValueOrIndex(valueParam, index) : undefined;

const isPressingRef = React.useRef(false);
const isMainButtonRef = React.useRef(false);

const getRootProps = React.useCallback(
(externalProps = {}) => {
return mergeReactProps<'button'>(
externalProps,
mergeReactProps<'button'>(
{
role: 'tab',
'aria-controls': tabPanelId,
'aria-selected': selected,
id,
ref: handleRef,
onClick(event) {
{
role: 'tab',
'aria-controls': tabPanelId,
'aria-selected': selected,
id,
ref: handleRef,
onClick(event) {
if (selected) {
return;
}

onTabActivation(tabValue, event.nativeEvent);
},
onFocus(event) {
if (!activateOnFocus || selected) {
return;
}

if (!isPressingRef.current || (isPressingRef.current && isMainButtonRef.current)) {
onTabActivation(tabValue, event.nativeEvent);
},
onFocus(event) {
if (!activateOnFocus) {
return;
}

if (selectedTabValue !== tabValue) {
onTabActivation(tabValue, event.nativeEvent);
}
},
}
},
onPointerDown(event) {
if (selected) {
return;
}

isPressingRef.current = true;

function handlePointerUp() {
isPressingRef.current = false;
isMainButtonRef.current = false;
}

if (!event.button || event.button === 0) {
isMainButtonRef.current = true;

const doc = ownerDocument(event.currentTarget);
doc.addEventListener('pointerup', handlePointerUp, { once: true });
}
},
mergeReactProps(getItemProps(), getButtonProps()),
),
},
mergeReactProps(getItemProps(), getButtonProps()),
);
},
[
Expand All @@ -111,7 +136,6 @@ function useTabsTab(parameters: useTabsTab.Parameters): useTabsTab.ReturnValue {
id,
onTabActivation,
selected,
selectedTabValue,
tabPanelId,
tabValue,
],
Expand Down

0 comments on commit 8256095

Please sign in to comment.