Skip to content

fix(tabs): honor hash-linked nav tabs #12303

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.

Sign up for GitHub

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

Open
tarunvashishth wants to merge 3 commits into
base: main
Choose a base branch
from
+285 −25
Open

fix(tabs): honor hash-linked nav tabs #12303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17b55f9
fix(tabs): honor hash-linked nav tabs
tarunvashishth Mar 28, 2026
c6d6d1c
fix(tabs): preserve initial hash selection
tarunvashishth Mar 29, 2026
ca039ec
chore(Tabs): Fix re-render bug
rebeccaalpert Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 51 additions & 7 deletions packages/react-core/src/components/Tabs/Tabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,25 @@ const variantStyle = {
secondary: styles.modifiers.secondary
};

const getHashFromHref = (href?: string) => {
const hashIndex = href?.indexOf('#') ?? -1;

return hashIndex >= 0 ? href.slice(hashIndex) : undefined;
};

const getTabHashActiveKey = ({ children, component, isNav }: Pick<TabsProps, 'children' | 'component' | 'isNav'>) => {
if (!canUseDOM || !(component === TabsComponent.nav || isNav) || !window.location.hash) {
return undefined;
}

return Children.toArray(children)
.filter((child): child is TabElement => isValidElement(child))
.filter(({ props }) => !props.isHidden)
.find(
({ props }) => !props.isDisabled && !props.isAriaDisabled && getHashFromHref(props.href) === window.location.hash
)?.props.eventKey;
};
Comment on lines +149 to +160

@coderabbitai coderabbitai Bot Mar 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if Tabs tests have any hash-related setup
rg -n "location.hash|window.location" packages/react-core/src/components/Tabs/__tests__/

Repository: patternfly/patternfly-react

Length of output: 53

Add test coverage for the getTabHashActiveKey function with hash-based navigation.

