pr feedback & css updates

Author: kmcfaul

packages/react-core/src/components/TreeView/TreeViewListItem.tsx

@@ -131,10 +131,11 @@ 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, ToggleComponent === 'button' && isToggleDisabled && 'pf-m-disabled')}
+      className={css(styles.treeViewNodeToggle, hasDisabledToggleClass && 'pf-m-disabled')}
       onClick={(evt: React.MouseEvent) => {
         if (!isToggleDisabled && (isSelectable || hasCheckbox)) {
           if (internalIsExpanded) {
@@ -186,12 +187,7 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
       <>
         {isCompact && title && <span className={css(styles.treeViewNodeTitle)}>{title}</span>}
         {isSelectable ? (
-          <button
-            tabIndex={-1}
-            className={css(styles.treeViewNodeText, isDisabled && 'pf-m-disabled')}
-            type="button"
-            disabled={isDisabled}
-          >
+          <button tabIndex={-1} className={css(styles.treeViewNodeText)} type="button" disabled={isDisabled}>
             {name}
           </button>
         ) : (
@@ -231,6 +227,9 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
       })
   );
 
+  const isFullyDisabled =
+    (Component === 'button' && isDisabled) || (Component !== 'button' && isDisabled && isToggleDisabled);
+
   return (
     <li
       id={id}
@@ -240,6 +239,7 @@ 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'}>
@@ -248,7 +248,7 @@ const TreeViewListItemBase: React.FunctionComponent<TreeViewListItemProps> = ({
               className={css(
                 styles.treeViewNode,
                 isSelected && styles.modifiers.current,
-                Component === 'button' && isDisabled && 'pf-m-disabled'
+                isDisabled && 'pf-m-disabled'
               )}
               onClick={(evt: React.MouseEvent) => {
                 if (!hasCheckbox) {

packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx

@@ -440,15 +440,6 @@ describe('isDisabled prop', () => {
     expect(screen.getByRole('button', { name: requiredProps.name })).not.toBeDisabled();
   });
 
-  test('Renders selectable button with disabled attribute when isDisabled is true', () => {
-    render(<TreeViewListItem isSelectable isDisabled {...requiredProps} />);
-
-    const treeViewNode = screen.getByRole('treeitem').querySelector(`.${styles.treeViewNode}`);
-    const selectableButton = treeViewNode?.querySelector('button');
-    expect(selectableButton).toBeDisabled();
-    expect(selectableButton).toHaveClass('pf-m-disabled');
-  });
-
   test('Does not call onSelect when isDisabled is true', async () => {
     render(<TreeViewListItem isDisabled onSelect={onSelectMock} {...requiredProps} />);
 
@@ -480,6 +471,59 @@ describe('isDisabled prop', () => {
 
     expect(onCollapseMock).not.toHaveBeenCalled();
   });
+
+  test('Renders toggle with pf-m-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('pf-m-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
@@ -579,6 +623,39 @@ describe('isToggleDisabled prop', () => {
 
     expect(onCollapseMock).not.toHaveBeenCalled();
   });
+
+  test('Renders toggle span with pf-m-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('pf-m-disabled');
+  });
+
+  test('Does not render toggle with pf-m-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('pf-m-disabled');
+  });
+
+  test('Does not render toggle with pf-m-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('pf-m-disabled');
+  });
 });
 
 describe('Callback props', () => {