From 83ceacf588bdef38e42e539aaf674de7d3a45da4 Mon Sep 17 00:00:00 2001
From: "Keith J. Grant" <barkeep@keithjgrant.com>
Date: Tue, 27 Apr 2021 11:38:46 -0700
Subject: [PATCH] convert inventory groups list to tables

---
 .../InventoryGroups/InventoryGroupItem.jsx    | 88 ++++++++-----------
 .../InventoryGroupItem.test.jsx               | 32 ++++---
 .../InventoryGroups/InventoryGroupsList.jsx   | 25 +++---
 .../InventoryGroupsList.test.jsx              | 62 ++++++++-----
 .../InventoryHosts/InventoryHostItem.jsx      | 30 +++----
 5 files changed, 130 insertions(+), 107 deletions(-)

diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.jsx
index 8ebe0d604d..8a0b95ae2f 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.jsx
@@ -1,67 +1,57 @@
 import React from 'react';
 import { bool, func, number, oneOfType, string } from 'prop-types';
-
 import { t } from '@lingui/macro';
 
-import {
-  Button,
-  DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemCells,
-  DataListItemRow,
-  Tooltip,
-} from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
+import { Tr, Td } from '@patternfly/react-table';
 
 import { Link } from 'react-router-dom';
 import { PencilAltIcon } from '@patternfly/react-icons';
-import DataListCell from '../../../components/DataListCell';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
 import { Group } from '../../../types';
 
