Skip to content
Open
Changes from 1 commit
Commits
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
31 changes: 29 additions & 2 deletions packages/react-core/src/components/Tabs/Tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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 +145 to +156
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 Down Expand Up @@ -164,13 +183,18 @@ 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
shownKeys:
this.props.defaultActiveKey !== undefined
? [hashActiveKey ?? this.props.defaultActiveKey]
: [hashActiveKey ?? this.props.activeKey], // only for mountOnEnter case
uncontrolledActiveKey: this.props.defaultActiveKey,
uncontrolledIsExpandedLocal: this.props.defaultIsExpanded,
ouiaStateId: getDefaultOUIAId(Tabs.displayName),
Expand Down Expand Up @@ -221,6 +245,7 @@ class Tabs extends Component<TabsProps, TabsState> {
) {
const { shownKeys } = this.state;
const { onSelect, defaultActiveKey } = this.props;

// if defaultActiveKey Tabs are uncontrolled, set new active key internally
if (defaultActiveKey !== undefined) {
this.setState({
Expand Down Expand Up @@ -401,6 +426,7 @@ class Tabs extends Component<TabsProps, TabsState> {
const { activeKey, mountOnEnter, isOverflowHorizontal, children, defaultActiveKey } = this.props;
const { shownKeys, overflowingTabCount, enableScrollButtons, uncontrolledActiveKey } = this.state;
const isOnCloseUpdate = !!prevProps.onClose !== !!this.props.onClose;

if (
(defaultActiveKey !== undefined && prevState.uncontrolledActiveKey !== uncontrolledActiveKey) ||
(defaultActiveKey === undefined && prevProps.activeKey !== activeKey) ||
Expand Down Expand Up @@ -530,7 +556,8 @@ class Tabs extends Component<TabsProps, TabsState> {
const uniqueId = id || getUniqueId();
const defaultComponent = isNav && !component ? 'nav' : 'div';
const Component: any = component !== undefined ? component : defaultComponent;
const localActiveKey = defaultActiveKey !== undefined ? uncontrolledActiveKey : activeKey;
const hashActiveKey = getTabHashActiveKey({ children, component, isNav });
const localActiveKey = hashActiveKey ?? (defaultActiveKey !== undefined ? uncontrolledActiveKey : activeKey);

const isExpandedLocal = defaultIsExpanded !== undefined ? uncontrolledIsExpandedLocal : isExpanded;
/* Uncontrolled expandable tabs */
Expand Down
Loading