From 8081d668725b4e8b77ba056d83ceb2ee72ecf3cd Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Thu, 21 Jan 2021 15:33:08 -0800
Subject: [PATCH] convert credentials list to table

---
 .../components/PaginatedTable/HeaderRow.jsx   |   3 +
 .../CredentialList/CredentialList.jsx         |  18 ++-
 .../CredentialList/CredentialList.test.jsx    |  34 ++++-
 .../CredentialList/CredentialListItem.jsx     | 123 +++++++-----------
 .../CredentialListItem.test.jsx               |  80 +++++++-----
 5 files changed, 142 insertions(+), 116 deletions(-)

diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx
index 5ea13ee6b3..cec9a984ef 100644
--- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx
+++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx
@@ -1,3 +1,4 @@
+import 'styled-components/macro';
 import React from 'react';
 import { useLocation, useHistory } from 'react-router-dom';
 import { Thead, Tr, Th as PFTh } from '@patternfly/react-table';
@@ -72,6 +73,7 @@ export function HeaderCell({
   columnIndex,
   idPrefix,
   className,
+  alignRight,
   children,
 }) {
   const sort = sortKey
@@ -86,6 +88,7 @@ export function HeaderCell({
       id={sortKey ? `${idPrefix}-${sortKey}` : null}
       className={className}
       sort={sort}
+      css={alignRight ? 'text-align: right' : null}
     >
       {children}
     </Th>
diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx
index e4a10ed86c..ced36412f5 100644
--- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx
+++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx
@@ -7,10 +7,14 @@ import { CredentialsAPI } from '../../../api';
 import AlertModal from '../../../components/AlertModal';
 import ErrorDetail from '../../../components/ErrorDetail';
 import DataListToolbar from '../../../components/DataListToolbar';
-import PaginatedDataList, {
+import {
   ToolbarAddButton,
   ToolbarDeleteButton,
 } from '../../../components/PaginatedDataList';
+import PaginatedTable, {
+  HeaderRow,
+  HeaderCell,
+} from '../../../components/PaginatedTable';
 import useRequest, { useDeleteItems } from '../../../util/useRequest';
 import { getQSConfig, parseQueryString } from '../../../util/qs';
 import CredentialListItem from './CredentialListItem';
@@ -114,7 +118,7 @@ function CredentialList({ i18n }) {
   return (
     <PageSection>
       <Card>
-        <PaginatedDataList
+        <PaginatedTable
           contentError={contentError}
           hasContentLoading={isLoading || isDeleteLoading}
           items={credentials}
@@ -142,7 +146,14 @@ function CredentialList({ i18n }) {
               key: 'modified_by__username__icontains',
             },
           ]}
-          renderItem={item => (
+          headerRow={
+            <HeaderRow qsConfig={QS_CONFIG}>
+              <HeaderCell sortKey="name">{i18n._(t`Name`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Type`)}</HeaderCell>
+              <HeaderCell alignRight>{i18n._(t`Actions`)}</HeaderCell>
+            </HeaderRow>
+          }
+          renderRow={(item, index) => (
             <CredentialListItem
               key={item.id}
               credential={item}
@@ -150,6 +161,7 @@ function CredentialList({ i18n }) {
               detailUrl={`/credentials/${item.id}/details`}
               isSelected={selected.some(row => row.id === item.id)}
               onSelect={() => handleSelect(item)}
+              rowIndex={index}
             />
           )}
           renderToolbar={props => (
diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.test.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.test.jsx
index 88c52ff228..35bcce1d64 100644
--- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.test.jsx
+++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.test.jsx
@@ -57,25 +57,41 @@ describe('<CredentialList />', () => {
 
   test('should check and uncheck the row item', async () => {
     expect(
-      wrapper.find('DataListCheck[id="select-credential-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(false);
     await act(async () => {
       wrapper
-        .find('DataListCheck[id="select-credential-1"]')
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
         .invoke('onChange')(true);
     });
     wrapper.update();
     expect(
-      wrapper.find('DataListCheck[id="select-credential-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(true);
     await act(async () => {
       wrapper
-        .find('DataListCheck[id="select-credential-1"]')
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
         .invoke('onChange')(false);
     });
     wrapper.update();
     expect(
-      wrapper.find('DataListCheck[id="select-credential-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(false);
   });
 
@@ -105,7 +121,9 @@ describe('<CredentialList />', () => {
 
     await act(async () => {
       wrapper
-        .find('DataListCheck[id="select-credential-3"]')
+        .find('.pf-c-table__check')
+        .at(2)
+        .find('input')
         .invoke('onChange')();
     });
     wrapper.update();
@@ -122,7 +140,9 @@ describe('<CredentialList />', () => {
     );
     await act(async () => {
       wrapper
-        .find('DataListCheck[id="select-credential-2"]')
+        .find('.pf-c-table__check')
+        .at(1)
+        .find('input')
         .invoke('onChange')();
     });
     wrapper.update();
diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx
index a4e436c073..9886cf3572 100644
--- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx
+++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx
@@ -3,31 +3,16 @@ import { string, bool, func } from 'prop-types';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Link } from 'react-router-dom';
-import {
-  Button,
-  DataListAction as _DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemRow,
-  DataListItemCells,
-  Tooltip,
-} from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
+import { Tr, Td } from '@patternfly/react-table';
 import { PencilAltIcon } from '@patternfly/react-icons';
-import styled from 'styled-components';
-import DataListCell from '../../../components/DataListCell';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
 import { timeOfDay } from '../../../util/dates';
 
 import { Credential } from '../../../types';
 import { CredentialsAPI } from '../../../api';
 import CopyButton from '../../../components/CopyButton';
 
-const DataListAction = styled(_DataListAction)`
-  align-items: center;
-  display: grid;
-  grid-gap: 16px;
-  grid-template-columns: repeat(2, 40px);
-`;
-
 function CredentialListItem({
   credential,
   detailUrl,
@@ -35,6 +20,7 @@ function CredentialListItem({
   onSelect,
   i18n,
   fetchCredentials,
+  rowIndex,
 }) {
   const [isDisabled, setIsDisabled] = useState(false);
 
@@ -57,64 +43,49 @@ function CredentialListItem({
   }, []);
 
   return (
-    <DataListItem
-      key={credential.id}
-      aria-labelledby={labelId}
-      id={`${credential.id}`}
-    >
-      <DataListItemRow>
-        <DataListCheck
-          isDisabled={isDisabled}
-          id={`select-credential-${credential.id}`}
-          checked={isSelected}
-          onChange={onSelect}
-          aria-labelledby={labelId}
-        />
-        <DataListItemCells
-          dataListCells={[
-            <DataListCell key="name">
-              <Link to={`${detailUrl}`}>
-                <b>{credential.name}</b>
-              </Link>
-            </DataListCell>,
-            <DataListCell key="type">
-              {credential.summary_fields.credential_type.name}
-            </DataListCell>,
-          ]}
-        />
-        <DataListAction
-          aria-label={i18n._(t`actions`)}
-          aria-labelledby={labelId}
-          id={labelId}
-        >
-          {canEdit && (
-            <Tooltip content={i18n._(t`Edit Credential`)} position="top">
-              <Button
-                isDisabled={isDisabled}
-                aria-label={i18n._(t`Edit Credential`)}
-                variant="plain"
-                component={Link}
-                to={`/credentials/${credential.id}/edit`}
-              >
-                <PencilAltIcon />
-              </Button>
-            </Tooltip>
-          )}
-          {credential.summary_fields.user_capabilities.copy && (
-            <CopyButton
-              isDisabled={isDisabled}
-              onCopyStart={handleCopyStart}
-              onCopyFinish={handleCopyFinish}
-              copyItem={copyCredential}
-              helperText={{
-                tooltip: i18n._(t`Copy Credential`),
-                errorMessage: i18n._(t`Failed to copy credential.`),
-              }}
-            />
-          )}
-        </DataListAction>
-      </DataListItemRow>
-    </DataListItem>
+    <Tr id={`${credential.id}`}>
+      <Td
+        select={{
+          rowIndex,
+          isSelected,
+          onSelect,
+        }}
+        dataLabel={i18n._(t`Selected`)}
+      />
+      <Td id={labelId} dataLabel={i18n._(t`Name`)}>
+        <Link to={`${detailUrl}`}>
+          <b>{credential.name}</b>
+        </Link>
+      </Td>
+      <Td dataLabel={i18n._(t`Type`)}>
+        {credential.summary_fields.credential_type.name}
+      </Td>
+      <ActionsTd dataLabel={i18n._(t`Actions`)}>
+        <ActionItem visible={canEdit} tooltip={i18n._(t`Edit Credential`)}>
+          <Button
+            isDisabled={isDisabled}
+            aria-label={i18n._(t`Edit Credential`)}
+            variant="plain"
+            component={Link}
+            to={`/credentials/${credential.id}/edit`}
+          >
+            <PencilAltIcon />
+          </Button>
+        </ActionItem>
+        <ActionItem visible={credential.summary_fields.user_capabilities.copy}>
+          <CopyButton
+            isDisabled={isDisabled}
+            onCopyStart={handleCopyStart}
+            onCopyFinish={handleCopyFinish}
+            copyItem={copyCredential}
+            helperText={{
+              tooltip: i18n._(t`Copy Credential`),
+              errorMessage: i18n._(t`Failed to copy credential.`),
+            }}
+          />
+        </ActionItem>
+      </ActionsTd>
+    </Tr>
   );
 }
 
diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx
index fcfc085d2d..74bb7e6915 100644
--- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx
+++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx
@@ -16,24 +16,32 @@ describe('<CredentialListItem />', () => {
 
   test('edit button shown to users with edit capabilities', () => {
     wrapper = mountWithContexts(
-      <CredentialListItem
-        credential={mockCredentials.results[0]}
-        detailUrl="/foo/bar"
-        isSelected={false}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <CredentialListItem
+            credential={mockCredentials.results[0]}
+            detailUrl="/foo/bar"
+            isSelected={false}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeTruthy();
   });
 
   test('edit button hidden from users without edit capabilities', () => {
     wrapper = mountWithContexts(
-      <CredentialListItem
-        credential={mockCredentials.results[1]}
-        detailUrl="/foo/bar"
-        isSelected={false}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <CredentialListItem
+            credential={mockCredentials.results[1]}
+            detailUrl="/foo/bar"
+            isSelected={false}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy();
   });
@@ -41,12 +49,16 @@ describe('<CredentialListItem />', () => {
     CredentialsAPI.copy.mockResolvedValue();
 
     wrapper = mountWithContexts(
-      <CredentialListItem
-        isSelected={false}
-        detailUrl="/foo/bar"
-        credential={mockCredentials.results[0]}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <CredentialListItem
+            isSelected={false}
+            detailUrl="/foo/bar"
+            credential={mockCredentials.results[0]}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
 
     await act(async () =>
@@ -60,12 +72,16 @@ describe('<CredentialListItem />', () => {
     CredentialsAPI.copy.mockRejectedValue(new Error());
 
     wrapper = mountWithContexts(
-      <CredentialListItem
-        isSelected={false}
-        detailUrl="/foo/bar"
-        onSelect={() => {}}
-        credential={mockCredentials.results[0]}
-      />
+      <table>
+        <tbody>
+          <CredentialListItem
+            isSelected={false}
+            detailUrl="/foo/bar"
+            onSelect={() => {}}
+            credential={mockCredentials.results[0]}
+          />
+        </tbody>
+      </table>
     );
     await act(async () =>
       wrapper.find('Button[aria-label="Copy"]').prop('onClick')()
@@ -77,12 +93,16 @@ describe('<CredentialListItem />', () => {
 
   test('should not render copy button', async () => {
     wrapper = mountWithContexts(
-      <CredentialListItem
-        isSelected={false}
-        detailUrl="/foo/bar"
-        onSelect={() => {}}
-        credential={mockCredentials.results[1]}
-      />
+      <table>
+        <tbody>
+          <CredentialListItem
+            isSelected={false}
+            detailUrl="/foo/bar"
+            onSelect={() => {}}
+            credential={mockCredentials.results[1]}
+          />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('CopyButton').length).toBe(0);
   });