The new getTabHashActiveKey function accesses window.location.hash and should be tested to ensure the hash-based tab selection works correctly and prevent regressions. Currently, no hash-related test setup exists in the Tabs tests, while the Nav component tests demonstrate the pattern with window.location.hash setup in a beforeEach block.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/react-core/src/components/Tabs/Tabs.tsx` around lines 145 - 156,
Tests are missing for getTabHashActiveKey to verify hash-based selection; add
unit tests in the Tabs spec that set window.location.hash (use a beforeEach to
assign a hash and afterEach to clear it) and assert getTabHashActiveKey (via
rendering Tabs or importing the function if exported) returns the expected
eventKey when a child TabElement has matching href, and does not return keys for
hidden/disabled tabs or when component/isNav conditions are unmet; target the
getTabHashActiveKey logic and exercise Tabs/TabsProps behavior with TabElement
props (href, isHidden, isDisabled, isAriaDisabled) so hash-based navigation is
covered.


interface TabsState {
/** Used to signal if the scroll buttons should be used */
enableScrollButtons: boolean;
Expand All @@ -152,7 +171,8 @@ interface TabsState {
disableBackScrollButton: boolean;
disableForwardScrollButton: boolean;
shownKeys: (string | number)[];
uncontrolledActiveKey: number | string;
uncontrolledActiveKey: number | string | undefined;
initialActiveKey: number | string | undefined;
uncontrolledIsExpandedLocal: boolean;
overflowingTabCount: number;
isInitializingAccent: boolean;
Expand All @@ -167,14 +187,20 @@ class Tabs extends Component<TabsProps, TabsState> {
private direction = 'ltr';
constructor(props: TabsProps) {
super(props);
const hashActiveKey = getTabHashActiveKey(props);

this.state = {
enableScrollButtons: false,
showScrollButtons: false,
renderScrollButtons: false,
disableBackScrollButton: true,
disableForwardScrollButton: true,
shownKeys: this.props.defaultActiveKey !== undefined ? [this.props.defaultActiveKey] : [this.props.activeKey], // only for mountOnEnter case
uncontrolledActiveKey: this.props.defaultActiveKey,
shownKeys:
this.props.defaultActiveKey !== undefined

@kmcfaul kmcfaul Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This check is a little nested, hashActiveKey ?? this.props.defaultActiveKey ?? this.props.activeKey should mean the same thing and read a little clearer I think.

Not a blocker overall.

? [hashActiveKey ?? this.props.defaultActiveKey]
: [hashActiveKey ?? this.props.activeKey], // only for mountOnEnter case
uncontrolledActiveKey: hashActiveKey ?? this.props.defaultActiveKey,
initialActiveKey: this.props.defaultActiveKey === undefined ? hashActiveKey : undefined,
uncontrolledIsExpandedLocal: this.props.defaultIsExpanded,
overflowingTabCount: 0,
isInitializingAccent: true,
Expand Down Expand Up @@ -221,14 +247,26 @@ class Tabs extends Component<TabsProps, TabsState> {
eventKey: number | string,
tabContentRef: React.RefObject<any>
) {
const { shownKeys } = this.state;
const { onSelect, defaultActiveKey } = this.props;
const { shownKeys, initialActiveKey } = this.state;
const { onSelect, defaultActiveKey, component, isNav } = this.props;

if (event.currentTarget instanceof HTMLAnchorElement && event.currentTarget.hasAttribute('href')) {
event.preventDefault();

if (canUseDOM && (component === TabsComponent.nav || isNav)) {
window.history.pushState(null, '', event.currentTarget.href);
}
}

// if defaultActiveKey Tabs are uncontrolled, set new active key internally
if (defaultActiveKey !== undefined) {
this.setState({
uncontrolledActiveKey: eventKey
});
} else {
if (initialActiveKey !== undefined) {
this.setState({ initialActiveKey: undefined });
}
onSelect(event, eventKey);
}

Expand Down Expand Up @@ -401,8 +439,9 @@ class Tabs extends Component<TabsProps, TabsState> {
componentDidUpdate(prevProps: TabsProps, prevState: TabsState) {
this.direction = getLanguageDirection(this.tabList.current);
const { activeKey, mountOnEnter, isOverflowHorizontal, children, defaultActiveKey } = this.props;
const { shownKeys, overflowingTabCount, enableScrollButtons, uncontrolledActiveKey } = this.state;
const { shownKeys, overflowingTabCount, enableScrollButtons, uncontrolledActiveKey, initialActiveKey } = this.state;
const isOnCloseUpdate = !!prevProps.onClose !== !!this.props.onClose;

if (
(defaultActiveKey !== undefined && prevState.uncontrolledActiveKey !== uncontrolledActiveKey) ||
(defaultActiveKey === undefined && prevProps.activeKey !== activeKey) ||
Expand All @@ -417,6 +456,10 @@ class Tabs extends Component<TabsProps, TabsState> {
});
}

if (defaultActiveKey === undefined && prevProps.activeKey !== activeKey && initialActiveKey !== undefined) {
this.setState({ initialActiveKey: undefined });
}

if (
prevProps.children &&
children &&
Expand Down Expand Up @@ -517,6 +560,7 @@ class Tabs extends Component<TabsProps, TabsState> {
disableForwardScrollButton,
shownKeys,
uncontrolledActiveKey,
initialActiveKey,
uncontrolledIsExpandedLocal,
overflowingTabCount,
isInitializingAccent,
Expand All @@ -533,7 +577,7 @@ class Tabs extends Component<TabsProps, TabsState> {

const defaultComponent = isNav && !component ? 'nav' : 'div';
const Component: any = component !== undefined ? component : defaultComponent;
const localActiveKey = defaultActiveKey !== undefined ? uncontrolledActiveKey : activeKey;
const localActiveKey = defaultActiveKey !== undefined ? uncontrolledActiveKey : (initialActiveKey ?? activeKey);

const isExpandedLocal = defaultIsExpanded !== undefined ? uncontrolledIsExpandedLocal : isExpanded;
/* Uncontrolled expandable tabs */
Expand Down
218 changes: 217 additions & 1 deletion packages/react-core/src/components/Tabs/__tests__/Tabs.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { TabTitleText } from '../TabTitleText';
import { TabTitleIcon } from '../TabTitleIcon';
import { TabContent } from '../TabContent';
import { TabContentBody } from '../TabContentBody';
import { createRef } from 'react';
import { createRef, useState } from 'react';

jest.mock('../../../helpers/GenerateId/GenerateId');

Expand Down Expand Up @@ -119,6 +119,222 @@ test(`Does not render with class ${styles.modifiers.initializingAccent} when com
jest.useRealTimers();
});

