feat(DualListSelector): enabled opt-in animations (#11837)

Author: thatblindgeye

packages/react-core/src/components/DualListSelector/DualListSelector.tsx

@@ -17,27 +17,41 @@ export interface DualListSelectorProps {
   isTree?: boolean;
   /** Content to be rendered in the dual list selector. */
   children?: React.ReactNode;
+  /** Flag indicating whether a tree dual list selector has animations. This will always render
+   * nested dual list selector items rather than dynamically rendering them. This prop will be removed in
+   * the next breaking change release in favor of defaulting to always-rendered items.
+   */
+  hasAnimations?: boolean;
 }
 
 class DualListSelector extends Component<DualListSelectorProps> {
   static displayName = 'DualListSelector';
   static defaultProps: PickOptional<DualListSelectorProps> = {
     children: '',
-    isTree: false
+    isTree: false,
+    hasAnimations: false
   };
 
   constructor(props: DualListSelectorProps) {
     super(props);
   }
 
   render() {
-    const { className, children, id, isTree, ...props } = this.props;
+    const { className, children, id, isTree, hasAnimations, ...props } = this.props;
 
     return (
-      <DualListSelectorContext.Provider value={{ isTree }}>
+      <DualListSelectorContext.Provider value={{ isTree, hasAnimations }}>
         <GenerateId>
           {(randomId) => (
-            <div className={css(styles.dualListSelector, className)} id={id || randomId} {...props}>
+            <div
+              className={css(
+                styles.dualListSelector,
+                hasAnimations && isTree && styles.modifiers.animateExpand,
+                className
+              )}
+              id={id || randomId}
+              {...props}
+            >
               {children}
             </div>
           )}

packages/react-core/src/components/DualListSelector/DualListSelectorContext.ts

@@ -1,7 +1,8 @@
 import { createContext } from 'react';
 export const DualListSelectorContext = createContext<{
   isTree?: boolean;
-}>({ isTree: false });
+  hasAnimations?: boolean;
+}>({ isTree: false, hasAnimations: false });
 
 export const DualListSelectorListContext = createContext<{
   setFocusedOption?: (id: string) => void;

packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx

@@ -56,14 +56,17 @@ export const DualListSelectorListWrapperBase: React.FunctionComponent<DualListSe
     }
     event.stopImmediatePropagation();
     const validOptions = isTree
-      ? (Array.from(
-          menuRef.current.querySelectorAll(
-            `.${styles.dualListSelectorItemToggle}, .${styles.dualListSelectorItemCheck} > input`
-          )
-        ) as Element[])
+      ? (
+          Array.from(
+            menuRef.current.querySelectorAll(
+              `.${styles.dualListSelectorItemToggle}, .${styles.dualListSelectorItemCheck} > input`
+            )
+          ) as Element[]
+        ).filter((item) => !item.closest(`.${styles.dualListSelectorList}[inert]`))
       : (Array.from(menuRef.current.getElementsByTagName('LI')) as Element[]).filter(
           (el) => !el.classList.contains('pf-m-disabled')
         );
+
     const activeElement = document.activeElement;
     handleArrows(
       event,

packages/react-core/src/components/DualListSelector/DualListSelectorTree.tsx

@@ -1,5 +1,7 @@
+import { useContext } from 'react';
 import { css } from '@patternfly/react-styles';
 import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector';
+import { DualListSelectorContext } from './DualListSelectorContext';
 import { DualListSelectorTreeItem } from './DualListSelectorTreeItem';
 
 export interface DualListSelectorTreeItemData {
@@ -68,11 +70,13 @@ export const DualListSelectorTree: React.FunctionComponent<DualListSelectorTreeP
   isDisabled = false,
   ...props
 }: DualListSelectorTreeProps) => {
+  const { hasAnimations } = useContext(DualListSelectorContext);
   const dataToRender = typeof data === 'function' ? data() : data;
   const tree = dataToRender.map((item) => (
     <DualListSelectorTreeItem
       key={item.id}
       text={item.text}
+      hasAnimations={hasAnimations}
       id={item.id}
       defaultExpanded={item.defaultExpanded !== undefined ? item.defaultExpanded : defaultAllExpanded}
       onOptionCheck={onOptionCheck}

packages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsx

@@ -1,4 +1,4 @@
-import { memo, useContext, useEffect, useRef, useState } from 'react';
+import { memo, useContext, useEffect, useRef, useState, cloneElement, Children, isValidElement } from 'react';
 import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector';
 import { css } from '@patternfly/react-styles';
 import { DualListSelectorTreeItemData } from './DualListSelectorTree';
@@ -38,6 +38,11 @@ export interface DualListSelectorTreeItemProps extends React.HTMLProps<HTMLLIEle
   isDisabled?: boolean;
   /** Flag indicating the DualListSelector tree should utilize memoization to help render large data sets. */
   useMemo?: boolean;
+  /** Flag indicating whether a tree dual list selector has animations. This will always render
+   * nested dual list selector items rather than dynamically rendering them. This prop will be removed in
+   * the next breaking change release in favor of defaulting to always-rendered items.
+   */
+  hasAnimations?: boolean;
 }
 
 const DualListSelectorTreeItemBase: React.FunctionComponent<DualListSelectorTreeItemProps> = ({
@@ -53,6 +58,7 @@ const DualListSelectorTreeItemBase: React.FunctionComponent<DualListSelectorTree
   badgeProps,
   itemData,
   isDisabled = false,
+  hasAnimations,
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   useMemo,
   ...props
@@ -65,6 +71,15 @@ const DualListSelectorTreeItemBase: React.FunctionComponent<DualListSelectorTree
     setIsExpanded(defaultExpanded);
   }, [defaultExpanded]);
 
+  const clonedChildren = Children.map(
+    children,
+    (child) =>
+      isValidElement(child) &&
+      cloneElement(child as React.ReactElement<any>, {
+        inert: isExpanded ? undefined : ''
+      })
+  );
+
   return (
     <li
       className={css(
@@ -156,7 +171,7 @@ const DualListSelectorTreeItemBase: React.FunctionComponent<DualListSelectorTree
           </span>
         </div>
       </div>
-      {isExpanded && children}
+      {(isExpanded || hasAnimations) && clonedChildren}
     </li>
   );
 };

packages/react-core/src/components/DualListSelector/__tests__/DualListSelector.test.tsx

@@ -1,6 +1,37 @@
-import { render } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector';
+import { DualListSelector } from '../DualListSelector';
 import { DualListSelectorPane } from '../DualListSelectorPane';
 import { SearchInput } from '../../SearchInput';
+
+// The following tests can be removed as part of https://github.com/patternfly/patternfly-react/issues/11838
+describe('Opt-in animations', () => {
+  test(`Does not render with class ${styles.modifiers.animateExpand} by default`, () => {
+    render(<DualListSelector data-testid="test-id" />);
+
+    expect(screen.getByTestId('test-id')).not.toHaveClass(styles.modifiers.animateExpand);
+  });
+
+  test(`Does not render with class ${styles.modifiers.animateExpand} when hasAnimations is true and isTree is false`, () => {
+    render(<DualListSelector hasAnimations data-testid="test-id" />);
+
+    expect(screen.getByTestId('test-id')).not.toHaveClass(styles.modifiers.animateExpand);
+  });
+
+  test(`Does not render with class ${styles.modifiers.animateExpand} by default when isTree is true`, () => {
+    render(<DualListSelector isTree data-testid="test-id" />);
+
+    expect(screen.getByTestId('test-id')).not.toHaveClass(styles.modifiers.animateExpand);
+  });
+
+  test(`Renders with class ${styles.modifiers.animateExpand} when both isTree and hasAnimations are true`, () => {
+    render(<DualListSelector isTree hasAnimations data-testid="test-id" />);
+
+    expect(screen.getByTestId('test-id')).toHaveClass(styles.modifiers.animateExpand);
+  });
+});
+
+// Following tests should be moved to a separate DualListSelectorPane test file
 describe('DualListSelector', () => {
   test('basic', () => {
     const { asFragment } = render(<DualListSelectorPane id="basicTest" />);

packages/react-core/src/components/DualListSelector/__tests__/DualListSelectorTreeItem.test.tsx

@@ -0,0 +1,55 @@
+import { render, screen } from '@testing-library/react';
+import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector';
+import { DualListSelectorTreeItem } from '../DualListSelectorTreeItem';
+
+// The following tests checking for children to not be/to be rendered will need to be refactored
+// as part of https://github.com/patternfly/patternfly-react/issues/11838
+test('Does not render children by default', () => {
+  render(
+    <DualListSelectorTreeItem id="item-id" text="Test text">
+      <div>Children content</div>
+    </DualListSelectorTreeItem>
+  );
+
+  expect(screen.queryByText('Children content')).not.toBeInTheDocument();
+});
+
+test('Renders children when defaultExpanded is true', () => {
+  render(
+    <DualListSelectorTreeItem defaultExpanded id="item-id" text="Test text">
+      <div>Children content</div>
+    </DualListSelectorTreeItem>
+  );
+
+  expect(screen.getByText('Children content')).toBeVisible();
+});
+
+test('Renders children when hasAnimations is true', () => {
+  render(
+    <DualListSelectorTreeItem hasAnimations id="item-id" text="Test text">
+      <div>Children content</div>
+    </DualListSelectorTreeItem>
+  );
+
+  expect(screen.getByText('Children content')).toBeVisible();
+});
+
+test('Renders children with inert attribute by default when hasAnimations is true', () => {
+  render(
+    <DualListSelectorTreeItem hasAnimations id="item-id" text="Test text">
+      <div>Children content</div>
+    </DualListSelectorTreeItem>
+  );
+
+  expect(screen.getByText('Children content')).toHaveAttribute('inert', '');
+});
+
+test('Does not render children with inert attribute when hasAnimations and defaultExpanded are true', () => {
+  render(
+    <DualListSelectorTreeItem hasAnimations defaultExpanded id="item-id" text="Test text">
+      <div>Children content</div>
+    </DualListSelectorTreeItem>
+  );
+
+  expect(screen.getByText('Children content')).not.toHaveAttribute('inert');
+});

packages/react-core/src/components/DualListSelector/examples/DualListSelectorTree.tsx

@@ -283,7 +283,7 @@ export const DualListSelectorComposableTree: React.FunctionComponent<ExampleProp
   };
 
   return (
-    <DualListSelector isTree>
+    <DualListSelector hasAnimations isTree>
       {buildPane(false)}
       <DualListSelectorControlsWrapper>
         <DualListSelectorControl

packages/react-core/src/helpers/KeyboardHandler.tsx

@@ -116,12 +116,12 @@ export const handleArrows = (
                   : nextSiblingMainElement.nextElementSibling;
 
               if (nextSibling) {
-                if (validSiblingTags.includes(nextSibling.tagName)) {
+                if (validSiblingTags.includes(nextSibling?.tagName)) {
                   moveTarget = nextSibling;
                   break;
                 }
                 // For cases where the validSiblingTag is inside a div wrapper
-                if (validSiblingTags.includes(nextSibling.children[0].tagName)) {
+                if (validSiblingTags.includes(nextSibling.children[0]?.tagName)) {
                   moveTarget = nextSibling.children[0];
                   break;
                 }