Merge pull request #9148 from keithjgrant/6189-credentials-table

Convert Credentials & Projects lists to PaginatedTable

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

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>

awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx

@@ -89,7 +89,7 @@ function PaginatedTable({
     );
   } else {
     Content = (
-      <div css="overflow: scroll">
+      <div css="overflow: auto">
         {hasContentLoading && <LoadingSpinner />}
         <TableComposable aria-label={dataListLabel}>
           {headerRow}

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 => (

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();

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 {
+import { Button } from '@patternfly/react-core';
-  Button,
+import { Tr, Td } from '@patternfly/react-table';
-  DataListAction as _DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemRow,
-  DataListItemCells,
-  Tooltip,
-} from '@patternfly/react-core';
 import { PencilAltIcon } from '@patternfly/react-icons';
-import styled from 'styled-components';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
-import DataListCell from '../../../components/DataListCell';
 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,38 +43,25 @@ function CredentialListItem({
   }, []);
 
   return (
-    <DataListItem
+    <Tr id={`${credential.id}`}>
-      key={credential.id}
+      <Td
-      aria-labelledby={labelId}
+        select={{
-      id={`${credential.id}`}
+          rowIndex,
-    >
+          isSelected,
-      <DataListItemRow>
+          onSelect,
-        <DataListCheck
+        }}
-          isDisabled={isDisabled}
+        dataLabel={i18n._(t`Selected`)}
-          id={`select-credential-${credential.id}`}
-          checked={isSelected}
-          onChange={onSelect}
-          aria-labelledby={labelId}
       />
-        <DataListItemCells
+      <Td id={labelId} dataLabel={i18n._(t`Name`)}>
-          dataListCells={[
-            <DataListCell key="name">
         <Link to={`${detailUrl}`}>
           <b>{credential.name}</b>
         </Link>
-            </DataListCell>,
+      </Td>
-            <DataListCell key="type">
+      <Td dataLabel={i18n._(t`Type`)}>
         {credential.summary_fields.credential_type.name}
-            </DataListCell>,
+      </Td>
-          ]}
+      <ActionsTd dataLabel={i18n._(t`Actions`)}>
-        />
+        <ActionItem visible={canEdit} tooltip={i18n._(t`Edit Credential`)}>
-        <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`)}
@@ -98,9 +71,8 @@ function CredentialListItem({
           >
             <PencilAltIcon />
           </Button>
-            </Tooltip>
+        </ActionItem>
-          )}
+        <ActionItem visible={credential.summary_fields.user_capabilities.copy}>
-          {credential.summary_fields.user_capabilities.copy && (
           <CopyButton
             isDisabled={isDisabled}
             onCopyStart={handleCopyStart}
@@ -111,10 +83,9 @@ function CredentialListItem({
               errorMessage: i18n._(t`Failed to copy credential.`),
             }}
           />
-          )}
+        </ActionItem>
-        </DataListAction>
+      </ActionsTd>
-      </DataListItemRow>
+    </Tr>
-    </DataListItem>
   );
 }
 

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(
+      <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(
+      <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(
+      <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(
+      <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(
+      <table>
+        <tbody>
           <CredentialListItem
             isSelected={false}
             detailUrl="/foo/bar"
             onSelect={() => {}}
             credential={mockCredentials.results[1]}
           />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('CopyButton').length).toBe(0);
   });

awx/ui_next/src/screens/Project/ProjectList/ProjectList.jsx

@@ -9,10 +9,14 @@ import useRequest, { useDeleteItems } from '../../../util/useRequest';
 import AlertModal from '../../../components/AlertModal';
 import DataListToolbar from '../../../components/DataListToolbar';
 import ErrorDetail from '../../../components/ErrorDetail';
-import PaginatedDataList, {
+import {
   ToolbarAddButton,
   ToolbarDeleteButton,
 } from '../../../components/PaginatedDataList';
+import PaginatedTable, {
+  HeaderRow,
+  HeaderCell,
+} from '../../../components/PaginatedTable';
 import useWsProjects from './useWsProjects';
 import { getQSConfig, parseQueryString } from '../../../util/qs';
 
@@ -116,7 +120,7 @@ function ProjectList({ i18n }) {
     <Fragment>
       <PageSection>
         <Card>
-          <PaginatedDataList
+          <PaginatedTable
             contentError={contentError}
             hasContentLoading={hasContentLoading}
             items={projects}
@@ -160,12 +164,15 @@ function ProjectList({ i18n }) {
             ]}
             toolbarSearchableKeys={searchableKeys}
             toolbarRelatedSearchableKeys={relatedSearchableKeys}
-            toolbarSortColumns={[
+            headerRow={
-              {
+              <HeaderRow qsConfig={QS_CONFIG}>
-                name: i18n._(t`Name`),
+                <HeaderCell sortKey="name">{i18n._(t`Name`)}</HeaderCell>
-                key: 'name',
+                <HeaderCell>{i18n._(t`Status`)}</HeaderCell>
-              },
+                <HeaderCell>{i18n._(t`Type`)}</HeaderCell>
-            ]}
+                <HeaderCell>{i18n._(t`Revision`)}</HeaderCell>
+                <HeaderCell>{i18n._(t`Actions`)}</HeaderCell>
+              </HeaderRow>
+            }
             renderToolbar={props => (
               <DataListToolbar
                 {...props}
@@ -191,14 +198,15 @@ function ProjectList({ i18n }) {
                 ]}
               />
             )}
-            renderItem={o => (
+            renderRow={(project, index) => (
               <ProjectListItem
                 fetchProjects={fetchProjects}
-                key={o.id}
+                key={project.id}
-                project={o}
+                project={project}
-                detailUrl={`${match.url}/${o.id}`}
+                detailUrl={`${match.url}/${project.id}`}
-                isSelected={selected.some(row => row.id === o.id)}
+                isSelected={selected.some(row => row.id === project.id)}
-                onSelect={() => handleSelect(o)}
+                onSelect={() => handleSelect(project)}
+                rowIndex={index}
               />
             )}
             emptyStateControls={

awx/ui_next/src/screens/Project/ProjectList/ProjectListItem.jsx

@@ -2,37 +2,22 @@ import 'styled-components/macro';
 import React, { Fragment, useState, useCallback } from 'react';
 import { string, bool, func } from 'prop-types';
 import { withI18n } from '@lingui/react';
-import {
+import { Button, Tooltip } from '@patternfly/react-core';
-  Button,
+import { Tr, Td } from '@patternfly/react-table';
-  DataListAction as _DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemRow,
-  DataListItemCells,
-  Tooltip,
-} from '@patternfly/react-core';
-
 import { t } from '@lingui/macro';
 import { Link } from 'react-router-dom';
 import { PencilAltIcon } from '@patternfly/react-icons';
 import styled from 'styled-components';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
 import { formatDateString, timeOfDay } from '../../../util/dates';
 import { ProjectsAPI } from '../../../api';
 import ClipboardCopyButton from '../../../components/ClipboardCopyButton';
-import StatusIcon from '../../../components/StatusIcon';
+import StatusLabel from '../../../components/StatusLabel';
-import DataListCell from '../../../components/DataListCell';
 import { toTitleCase } from '../../../util/strings';
 import CopyButton from '../../../components/CopyButton';
 import ProjectSyncButton from '../shared/ProjectSyncButton';
 import { Project } from '../../../types';
 
-const DataListAction = styled(_DataListAction)`
-  align-items: center;
-  display: grid;
-  grid-gap: 16px;
-  grid-template-columns: repeat(3, 40px);
-`;
-
 const Label = styled.span`
   color: var(--pf-global--disabled-color--100);
 `;
@@ -42,8 +27,9 @@ function ProjectListItem({
   isSelected,
   onSelect,
   detailUrl,
-  i18n,
   fetchProjects,
+  rowIndex,
+  i18n,
 }) {
   const [isDisabled, setIsDisabled] = useState(false);
   ProjectListItem.propTypes = {
@@ -88,51 +74,41 @@ function ProjectListItem({
   }, []);
 
   const labelId = `check-action-${project.id}`;
+
   return (
-    <DataListItem
+    <Tr id={`${project.id}`}>
-      key={project.id}
+      <Td
-      aria-labelledby={labelId}
+        select={{
-      id={`${project.id}`}
+          rowIndex,
-    >
+          isSelected,
-      <DataListItemRow>
+          onSelect,
-        <DataListCheck
+        }}
-          id={`select-project-${project.id}`}
+        dataLabel={i18n._(t`Selected`)}
-          checked={isSelected}
-          onChange={onSelect}
-          aria-labelledby={labelId}
       />
-        <DataListItemCells
+      <Td id={labelId} dataLabel={i18n._(t`Name`)}>
-          dataListCells={[
-            <DataListCell key="status" isFilled={false}>
-              {project.summary_fields.last_job && (
-                <Tooltip
-                  position="top"
-                  content={generateLastJobTooltip(
-                    project.summary_fields.last_job
-                  )}
-                  key={project.summary_fields.last_job.id}
-                >
-                  <Link
-                    to={`/jobs/project/${project.summary_fields.last_job.id}`}
-                  >
-                    <StatusIcon
-                      status={project.summary_fields.last_job.status}
-                    />
-                  </Link>
-                </Tooltip>
-              )}
-            </DataListCell>,
-            <DataListCell key="name">
         <Link id={labelId} to={`${detailUrl}`}>
           <b>{project.name}</b>
         </Link>
-            </DataListCell>,
+      </Td>
-            <DataListCell key="type">
+      <Td dataLabel={i18n._(t`Status`)}>
+        {project.summary_fields.last_job && (
+          <Tooltip
+            position="top"
+            content={generateLastJobTooltip(project.summary_fields.last_job)}
+            key={project.summary_fields.last_job.id}
+          >
+            <Link to={`/jobs/project/${project.summary_fields.last_job.id}`}>
+              <StatusLabel status={project.summary_fields.last_job.status} />
+            </Link>
+          </Tooltip>
+        )}
+      </Td>
+      <Td dataLabel={i18n._(t`Type`)}>
         {project.scm_type === ''
           ? i18n._(t`Manual`)
           : toTitleCase(project.scm_type)}
-            </DataListCell>,
+      </Td>
-            <DataListCell key="revision">
+      <Td dataLabel={i18n._(t`Revision`)}>
         {project.scm_revision.substring(0, 7)}
         {!project.scm_revision && (
           <Label aria-label={i18n._(t`copy to clipboard disabled`)}>
@@ -145,21 +121,18 @@ function ProjectListItem({
           copyTip={i18n._(t`Copy full revision to clipboard.`)}
           copiedSuccessTip={i18n._(t`Successfully copied to clipboard!`)}
         />
-            </DataListCell>,
+      </Td>
-          ]}
+      <ActionsTd dataLabel={i18n._(t`Actions`)}>
-        />
+        <ActionItem
-        <DataListAction
+          visible={project.summary_fields.user_capabilities.start}
-          aria-label={i18n._(t`actions`)}
+          tooltip={i18n._(t`Sync Project`)}
-          aria-labelledby={labelId}
-          id={labelId}
         >
-          {project.summary_fields.user_capabilities.start && (
-            <Tooltip content={i18n._(t`Sync Project`)} position="top">
           <ProjectSyncButton projectId={project.id} />
-            </Tooltip>
+        </ActionItem>
-          )}
+        <ActionItem
-          {project.summary_fields.user_capabilities.edit ? (
+          visible={project.summary_fields.user_capabilities.edit}
-            <Tooltip content={i18n._(t`Edit Project`)} position="top">
+          tooltip={i18n._(t`Edit Project`)}
+        >
           <Button
             isDisabled={isDisabled}
             aria-label={i18n._(t`Edit Project`)}
@@ -169,11 +142,8 @@ function ProjectListItem({
           >
             <PencilAltIcon />
           </Button>
-            </Tooltip>
+        </ActionItem>
-          ) : (
+        <ActionItem visible={project.summary_fields.user_capabilities.copy}>
-            ''
-          )}
-          {project.summary_fields.user_capabilities.copy && (
           <CopyButton
             copyItem={copyProject}
             isDisabled={isDisabled}
@@ -184,10 +154,9 @@ function ProjectListItem({
               errorMessage: i18n._(t`Failed to copy project.`),
             }}
           />
-          )}
+        </ActionItem>
-        </DataListAction>
+      </ActionsTd>
-      </DataListItemRow>
+    </Tr>
-    </DataListItem>
   );
 }
 export default withI18n()(ProjectListItem);

awx/ui_next/src/screens/Project/ProjectList/ProjectListItem.test.jsx

@@ -10,6 +10,8 @@ jest.mock('../../../api/models/Projects');
 describe('<ProjectsListItem />', () => {
   test('launch button shown to users with start capabilities', () => {
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -32,12 +34,16 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('ProjectSyncButton').exists()).toBeTruthy();
   });
 
   test('launch button hidden from users without start capabilities', () => {
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -60,12 +66,16 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('ProjectSyncButton').exists()).toBeFalsy();
   });
 
   test('edit button shown to users with edit capabilities', () => {
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -88,12 +98,16 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeTruthy();
   });
 
   test('edit button hidden from users without edit capabilities', () => {
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -116,6 +130,8 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy();
   });
@@ -123,6 +139,8 @@ describe('<ProjectsListItem />', () => {
   test('should call api to copy project', async () => {
     ProjectsAPI.copy.mockResolvedValue();
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -146,6 +164,8 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
 
     await act(async () =>
@@ -159,6 +179,8 @@ describe('<ProjectsListItem />', () => {
     ProjectsAPI.copy.mockRejectedValue(new Error());
 
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -182,6 +204,8 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     await act(async () =>
       wrapper.find('Button[aria-label="Copy"]').prop('onClick')()
@@ -192,6 +216,8 @@ describe('<ProjectsListItem />', () => {
   });
   test('should not render copy button', async () => {
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/foo/bar"
@@ -215,11 +241,15 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('CopyButton').length).toBe(0);
   });
   test('should render disabled copy to clipboard button', () => {
     const wrapper = mountWithContexts(
+      <table>
+        <tbody>
           <ProjectsListItem
             isSelected={false}
             detailUrl="/project/1"
@@ -242,6 +272,8 @@ describe('<ProjectsListItem />', () => {
               },
             }}
           />
+        </tbody>
+      </table>
     );
     expect(
       wrapper.find('span[aria-label="copy to clipboard disabled"]').text()