chore(List): updated unit tests

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

@@ -49,12 +49,14 @@ export const List: React.FunctionComponent<ListProps> = ({
   type = OrderType.number,
   ref = null,
   component = ListComponent.ul,
+  'aria-label': ariaLabel,
   ...props
 }: ListProps) =>
   component === ListComponent.ol ? (
     <ol
       ref={ref as React.Ref<HTMLOListElement>}
       type={type}
+      aria-label={ariaLabel}
       {...(isPlain && { role: 'list' })}
       {...props}
       className={css(
@@ -71,6 +73,7 @@ export const List: React.FunctionComponent<ListProps> = ({
   ) : (
     <ul
       ref={ref as React.Ref<HTMLUListElement>}
+      aria-label={ariaLabel}
       {...(isPlain && { role: 'list' })}
       {...props}
       className={css(

packages/react-core/src/components/List/ListItem.tsx

@@ -2,18 +2,21 @@ import styles from '@patternfly/react-styles/css/components/List/list';
 import { css } from '@patternfly/react-styles';
 
 export interface ListItemProps extends React.HTMLProps<HTMLLIElement> {
-  /** Icon for the list item */
-  icon?: React.ReactNode | null;
+  /** Additional classes added to the list item */
+  className?: string;
   /** Anything that can be rendered inside of list item */
   children: React.ReactNode;
+  /** Icon for the list item */
+  icon?: React.ReactNode | null;
 }
 
 export const ListItem: React.FunctionComponent<ListItemProps> = ({
-  icon = null,
+  className,
   children = null,
+  icon = null,
   ...props
 }: ListItemProps) => (
-  <li className={css(icon && styles.listItem)} {...props}>
+  <li className={css(icon && styles.listItem, className)} {...props}>
     {icon && <span className={css(styles.listItemIcon)}>{icon}</span>}
     <span className={icon && css(`${styles.list}__item-text`)}>{children}</span>
   </li>

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

@@ -1,117 +1,113 @@
 import { render, screen } from '@testing-library/react';
-
-import BookOpen from '@patternfly/react-icons/dist/esm/icons/book-open-icon';
-import Key from '@patternfly/react-icons/dist/esm/icons/key-icon';
-import Desktop from '@patternfly/react-icons/dist/esm/icons/desktop-icon';
 import { List, ListVariant, ListComponent, OrderType } from '../List';
-import { ListItem } from '../ListItem';
-
-const ListItems = () => (
-  <List>
-    <ListItem>First</ListItem>
-    <ListItem>Second</ListItem>
-    <ListItem>Third</ListItem>
-  </List>
-);
-
-describe('List', () => {
-  test('simple list', () => {
-    const { asFragment } = render(
-      <List>
-        <ListItems />
-      </List>
-    );
-    expect(asFragment()).toMatchSnapshot();
-  });
+import styles from '@patternfly/react-styles/css/components/List/list';
 
-  test('inline list', () => {
-    const { asFragment } = render(
-      <List variant={ListVariant.inline}>
-        <ListItems />
-      </List>
-    );
-    expect(asFragment()).toMatchSnapshot();
-  });
+describe('Shared tests between ol and ul lists', () => {
+  Object.values(ListComponent).forEach((component) => {
+    test(`Renders without children for ${component} list`, () => {
+      render(<List component={component}></List>);
 
-  test('ordered list', () => {
-    const { asFragment } = render(
-      <List component={ListComponent.ol}>
-        <ListItem>Apple</ListItem>
-        <ListItem>Banana</ListItem>
-        <ListItem>Orange</ListItem>
-      </List>
-    );
-    expect(asFragment()).toMatchSnapshot();
-  });
+      expect(screen.getByRole('list')).toBeVisible();
+    });
 
-  test('ordered list starts with 2nd item', () => {
-    render(
-      <List component={ListComponent.ol} start={2}>
-        <ListItem>Banana</ListItem>
-        <ListItem>Orange</ListItem>
-      </List>
-    );
-    expect(screen.getByRole('list')).toHaveAttribute('start', '2');
-  });
+    test(`Renders children for ${component} list`, () => {
+      render(<List component={component}>Children content</List>);
 
-  test('ordered list items will be numbered with uppercase letters', () => {
-    render(
-      <List component={ListComponent.ol} type={OrderType.uppercaseLetter}>
-        <ListItem>Banana</ListItem>
-        <ListItem>Orange</ListItem>
-      </List>
-    );
-    expect(screen.getByRole('list')).toHaveAttribute('type', 'A');
-  });
+      expect(screen.getByRole('list')).toHaveTextContent('Children content');
+    });
 
-  test('inlined ordered list', () => {
-    render(
-      <List variant={ListVariant.inline} component={ListComponent.ol}>
-        <ListItem>Apple</ListItem>
-        <ListItem>Banana</ListItem>
-        <ListItem>Orange</ListItem>
-      </List>
-    );
-    expect(screen.getByRole('list')).toHaveClass('pf-m-inline');
-  });
+    test(`Renders with ${component} tag`, () => {
+      render(<List component={component}></List>);
+
+      expect(screen.getByRole('list').tagName).toBe(component.toUpperCase());
+    });
+
+    test(`Renders with only class ${styles.list} by default for ${component} list`, () => {
+      render(<List component={component}></List>);
+
+      expect(screen.getByRole('list')).toHaveClass(styles.list, { exact: true });
+    });
+
+    test(`Renders with custom class when className is passed for ${component} list`, () => {
+      render(<List component={component} className="custom-class"></List>);
+
+      expect(screen.getByRole('list')).toHaveClass('custom-class');
+    });
+
+    test(`Renders with variant class ${styles.modifiers.inline} when variant prop is inline for ${component} list`, () => {
+      render(<List component={component} variant={ListVariant.inline}></List>);
+
+      expect(screen.getByRole('list')).toHaveClass(styles.modifiers.inline);
+    });
+
+    test(`Renders with class ${styles.modifiers.bordered} when isBordered is true for ${component} list`, () => {
+      render(<List component={component} isBordered></List>);
+
+      expect(screen.getByRole('list')).toHaveClass(styles.modifiers.bordered);
+    });
+
+    test(`Renders with class ${styles.modifiers.plain} when isPlain is true for ${component} list`, () => {
+      render(<List component={component} isPlain></List>);
 
-  test('bordered list', () => {
-    render(
-      <List isBordered>
-        <ListItems />
-      </List>
-    );
-    expect(screen.getAllByRole('list')[0]).toHaveClass('pf-m-bordered');
+      expect(screen.getByRole('list')).toHaveClass(styles.modifiers.plain);
+    });
+
+    test(`Renders with class ${styles.modifiers.iconLg} when iconSize is "large" for ${component} list`, () => {
+      render(<List component={component} iconSize="large"></List>);
+
+      expect(screen.getByRole('list')).toHaveClass(styles.modifiers.iconLg);
+    });
+
+    test(`Renders with aria-label for ${component} list`, () => {
+      render(<List component={component} aria-label="Testing stuff"></List>);
+
+      expect(screen.getByRole('list')).toHaveAccessibleName('Testing stuff');
+    });
+
+    test(`Spreads additional props when passed for ${component} list`, () => {
+      render(<List component={component} id="Test ID"></List>);
+
+      expect(screen.getByRole('list')).toHaveAttribute('id', 'Test ID');
+    });
+
+    test(`Does not render with role attribute when isPlain is false for ${component} list`, () => {
+      render(<List component={component}></List>);
+
+      expect(screen.getByRole('list')).not.toHaveAttribute('role');
+    });
+
+    test(`Renders with role attribute of "list" when isPlain is true for ${component} list`, () => {
+      render(<List component={component} isPlain></List>);
+
+      expect(screen.getByRole('list')).toHaveAttribute('role', 'list');
+    });
+
+    test(`Matches snapshot for ${component} list`, () => {
+      const { asFragment } = render(<List component={component}></List>);
+
+      expect(asFragment()).toMatchSnapshot();
+    });
   });
+});
+
+describe('Ol component list', () => {
+  test(`Renders with type of "1" by default`, () => {
+    render(<List component={ListComponent.ol}></List>);
 
-  test('plain list', () => {
-    render(
-      <List isPlain>
-        <ListItems />
-      </List>
-    );
-    expect(screen.getAllByRole('list')[0]).toHaveClass('pf-m-plain');
+    expect(screen.getByRole('list')).toHaveAttribute('type', '1');
   });
 
-  test('icon list', () => {
-    const { asFragment } = render(
-      <List isPlain>
-        <ListItem icon={<BookOpen />}>Apple</ListItem>
-        <ListItem icon={<Key />}>Banana</ListItem>
-        <ListItem icon={<Desktop />}>Orange</ListItem>
-      </List>
-    );
-    expect(asFragment()).toMatchSnapshot();
+  test(`Renders with type attribute when type is passed`, () => {
+    render(<List component={ListComponent.ol} type={OrderType.uppercaseLetter}></List>);
+
+    expect(screen.getByRole('list')).toHaveAttribute('type', 'A');
   });
+});
+
+describe('Ul component list', () => {
+  test(`Does not render with type attribute when type is passed`, () => {
+    render(<List type={OrderType.lowercaseRomanNumber}></List>);
 
-  test('icon large list', () => {
-    const { asFragment } = render(
-      <List iconSize="large">
-        <ListItem icon={<BookOpen />}>Apple</ListItem>
-        <ListItem icon={<Key />}>Banana</ListItem>
-        <ListItem icon={<Desktop />}>Orange</ListItem>
-      </List>
-    );
-    expect(asFragment()).toMatchSnapshot();
+    expect(screen.getByRole('list')).not.toHaveAttribute('type');
   });
 });

packages/react-core/src/components/List/__tests__/ListItem.test.tsx

@@ -0,0 +1,51 @@
+import { render, screen } from '@testing-library/react';
+import { ListItem } from '../ListItem';
+import styles from '@patternfly/react-styles/css/components/List/list';
+
+test('Renders with children', () => {
+  render(<ListItem>List item content</ListItem>);
+
+  expect(screen.getByRole('listitem')).toHaveTextContent('List item content');
+});
+
+test(`Does not render with a class by default`, () => {
+  render(<ListItem>List item content</ListItem>);
+
+  expect(screen.getByRole('listitem')).not.toHaveClass();
+});
+
+test(`Renders with custom class when className is passed`, () => {
+  render(<ListItem className="test-class">List item content</ListItem>);
+
+  expect(screen.getByRole('listitem')).toHaveClass('test-class', { exact: true });
+});
+
+test(`Renders with icon content when icon prop is passed`, () => {
+  render(<ListItem icon={<div>Icon content</div>}>List item content</ListItem>);
+
+  expect(screen.getByRole('listitem')).toContainHTML('<div>Icon content</div>');
+});
+
+test(`Renders with class ${styles.listItem} when icon prop is passed`, () => {
+  render(<ListItem icon={<div>Icon content</div>}>List item content</ListItem>);
+
+  expect(screen.getByRole('listitem')).toHaveClass(styles.listItem, { exact: true });
+});
+
+test(`Spreads additional props when passed`, () => {
+  render(<ListItem id="test-ID">List item content</ListItem>);
+
+  expect(screen.getByRole('listitem')).toHaveAttribute('id', 'test-ID');
+});
+
+test('Matches snapshot without icon', () => {
+  const { asFragment } = render(<ListItem>List item content</ListItem>);
+
+  expect(asFragment()).toMatchSnapshot();
+});
+
+test('Matches snapshot with icon', () => {
+  const { asFragment } = render(<ListItem icon={<div>Icon content</div>}>List item content</ListItem>);
+
+  expect(asFragment()).toMatchSnapshot();
+});