Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion packages/react-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"tslib": "^2.8.1"
},
"devDependencies": {
"@patternfly/patternfly": "6.5.0-prerelease.33",
"@patternfly/patternfly": "6.5.0-prerelease.34",
"case-anything": "^3.1.2",
"css": "^3.0.0",
"fs-extra": "^11.3.0"
Expand Down
6 changes: 6 additions & 0 deletions packages/react-core/src/components/TreeView/TreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ export interface TreeViewDataItem {
name: React.ReactNode;
/** Title of a tree view item. Only used in compact presentations. */
title?: React.ReactNode;
/** Flag indicating if the tree view item is disabled. */
isDisabled?: boolean;
/** Flag indicating if the tree view item toggle is disabled. */
isToggleDisabled?: boolean;
Comment on lines +38 to +41
Copy link
Contributor

@thatblindgeye thatblindgeye Dec 12, 2025

Choose a reason for hiding this comment

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

Would we want isDisabled to disable everything (selection and expansion)? Might just be me, just that's what I expected to happen when passing isDisabled, but that only disabled selection for the example where the selection is a separate action; for the default behavior isDisabled works as expected since there's only one button.

Or would we want to have isToggleDisabled and isSelectionDisabled props instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe a prop description update would work ("For tree view items with separated toggle and selection behaviors, this will disable selection.")? I think I had gone with a plain isDisabled because for tree view items where selection isn't separate, everything is disabled.

}

/** The main tree view component. */
Expand Down Expand Up @@ -158,6 +162,8 @@ export const TreeView: React.FunctionComponent<TreeViewProps> = ({
id={item.id}
isExpanded={allExpanded}
isSelectable={hasSelectableNodes}
isDisabled={item.isDisabled}
isToggleDisabled={item.isToggleDisabled}
defaultExpanded={item.defaultExpanded !== undefined ? item.defaultExpanded : defaultAllExpanded}
onSelect={onSelect}
onCheck={onCheck}
Expand Down
36 changes: 27 additions & 9 deletions packages/react-core/src/components/TreeView/TreeViewListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ export interface TreeViewListItemProps {
* children.
*/
isSelectable?: boolean;
/** Flag indicating if the tree view item is disabled. */
isDisabled?: boolean;
/** Flag indicating if the tree view item toggle is disabled. */
isToggleDisabled?: boolean;
/** Data structure of tree view item. */
itemData?: TreeViewDataItem;
/** Internal content of a tree view item. */
Expand Down Expand Up @@ -81,6 +85,8 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
title,
id,
isExpanded,
isDisabled = false,
isToggleDisabled = false,
defaultExpanded = false,
children = null,
onSelect,
Expand Down Expand Up @@ -125,25 +131,26 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
}

const ToggleComponent = hasCheckbox || isSelectable ? 'button' : 'span';
const hasDisabledToggleClass = isToggleDisabled || (Component === 'button' && isDisabled);

const renderToggle = (randomId: string) => (
<ToggleComponent
className={css(styles.treeViewNodeToggle)}
className={css(styles.treeViewNodeToggle, hasDisabledToggleClass && styles.modifiers.disabled)}
onClick={(evt: React.MouseEvent) => {
if (isSelectable || hasCheckbox) {
if (!isToggleDisabled && (isSelectable || hasCheckbox)) {
if (internalIsExpanded) {
onCollapse && onCollapse(evt, itemData, parentItem);
} else {
onExpand && onExpand(evt, itemData, parentItem);
}
setIsExpanded(!internalIsExpanded);
}
if (isSelectable) {
if (!isToggleDisabled && isSelectable) {
evt.stopPropagation();
}
}}
{...((hasCheckbox || isSelectable) && { 'aria-labelledby': `label-${randomId}` })}
{...(ToggleComponent === 'button' && { type: 'button' })}
{...(ToggleComponent === 'button' && { disabled: isToggleDisabled, type: 'button' })}
tabIndex={-1}
>
<span className={css(styles.treeViewNodeToggleIcon)}>
Expand All @@ -162,6 +169,7 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
ref={(elem) => {
elem && (elem.indeterminate = checkProps.checked === null);
}}
disabled={isDisabled || checkProps.disabled}
{...checkProps}
checked={isCheckboxChecked}
id={randomId}
Expand All @@ -180,7 +188,7 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
<>
{isCompact && title && <span className={css(styles.treeViewNodeTitle)}>{title}</span>}
{isSelectable ? (
<button tabIndex={-1} className={css(styles.treeViewNodeText)} type="button">
<button tabIndex={-1} className={css(styles.treeViewNodeText)} type="button" disabled={isDisabled}>
{name}
</button>
) : (
Expand Down Expand Up @@ -220,6 +228,9 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
})
);

const isFullyDisabled =
(Component === 'button' && isDisabled) || (Component !== 'button' && isDisabled && isToggleDisabled);
Comment on lines +231 to +232
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

ARIA treeitem aria-disabled best practices accessibility

💡 Result:

role="treeitem" + aria-disabled: best-practice guidance

  • Use aria-disabled="true" to expose “disabled but still perceivable” tree items to assistive tech (screen readers will announce the disabled state). aria-disabled is semantic only: you must also prevent activation in code (e.g., ignore Enter/Space and click handlers for that node). [1][2]
  • Decide whether disabled nodes remain reachable in the tree’s arrow-key navigation, and implement consistently:
    • ARIA notes that disabled elements might not receive focus and some apps may not support navigation to descendants of disabled elements—so either (a) keep them navigable but non-operable, or (b) skip them in roving focus/aria-activedescendant movement. [2]
  • Provide a clear visual disabled affordance (e.g., “grayed out”), including in forced-colors/high-contrast modes; aria-disabled does not apply default browser disabled styling. [1][2]
  • Apply aria-disabled at the right level: it applies to the element it’s on and all focusable descendants, so don’t set it on a container unless you intend to disable everything inside. [1][2]
  • Keep treeview semantics correct while disabling:
    • Keep the tree pattern structure and keyboard model (tree owns treeitems; arrow keys move focus; Enter activates). Then layer “disabled = not operable” on top. [3]

Key takeaway: In a treeview, aria-disabled="true" is the right way to announce a disabled treeitem, but accessibility depends on your focus/navigation decision and on actually blocking activation via scripting. [1][2][3]

Sources

[1] MDN — aria-disabled attribute (updated Nov 6, 2025) (developer.mozilla.org)
[2] W3C — WAI-ARIA 1.3 (aria-disabled state; treeitem role characteristics) (w3.org)
[3] W3C APG — Tree View Pattern (roles/states + keyboard interaction model) (w3.org)

Citations:


🏁 Script executed:

cd /tmp && find /tmp -name "TreeViewListItem.tsx" 2>/dev/null | head -1 | xargs ls -la 2>/dev/null || echo "File not in /tmp"

Repository: patternfly/patternfly-react

Length of output: 552


🏁 Script executed:

# Search for the TreeViewListItem.tsx file and examine context around lines 231-232
fd -t f "TreeViewListItem.tsx" --exec cat -n {} \; | head -n 250 | tail -n 100

Repository: patternfly/patternfly-react

Length of output: 4180


Apply aria-disabled whenever the treeitem is not operable, regardless of toggle state.

Per ARIA standards, aria-disabled must announce to screen readers when a treeitem is not operable. The current logic creates an asymmetry: in button mode, aria-disabled is applied when isDisabled is true, but in checkbox/selectable modes, it's only applied when both isDisabled and isToggleDisabled are true.

This means when Component !== 'button' and isDisabled=true but isToggleDisabled=false, the selectable button/checkbox element is disabled={true} (not operable), yet aria-disabled is not set on the treeitem. Screen readers will not announce the disabled state, contradicting the accessibility requirement to expose disabled-but-perceivable items.

Recommend: set aria-disabled whenever isDisabled is true, regardless of toggle state. If the intent is to distinguish "item disabled + toggle functional" from "fully disabled," use separate states or attributes rather than omitting aria-disabled.

🤖 Prompt for AI Agents
In `@packages/react-core/src/components/TreeView/TreeViewListItem.tsx` around
lines 231 - 232, The aria-disabled logic in TreeViewListItem is asymmetric:
update the computation and usage around isFullyDisabled / aria-disabled so that
aria-disabled is set whenever isDisabled is true regardless of Component or
isToggleDisabled; specifically, change the condition that derives
isFullyDisabled (or use a separate flag) so that aria-disabled uses isDisabled
directly while preserving any existing UI disabled behavior for the toggle
(isToggleDisabled) when Component !== 'button', and apply the updated flag where
aria-disabled is rendered on the treeitem element.


return (
<li
id={id}
Expand All @@ -229,16 +240,21 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
tabIndex={-1}
{...(hasCheckbox && { 'aria-checked': isCheckboxChecked })}
{...(!hasCheckbox && { 'aria-selected': isSelected })}
{...(isFullyDisabled && { 'aria-disabled': true })}
>
<div className={css(styles.treeViewContent)}>
<GenerateId prefix={isSelectable ? 'selectable-id' : 'checkbox-id'}>
{(randomId) => (
<Component
className={css(styles.treeViewNode, isSelected && styles.modifiers.current)}
className={css(
styles.treeViewNode,
isSelected && styles.modifiers.current,
isDisabled && styles.modifiers.disabled
)}
onClick={(evt: React.MouseEvent) => {
if (!hasCheckbox) {
onSelect && onSelect(evt, itemData, parentItem);
if (!isSelectable && children && evt.isDefaultPrevented() !== true) {
!isDisabled && onSelect && onSelect(evt, itemData, parentItem);
if (!isDisabled && !isSelectable && children && evt.isDefaultPrevented() !== true) {
if (internalIsExpanded) {
onCollapse && onCollapse(evt, itemData, parentItem);
} else {
Expand All @@ -250,7 +266,7 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
}}
{...(hasCheckbox && { htmlFor: randomId })}
{...((hasCheckbox || (isSelectable && children)) && { id: `label-${randomId}` })}
{...(Component === 'button' && { type: 'button' })}
{...(Component === 'button' && { type: 'button', disabled: isDisabled })}
>
<span className={css(styles.treeViewNodeContainer)}>
{children && renderToggle(randomId)}
Expand Down Expand Up @@ -297,6 +313,8 @@ export const TreeViewListItem = memo(TreeViewListItemBase, (prevProps, nextProps
prevProps.id !== nextProps.id ||
prevProps.isExpanded !== nextProps.isExpanded ||
prevProps.defaultExpanded !== nextProps.defaultExpanded ||
prevProps.isDisabled !== nextProps.isDisabled ||
prevProps.isToggleDisabled !== nextProps.isToggleDisabled ||
prevProps.onSelect !== nextProps.onSelect ||
prevProps.onCheck !== nextProps.onCheck ||
prevProps.onExpand !== nextProps.onExpand ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,249 @@ test(`Does not render ${styles.treeViewNode} element with ${styles.modifiers.cur
expect(treeViewNode).not.toHaveClass(styles.modifiers.current);
});

// Assisted by Cursor AI
describe('isDisabled prop', () => {
const user = userEvent.setup();
const onSelectMock = jest.fn();
const onExpandMock = jest.fn();
const onCollapseMock = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
});

test(`Renders button with disabled attribute and ${styles.modifiers.disabled} class when isDisabled is true`, () => {
render(<TreeViewListItem isDisabled {...requiredProps} />);

const button = screen.getByRole('button', { name: requiredProps.name });
expect(button).toBeDisabled();
expect(button).toHaveClass(styles.modifiers.disabled);
});

test('Does not render button with disabled attribute when isDisabled is false', () => {
render(<TreeViewListItem isDisabled={false} {...requiredProps} />);

expect(screen.getByRole('button', { name: requiredProps.name })).not.toBeDisabled();
});

test('Does not call onSelect when isDisabled is true', async () => {
render(<TreeViewListItem isDisabled onSelect={onSelectMock} {...requiredProps} />);

await user.click(screen.getByRole('button', { name: requiredProps.name }));

expect(onSelectMock).not.toHaveBeenCalled();
});

test('Does not call onExpand when isDisabled is true and item is collapsed', async () => {
render(
<TreeViewListItem isDisabled onExpand={onExpandMock} {...requiredProps}>
Content
</TreeViewListItem>
);

await user.click(screen.getByRole('button', { name: requiredProps.name }));

expect(onExpandMock).not.toHaveBeenCalled();
});

test('Does not call onCollapse when isDisabled is true and item is expanded', async () => {
render(
<TreeViewListItem isDisabled isExpanded onCollapse={onCollapseMock} {...requiredProps}>
Content
</TreeViewListItem>
);

await user.click(screen.getByRole('button', { name: requiredProps.name }));

expect(onCollapseMock).not.toHaveBeenCalled();
});

test(`Renders toggle with ${styles.modifiers.disabled} class when isDisabled is true for default TreeViewListItem`, () => {
render(
<TreeViewListItem isDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
expect(toggle).toHaveClass(styles.modifiers.disabled);
});

test('Renders treeitem with aria-disabled when isDisabled is true for default TreeViewListItem', () => {
render(<TreeViewListItem isDisabled {...requiredProps} />);

expect(screen.getByRole('treeitem')).toHaveAttribute('aria-disabled', 'true');
});

test('Renders treeitem with aria-disabled when isDisabled and isToggleDisabled are true and isSelectable is true', () => {
render(
<TreeViewListItem isSelectable isDisabled isToggleDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

expect(screen.getByRole('treeitem')).toHaveAttribute('aria-disabled', 'true');
});

test('Renders treeitem with aria-disabled when isDisabled and isToggleDisabled are true and hasCheckbox is true', () => {
render(
<TreeViewListItem hasCheckbox isDisabled isToggleDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

expect(screen.getByRole('treeitem')).toHaveAttribute('aria-disabled', 'true');
});

test('Does not render treeitem with aria-disabled when isDisabled is true, isToggleDisabled is false, and isSelectable is true', () => {
render(
<TreeViewListItem isSelectable isDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

expect(screen.getByRole('treeitem')).not.toHaveAttribute('aria-disabled');
});

test('Does not render treeitem with aria-disabled when isDisabled is false', () => {
render(<TreeViewListItem isDisabled={false} {...requiredProps} />);

expect(screen.getByRole('treeitem')).not.toHaveAttribute('aria-disabled');
});
});

// Assisted by Cursor AI
describe('isToggleDisabled prop', () => {
const user = userEvent.setup();
const onExpandMock = jest.fn();
const onCollapseMock = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
});

test(`Renders toggle button with disabled attribute and ${styles.modifiers.disabled} class when isToggleDisabled is true and hasCheckbox is passed`, () => {
render(
<TreeViewListItem hasCheckbox isToggleDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling?.previousElementSibling;
expect(toggle).toBeDisabled();
expect(toggle).toHaveClass(styles.modifiers.disabled);
});

test(`Renders toggle button with disabled attribute and ${styles.modifiers.disabled} class when isToggleDisabled is true and isSelectable is passed`, () => {
render(
<TreeViewListItem isSelectable isToggleDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
expect(toggle).toBeDisabled();
expect(toggle).toHaveClass(styles.modifiers.disabled);
});

test('Does not render toggle span with disabled attribute when isToggleDisabled is true (toggle is span by default)', () => {
render(
<TreeViewListItem isToggleDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
expect(toggle?.tagName).toBe('SPAN');
expect(toggle).not.toHaveAttribute('disabled');
});

test('Does not call onExpand when isToggleDisabled is true and hasCheckbox is passed', async () => {
render(
<TreeViewListItem hasCheckbox isToggleDisabled onExpand={onExpandMock} {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling?.previousElementSibling;
await user.click(toggle as Element);

expect(onExpandMock).not.toHaveBeenCalled();
});

test('Does not call onCollapse when isToggleDisabled is true and hasCheckbox is passed', async () => {
render(
<TreeViewListItem hasCheckbox isToggleDisabled isExpanded onCollapse={onCollapseMock} {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling?.previousElementSibling;
await user.click(toggle as Element);

expect(onCollapseMock).not.toHaveBeenCalled();
});

test('Does not call onExpand when isToggleDisabled is true and isSelectable is passed', async () => {
render(
<TreeViewListItem isSelectable isToggleDisabled onExpand={onExpandMock} {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
await user.click(toggle as Element);

expect(onExpandMock).not.toHaveBeenCalled();
});

test('Does not call onCollapse when isToggleDisabled is true and isSelectable is passed', async () => {
render(
<TreeViewListItem isSelectable isToggleDisabled isExpanded onCollapse={onCollapseMock} {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
await user.click(toggle as Element);

expect(onCollapseMock).not.toHaveBeenCalled();
});

test(`Renders toggle span with ${styles.modifiers.disabled} class when isDisabled is true for default TreeViewListItem`, () => {
render(
<TreeViewListItem isDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
expect(toggle).toHaveClass(styles.modifiers.disabled);
});

test(`Does not render toggle with ${styles.modifiers.disabled} class when isDisabled is true and hasCheckbox is true`, () => {
render(
<TreeViewListItem hasCheckbox isDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling?.previousElementSibling;
expect(toggle).not.toHaveClass(styles.modifiers.disabled);
});

test(`Does not render toggle with ${styles.modifiers.disabled} class when isDisabled is true and isSelectable is true`, () => {
render(
<TreeViewListItem isSelectable isDisabled {...requiredProps}>
Content
</TreeViewListItem>
);

const toggle = screen.getByText(requiredProps.name).previousElementSibling;
expect(toggle).not.toHaveClass(styles.modifiers.disabled);
});
});

describe('Callback props', () => {
const user = userEvent.setup();
const compareItemsMock = jest.fn();
Expand Down
Loading
Loading