Skip to content
Open
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
35 changes: 26 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;
Comment on lines +49 to +52
Copy link

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:

find . -type f -name "TreeViewListItem.tsx" -path "*/react-core/*" | head -5

Repository: patternfly/patternfly-react

Length of output: 137


🏁 Script executed:

fd "TreeViewListItem" --type f

Repository: patternfly/patternfly-react

Length of output: 314


🏁 Script executed:

cat -n packages/react-core/src/components/TreeView/TreeViewListItem.tsx | head -200

Repository: patternfly/patternfly-react

Length of output: 9070


🏁 Script executed:

cat -n packages/react-core/src/components/TreeView/TreeViewListItem.tsx | sed -n '200,250p'

Repository: patternfly/patternfly-react

Length of output: 2174


🏁 Script executed:

cat -n packages/react-core/src/components/TreeView/TreeViewListItem.tsx | tail -100

Repository: patternfly/patternfly-react

Length of output: 4652


🏁 Script executed:

cat -n packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx | grep -i "disabled\|isDisabled" -A 2 -B 2 | head -50

Repository: patternfly/patternfly-react

Length of output: 2127


🏁 Script executed:

grep -n "isDisabled" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 2803


🏁 Script executed:

sed -n '502,511p' packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 430


🏁 Script executed:

sed -n '638,648p' packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 502


🏁 Script executed:

grep -n "hasCheckbox.*isDisabled\|isDisabled.*hasCheckbox" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx -A 10 | head -60

Repository: patternfly/patternfly-react

Length of output: 1330


🏁 Script executed:

grep -n "hasCheckbox" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx | grep -i "check\|click\|oncheck"

Repository: patternfly/patternfly-react

Length of output: 2982


🏁 Script executed:

sed -n '1,100p' packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx | grep -n "describe\|test"

Repository: patternfly/patternfly-react

Length of output: 1306


🏁 Script executed:

grep -n "describe.*[Cc]heck\|test.*[Cc]heck" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 1877


🏁 Script executed:

sed -n '731,770p' packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 1590


🏁 Script executed:

grep -n "hasCheckbox.*isDisabled\|isDisabled.*hasCheckbox" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx -B 2 -A 15

Repository: patternfly/patternfly-react

Length of output: 1647


🏁 Script executed:

grep -n "isDisabled.*hasCheckbox.*onCheck\|onCheck.*isDisabled.*hasCheckbox" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx -B 5 -A 10

Repository: patternfly/patternfly-react

Length of output: 53


🏁 Script executed:

sed -n '180,200p' packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 885


🏁 Script executed:

sed -n '160,180p' packages/react-core/src/components/TreeView/TreeViewListItem.tsx

Repository: patternfly/patternfly-react

Length of output: 734


🏁 Script executed:

grep -n "onCheck.*isDisabled\|isDisabled.*onCheck" packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

Repository: patternfly/patternfly-react

Length of output: 53


Checkbox should be disabled when isDisabled is true

When hasCheckbox is enabled, the checkbox input element is not disabled when isDisabled=true. The onChange callback fires normally regardless of the disabled state. Apply disabled={isDisabled || checkProps.disabled} to the checkbox input to prevent interaction when the item is disabled, consistent with how onSelect, onExpand, and onCollapse are already guarded.

🤖 Prompt for AI Agents
In `@packages/react-core/src/components/TreeView/TreeViewListItem.tsx` around
lines 49 - 52, In TreeViewListItem, the checkbox input remains interactive when
the item is disabled; update the checkbox element to set its disabled attribute
to the combined value (isDisabled || checkProps.disabled) so it is
non-interactive when the row is disabled. Locate the checkbox rendering in the
TreeViewListItem component (where hasCheckbox is handled and checkProps is
spread onto the input) and replace/augment the input's disabled prop to use
disabled={isDisabled || checkProps.disabled} and ensure the onChange handler is
short-circuited/guarded accordingly.

/** 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 Down Expand Up @@ -180,7 +187,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 +227,9 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
})
);

const isFullyDisabled =
(Component === 'button' && isDisabled) || (Component !== 'button' && isDisabled && isToggleDisabled);

return (
<li
id={id}
Expand All @@ -229,16 +239,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 +265,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 +312,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