Skip to content

feat(Nav): add icon prop to NavExpandable #12448

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

Closed
rebeccaalpert wants to merge 1 commit into from
+114 −0
Closed

feat(Nav): add icon prop to NavExpandable #12448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions packages/react-core/src/components/Nav/NavExpandable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export interface NavExpandableProps
extends Omit<React.DetailedHTMLProps<React.LiHTMLAttributes<HTMLLIElement>, HTMLLIElement>, 'title'>, OUIAProps {
/** Title content shown for the expandable list */
title: React.ReactNode;
/** Icon added before the nav expandable children. */
icon?: React.ReactNode;
/** If defined, screen readers will read this text instead of the list title */
srText?: string;
/** Boolean to pragmatically expand or collapse section */
Expand Down Expand Up @@ -85,6 +87,7 @@ class NavExpandable extends Component<NavExpandableProps, NavExpandableState> {
render() {
const {
title,
icon,
srText,
children,
className,
Expand Down Expand Up @@ -132,6 +135,7 @@ class NavExpandable extends Component<NavExpandableProps, NavExpandableState> {
tabIndex={isSidebarOpen ? null : -1}
{...buttonProps}
>
{icon && <span className={css(styles.navLinkIcon)}>{icon}</span>}

@coderabbitai coderabbitai Bot Jun 11, 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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check NavItem icon wrapper implementation for aria-hidden

rg -nP -A2 -B2 'navLinkIcon.*icon' --type=tsx --type=ts packages/react-core/src/components/Nav/NavItem.tsx

Repository: patternfly/patternfly-react

Length of output: 99

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate NavItem.tsx and inspect icon wrapper + aria-hidden usage
ls -la packages/react-core/src/components/Nav/ || true

if [ -f packages/react-core/src/components/Nav/NavItem.tsx ]; then
  rg -n 'navLinkIcon' packages/react-core/src/components/Nav/NavItem.tsx
  rg -n 'aria-hidden' packages/react-core/src/components/Nav/NavItem.tsx || true
  # show small surrounding area where icon wrapper is rendered
  rg -n --context=4 'navLinkIcon' packages/react-core/src/components/Nav/NavItem.tsx
else
  echo "NavItem.tsx not found at expected path" >&2
fi

# Also inspect NavExpandable.tsx around the referenced line for consistency
rg -n --context=3 'navLinkIcon' packages/react-core/src/components/Nav/NavExpandable.tsx

Repository: patternfly/patternfly-react

Length of output: 1596

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="packages/react-core/src/components/Nav/NavItem.tsx"

echo "== aria-hidden matches in NavItem.tsx =="
rg -n 'aria-hidden' "$FILE" || true

echo "== navLinkIcon context in NavItem.tsx =="
rg -n --context=4 'navLinkIcon' "$FILE"

Repository: patternfly/patternfly-react

Length of output: 504

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="packages/react-core/src/components/Nav/NavExpandable.tsx"

echo "== aria-hidden matches in NavExpandable.tsx =="
rg -n 'aria-hidden' "$FILE" || true

echo "== navLinkIcon context in NavExpandable.tsx =="
rg -n --context=4 'navLinkIcon' "$FILE"

Repository: patternfly/patternfly-react

Length of output: 679

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '180,240p' packages/react-core/src/components/Nav/NavItem.tsx | cat -n

echo "----"

sed -n '110,170p' packages/react-core/src/components/Nav/NavExpandable.tsx | cat -n

Repository: patternfly/patternfly-react

Length of output: 5416

Add aria-hidden="true" to decorative nav icon wrappers in NavExpandable/NavItem

Both packages/react-core/src/components/Nav/NavExpandable.tsx and packages/react-core/src/components/Nav/NavItem.tsx render the icon span without aria-hidden, even though the icon sits next to visible text.

♿ Proposed fix to add aria-hidden 
-                        {icon && <span className={css(styles.navLinkIcon)}>{icon}</span>}
+                        {icon && <span className={css(styles.navLinkIcon)} aria-hidden="true">{icon}</span>}

Apply the same aria-hidden="true" to the equivalent icon wrapper in NavItem.tsx.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{icon && <span className={css(styles.navLinkIcon)}>{icon}</span>}
{icon && <span className={css(styles.navLinkIcon)} aria-hidden="true">{icon}</span>}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react-core/src/components/Nav/NavExpandable.tsx` at line 138, The
icon span rendered in NavExpandable (the JSX expression {icon && <span
className={css(styles.navLinkIcon)}>{icon}</span>}) should be marked decorative
by adding aria-hidden="true" to that span; apply the identical change to the
equivalent icon wrapper in NavItem.tsx so both NavExpandable and NavItem treat
the displayed icons as non-accessible/decorative elements.

{typeof title !== 'string' ? (
<span className={css(`${styles.nav}__link-text`)}>{title}</span>
) : (
Expand Down
16 changes: 16 additions & 0 deletions packages/react-core/src/components/Nav/__tests__/NavExpandable.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';
import styles from '@patternfly/react-styles/css/components/Nav/nav';
import { NavExpandable } from '../NavExpandable';

test('Renders with the inert attribute by default', () => {
Expand All @@ -13,3 +14,18 @@ test('Does not render with the inert attribute when isExpanded is true', () => {

expect(screen.getByLabelText('NavExpandable')).not.toHaveAttribute('inert', '');
});

test('Renders icon with navLinkIcon class', () => {
render(
<NavExpandable id="grp-1" title="NavExpandable" icon={<div data-testid="nav-expandable-icon">Icon content</div>} />
);

expect(screen.getByTestId('nav-expandable-icon').parentElement).toHaveClass(styles.navLinkIcon);
});

test('Does not render icon wrapper when icon is not provided', () => {
render(<NavExpandable id="grp-1" title="NavExpandable" />);

const button = screen.getByRole('button', { name: 'NavExpandable' });
expect(button.querySelector(`.${styles.navLinkIcon}`)).toBeNull();
});
6 changes: 6 additions & 0 deletions packages/react-core/src/components/Nav/examples/Nav.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ A flyout should be a `Menu` component. Press `space` or `right arrow` to open a

```

### Expandable with icons

```ts file="./NavExpandableIcons.tsx"

```


## Types

Expand Down
88 changes: 88 additions & 0 deletions packages/react-core/src/components/Nav/examples/NavExpandableIcons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { useState } from 'react';
import { Nav, NavExpandable, NavItem, NavList } from '@patternfly/react-core';
import CubeIcon from '@patternfly/react-icons/dist/esm/icons/cube-icon';
import FolderIcon from '@patternfly/react-icons/dist/esm/icons/folder-icon';

export const NavExpandableIcons: React.FunctionComponent = () => {
const [activeGroup, setActiveGroup] = useState('nav-expandable-icon-group-1');
const [activeItem, setActiveItem] = useState('nav-expandable-icon-group-1_item-1');

const onSelect = (
_event: React.FormEvent<HTMLInputElement>,
result: { itemId: number | string; groupId: number | string }
) => {
setActiveGroup(result.groupId as string);
setActiveItem(result.itemId as string);
};

const onToggle = (
_event: React.MouseEvent<HTMLButtonElement>,
result: { groupId: number | string; isExpanded: boolean }
) => {
// eslint-disable-next-line no-console
console.log(`Group ${result.groupId} expanded? ${result.isExpanded}`);
};

return (
<Nav onSelect={onSelect} onToggle={onToggle} aria-label="Expandable with icons global">
<NavList>
<NavExpandable
title="Expandable Group 1"
icon={<CubeIcon />}
groupId="nav-expandable-icon-group-1"
isActive={activeGroup === 'nav-expandable-icon-group-1'}
isExpanded
>
<NavItem
preventDefault
id="expandable-icon-1"
to="#expandable-icon-1"
groupId="nav-expandable-icon-group-1"
itemId="nav-expandable-icon-group-1_item-1"
isActive={activeItem === 'nav-expandable-icon-group-1_item-1'}
>
Subnav 1 Link 1
</NavItem>
<NavItem
preventDefault
id="expandable-icon-2"
to="#expandable-icon-2"
groupId="nav-expandable-icon-group-1"
itemId="nav-expandable-icon-group-1_item-2"
isActive={activeItem === 'nav-expandable-icon-group-1_item-2'}
>
Subnav 1 Link 2
</NavItem>
</NavExpandable>
<NavExpandable
title="Expandable Group 2"
icon={<FolderIcon />}
groupId="nav-expandable-icon-group-2"
isActive={activeGroup === 'nav-expandable-icon-group-2'}
isExpanded
>
<NavItem
preventDefault
id="expandable-icon-3"
to="#expandable-icon-3"
groupId="nav-expandable-icon-group-2"
itemId="nav-expandable-icon-group-2_item-1"
isActive={activeItem === 'nav-expandable-icon-group-2_item-1'}
>
Subnav 2 Link 1
</NavItem>
<NavItem
preventDefault
id="expandable-icon-4"
to="#expandable-icon-4"
groupId="nav-expandable-icon-group-2"
itemId="nav-expandable-icon-group-2_item-2"
isActive={activeItem === 'nav-expandable-icon-group-2_item-2'}
>
Subnav 2 Link 2
</NavItem>
</NavExpandable>
</NavList>
</Nav>
);
};
Loading