-function InventoryGroupItem({ group, inventoryId, isSelected, onSelect }) {
+function InventoryGroupItem({
+  group,
+  inventoryId,
+  isSelected,
+  onSelect,
+  rowIndex,
+}) {
   const labelId = `check-action-${group.id}`;
   const detailUrl = `/inventories/inventory/${inventoryId}/groups/${group.id}/details`;
   const editUrl = `/inventories/inventory/${inventoryId}/groups/${group.id}/edit`;
 
   return (
-    <DataListItem key={group.id} aria-labelledby={labelId} id={`${group.id}`}>
-      <DataListItemRow>
-        <DataListCheck
-          aria-labelledby={labelId}
-          id={`select-group-${group.id}`}
-          checked={isSelected}
-          onChange={onSelect}
-        />
-        <DataListItemCells
-          dataListCells={[
-            <DataListCell key="name">
-              <Link to={`${detailUrl}`} id={labelId}>
-                <b>{group.name}</b>
-              </Link>
-            </DataListCell>,
-          ]}
-        />
-        <DataListAction
-          aria-label={t`actions`}
-          aria-labelledby={labelId}
-          id={labelId}
+    <Tr id={`group-row=${group.id}`}>
+      <Td
+        select={{
+          rowIndex,
+          isSelected,
+          onSelect,
+        }}
+      />
+      <Td id={labelId} dataLabel={t`Name`}>
+        <Link to={`${detailUrl}`} id={labelId}>
+          <b>{group.name}</b>
+        </Link>
+      </Td>
+      <ActionsTd dataLabel={t`Actions`} gridColumns="auto 40px">
+        <ActionItem
+          visible={group.summary_fields.user_capabilities.edit}
+          tooltip={t`Edit group`}
         >
-          {group.summary_fields.user_capabilities.edit && (
-            <Tooltip content={t`Edit Group`} position="top">
-              <Button
-                ouiaId={`${group.id}-edit-button`}
-                aria-label={t`Edit Group`}
-                variant="plain"
-                component={Link}
-                to={editUrl}
-              >
-                <PencilAltIcon />
-              </Button>
-            </Tooltip>
-          )}
-        </DataListAction>
-      </DataListItemRow>
-    </DataListItem>
+          <Button
+            ouiaId={`${group.id}-edit-button`}
+            aria-label={t`Edit Group`}
+            variant="plain"
+            component={Link}
+            to={editUrl}
+          >
+            <PencilAltIcon />
+          </Button>
+        </ActionItem>
+      </ActionsTd>
+    </Tr>
   );
 }
 
diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.test.jsx
index db0b2a0bd4..3d60389513 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.test.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupItem.test.jsx
@@ -18,12 +18,16 @@ describe('<InventoryGroupItem />', () => {
 
   beforeEach(() => {
     wrapper = mountWithContexts(
-      <InventoryGroupItem
-        group={mockGroup}
-        inventoryId={1}
-        isSelected={false}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <InventoryGroupItem
+            group={mockGroup}
+            inventoryId={1}
+            isSelected={false}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
   });
 
@@ -40,12 +44,16 @@ describe('<InventoryGroupItem />', () => {
     copyMockGroup.summary_fields.user_capabilities.edit = false;
 
     wrapper = mountWithContexts(
-      <InventoryGroupItem
-        group={copyMockGroup}
-        inventoryId={1}
-        isSelected={false}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <InventoryGroupItem
+            group={copyMockGroup}
+            inventoryId={1}
+            isSelected={false}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy();
   });
diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.jsx
index 70dc7ef5c6..c190c5b54c 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.jsx
@@ -8,9 +8,11 @@ import useSelected from '../../../util/useSelected';
 import useRequest from '../../../util/useRequest';
 import { InventoriesAPI } from '../../../api';
 import DataListToolbar from '../../../components/DataListToolbar';
-import PaginatedDataList, {
-  ToolbarAddButton,
-} from '../../../components/PaginatedDataList';
+import PaginatedTable, {
+  HeaderRow,
+  HeaderCell,
+} from '../../../components/PaginatedTable';
+import { ToolbarAddButton } from '../../../components/PaginatedDataList';
 
 import InventoryGroupItem from './InventoryGroupItem';
 import InventoryGroupsDeleteModal from '../shared/InventoryGroupsDeleteModal';
@@ -104,7 +106,7 @@ function InventoryGroupsList() {
 
   return (
     <>
-      <PaginatedDataList
+      <PaginatedTable
         contentError={contentError}
         hasContentLoading={isLoading || isAdHocLaunchLoading}
         items={groups}
@@ -135,21 +137,22 @@ function InventoryGroupsList() {
             key: 'modified_by__username__icontains',
           },
         ]}
-        toolbarSortColumns={[
-          {
-            name: t`Name`,
-            key: 'name',
-          },
-        ]}
         toolbarSearchableKeys={searchableKeys}
         toolbarRelatedSearchableKeys={relatedSearchableKeys}
-        renderItem={item => (
+        headerRow={
+          <HeaderRow qsConfig={QS_CONFIG}>
+            <HeaderCell sortKey="name">{t`Name`}</HeaderCell>
+            <HeaderCell>{t`Actions`}</HeaderCell>
+          </HeaderRow>
+        }
+        renderRow={(item, index) => (
           <InventoryGroupItem
             key={item.id}
             group={item}
             inventoryId={inventoryId}
             isSelected={selected.some(row => row.id === item.id)}
             onSelect={() => handleSelect(item)}
+            rowIndex={index}
           />
         )}
         renderToolbar={props => (
diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.test.jsx
index 1c6268fbcf..1dbd23b030 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.test.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupsList.test.jsx
@@ -93,15 +93,12 @@ describe('<InventoryGroupsList />', () => {
     });
     await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
   });
+
   afterEach(() => {
     jest.clearAllMocks();
     wrapper.unmount();
   });
 
-  test('initially renders successfully', () => {
-    expect(wrapper.find('InventoryGroupsList').length).toBe(1);
-  });
-
   test('should fetch groups from api and render them in the list', async () => {
     expect(InventoriesAPI.readGroups).toHaveBeenCalled();
     expect(wrapper.find('InventoryGroupItem').length).toBe(3);
@@ -109,52 +106,71 @@ describe('<InventoryGroupsList />', () => {
 
   test('should check and uncheck the row item', async () => {
     expect(
-      wrapper.find('DataListCheck[id="select-group-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(false);
 
     await act(async () => {
-      wrapper.find('DataListCheck[id="select-group-1"]').invoke('onChange')(
-        true
-      );
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .invoke('onChange')(true);
     });
     wrapper.update();
     expect(
-      wrapper.find('DataListCheck[id="select-group-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(true);
 
     await act(async () => {
-      wrapper.find('DataListCheck[id="select-group-1"]').invoke('onChange')(
-        false
-      );
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .invoke('onChange')(false);
     });
     wrapper.update();
     expect(
-      wrapper.find('DataListCheck[id="select-group-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(false);
   });
 
   test('should check all row items when select all is checked', async () => {
-    wrapper.find('DataListCheck').forEach(el => {
-      expect(el.props().checked).toBe(false);
+    expect.assertions(9);
+    wrapper.find('.pf-c-table__check').forEach(el => {
+      expect(el.find('input').props().checked).toBe(false);
     });
     await act(async () => {
       wrapper.find('Checkbox#select-all').invoke('onChange')(true);
     });
     wrapper.update();
-    wrapper.find('DataListCheck').forEach(el => {
-      expect(el.props().checked).toBe(true);
+    wrapper.find('.pf-c-table__check').forEach(el => {
+      expect(el.find('input').props().checked).toBe(true);
     });
     await act(async () => {
       wrapper.find('Checkbox#select-all').invoke('onChange')(false);
     });
     wrapper.update();
-    wrapper.find('DataListCheck').forEach(el => {
-      expect(el.props().checked).toBe(false);
+    wrapper.find('.pf-c-table__check').forEach(el => {
+      expect(el.find('input').props().checked).toBe(false);
     });
   });
 });
+
 describe('<InventoryGroupsList/> error handling', () => {
   let wrapper;
+
   beforeEach(() => {
     InventoriesAPI.readGroups.mockResolvedValue({
       data: {
@@ -182,10 +198,12 @@ describe('<InventoryGroupsList/> error handling', () => {
       })
     );
   });
+
   afterEach(() => {
     jest.clearAllMocks();
     wrapper.unmount();
   });
+
   test('should show content error when api throws error on initial render', async () => {
     InventoriesAPI.readGroupsOptions.mockImplementationOnce(() =>
       Promise.reject(new Error())
@@ -213,7 +231,11 @@ describe('<InventoryGroupsList/> error handling', () => {
     waitForElement(wrapper, 'ContentEmpty', el => el.length === 0);
 
     await act(async () => {
-      wrapper.find('DataListCheck[id="select-group-1"]').invoke('onChange')();
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .invoke('onChange')();
     });
     wrapper.update();
     await act(async () => {
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx
index 50795f7955..c9bc45a179 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx
@@ -1,12 +1,11 @@
 import React from 'react';
 import { string, bool, func } from 'prop-types';
-
 import { t } from '@lingui/macro';
-import { Button, Tooltip } from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
 import { Tr, Td } from '@patternfly/react-table';
 import { Link } from 'react-router-dom';
 import { PencilAltIcon } from '@patternfly/react-icons';
-import { ActionsTd } from '../../../components/PaginatedTable';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
 
 import HostToggle from '../../../components/HostToggle';
 import { Host } from '../../../types';
@@ -37,18 +36,19 @@ function InventoryHostItem({
       </Td>
       <ActionsTd dataLabel={t`Actions`} gridColumns="auto 40px">
         <HostToggle host={host} />
-        {host.summary_fields.user_capabilities?.edit ? (
-          <Tooltip content={t`Edit Host`} position="top">
-            <Button
-              ouiaId={`${host.id}-edit-button`}
-              variant="plain"
-              component={Link}
-              to={`${editUrl}`}
-            >
-              <PencilAltIcon />
-            </Button>
-          </Tooltip>
-        ) : null}
+        <ActionItem
+          visible={host.summary_fields.user_capabilities?.edit}
+          tooltip={t`Edit host`}
+        >
+          <Button
+            ouiaId={`${host.id}-edit-button`}
+            variant="plain"
+            component={Link}
+            to={`${editUrl}`}
+          >
+            <PencilAltIcon />
+          </Button>
+        </ActionItem>
       </ActionsTd>
     </Tr>
   );