describe('hash-based nav selection', () => {
beforeEach(() => {
window.location.hash = '#/items/2';
});

afterEach(() => {
window.location.hash = '';
});

test('should select the nav tab that matches the current URL hash on initial render', () => {
render(
<Tabs activeKey={0} onSelect={() => undefined} component="nav">
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
<Tab eventKey={2} title={<TabTitleText>Tab item 3</TabTitleText>} href="#/items/3">
Tab item 3
</Tab>
</Tabs>
);

expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'false');
});

test('should select the nav tab that matches the current URL hash when isNav is enabled', () => {
render(
<Tabs activeKey={0} onSelect={() => undefined} isNav>
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
</Tabs>
);

expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'false');
});

test('should fall back to activeKey when no tab href matches the current hash', () => {
window.location.hash = '#/items/unknown';

render(
<Tabs activeKey={0} onSelect={() => undefined} component="nav">
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
</Tabs>
);

expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'false');
});

test('should fall back to activeKey when the URL hash is empty', () => {
window.location.hash = '';

render(
<Tabs activeKey={0} onSelect={() => undefined} component="nav">
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
</Tabs>
);

expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'false');
});

test('should respect later controlled selections after the initial hash match', async () => {
const user = userEvent.setup();
const ControlledTabs = () => {
const [activeKey, setActiveKey] = useState<string | number>(0);

return (
<Tabs activeKey={activeKey} onSelect={(_event, eventKey) => setActiveKey(eventKey)} component="nav">
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
<Tab eventKey={2} title={<TabTitleText>Tab item 3</TabTitleText>} href="#/items/3">
Tab item 3
</Tab>
</Tabs>
);
};

render(<ControlledTabs />);

expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'true');

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

expect(screen.getByRole('tab', { name: 'Tab item 3' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'false');
});

test('should use the URL hash to initialize uncontrolled nav tabs', async () => {
const user = userEvent.setup();

render(
<Tabs defaultActiveKey={0} isNav>
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
<Tab eventKey={2} title={<TabTitleText>Tab item 3</TabTitleText>} href="#/items/3">
Tab item 3
</Tab>
</Tabs>
);

expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'true');

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

expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'false');
});

test('should ignore hidden, disabled and aria-disabled tabs when matching the current hash', () => {
render(
<Tabs activeKey={0} onSelect={() => undefined} component="nav">
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Hidden tab</TabTitleText>} href="#/items/2" isHidden>
Hidden tab
</Tab>
<Tab eventKey={2} title={<TabTitleText>Disabled tab</TabTitleText>} href="#/items/2" isDisabled>
Disabled tab
</Tab>
<Tab eventKey={3} title={<TabTitleText>Aria disabled tab</TabTitleText>} href="#/items/2" isAriaDisabled>
Aria disabled tab
</Tab>
</Tabs>
);

expect(screen.queryByRole('tab', { name: 'Hidden tab' })).not.toBeInTheDocument();
expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Disabled tab' })).toHaveAttribute('aria-selected', 'false');
expect(screen.getByRole('tab', { name: 'Aria disabled tab' })).toHaveAttribute('aria-selected', 'false');
});

test('should ignore the current URL hash when nav behavior is not enabled', () => {
render(
<Tabs activeKey={0} onSelect={() => undefined}>
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
</Tabs>
);

expect(screen.getByRole('tab', { name: 'Tab item 1' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Tab item 2' })).toHaveAttribute('aria-selected', 'false');
});

test('should prevent default anchor navigation when selecting a tab with an href', async () => {
const user = userEvent.setup();
window.location.hash = '';

render(
<Tabs activeKey={0} onSelect={() => undefined}>
<Tab eventKey={0} title={<TabTitleText>Tab item 1</TabTitleText>} href="#/items/1">
Tab item 1
</Tab>
<Tab eventKey={1} title={<TabTitleText>Tab item 2</TabTitleText>} href="#/items/2">
Tab item 2
</Tab>
</Tabs>
);

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

expect(window.location.hash).toBe('');
});

test('should update the URL hash when selecting a nav tab with an href', async () => {
const user = userEvent.setup();
window.location.hash = '';

render(
<Tabs activeKey={0} onSelect={() => undefined} isNav>
<Tab eventKey={0} title={<TabTitleText>Users</TabTitleText>} href="#users">
Users
</Tab>
<Tab eventKey={1} title={<TabTitleText>Containers</TabTitleText>} href="#containers">
Containers
</Tab>
</Tabs>
);

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

expect(window.location.hash).toBe('#containers');
});
});

test(`Renders with class ${styles.modifiers.initializingAccent} when uncontrolled expandable component initially mounts`, async () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

Expand Down
Loading
Loading