From 2419a592b452ebf53216da2c6fb1d428e6ad922a Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 4 Feb 2021 14:46:29 -0800 Subject: [PATCH] convert NotificationList to tables --- .../NotificationList/NotificationList.jsx | 26 +- .../NotificationList.test.jsx | 4 - .../NotificationList/NotificationListItem.jsx | 111 +-- .../NotificationListItem.test.jsx | 227 +++--- .../NotificationListItem.test.jsx.snap | 758 +++++++++--------- .../components/PaginatedTable/ActionItem.jsx | 10 +- .../components/PaginatedTable/HeaderRow.jsx | 13 +- 7 files changed, 607 insertions(+), 542 deletions(-) diff --git a/awx/ui_next/src/components/NotificationList/NotificationList.jsx b/awx/ui_next/src/components/NotificationList/NotificationList.jsx index 4bdba17027..4422dc8c01 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationList.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationList.jsx @@ -6,7 +6,7 @@ import { t } from '@lingui/macro'; import AlertModal from '../AlertModal'; import ErrorDetail from '../ErrorDetail'; import NotificationListItem from './NotificationListItem'; -import PaginatedDataList from '../PaginatedDataList'; +import PaginatedTable, { HeaderRow, HeaderCell } from '../PaginatedTable'; import { getQSConfig, parseQueryString } from '../../util/qs'; import useRequest from '../../util/useRequest'; import { NotificationTemplatesAPI } from '../../api'; @@ -169,7 +169,7 @@ function NotificationList({ return ( <> - ( + headerRow={ + + {i18n._(t`Name`)} + + {i18n._(t`Type`)} + + {i18n._(t`Options`)} + + } + renderRow={(notification, index) => ( )} /> diff --git a/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx b/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx index 071eb45e9f..720b2f15c0 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx @@ -87,10 +87,6 @@ describe('', () => { wrapper.unmount(); }); - test('initially renders succesfully', () => { - expect(wrapper.find('PaginatedDataList')).toHaveLength(1); - }); - test('should render list fetched of items', () => { expect(NotificationTemplatesAPI.read).toHaveBeenCalled(); expect(NotificationTemplatesAPI.readOptions).toHaveBeenCalled(); diff --git a/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx b/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx index 8419db0977..5b0fc2fad7 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx @@ -3,25 +3,9 @@ import { shape, number, string, bool, func } from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Link } from 'react-router-dom'; -import { - DataListAction as _DataListAction, - DataListItem, - DataListItemCells, - DataListItemRow, - Switch, -} from '@patternfly/react-core'; -import styled from 'styled-components'; -import DataListCell from '../DataListCell'; - -const DataListAction = styled(_DataListAction)` - align-items: center; - display: grid; - grid-gap: 16px; - grid-template-columns: ${props => `repeat(${props.columns}, max-content)`}; -`; -const Label = styled.b` - margin-right: 20px; -`; +import { Switch } from '@patternfly/react-core'; +import { Tr, Td } from '@patternfly/react-table'; +import { ActionsTd, ActionItem } from '../PaginatedTable'; function NotificationListItem({ canToggleNotifications, @@ -37,54 +21,37 @@ function NotificationListItem({ showApprovalsToggle, }) { return ( - - - - - - {notification.name} - - - , - - - {typeLabels[notification.notification_type]} - , - ]} - /> - - {showApprovalsToggle && ( - - toggleNotification( - notification.id, - approvalsTurnedOn, - 'approvals' - ) - } - aria-label={i18n._(t`Toggle notification approvals`)} - /> - )} + + + + {notification.name} + + + + {typeLabels[notification.notification_type]} + + + + + toggleNotification( + notification.id, + approvalsTurnedOn, + 'approvals' + ) + } + aria-label={i18n._(t`Toggle notification approvals`)} + /> + + + + + + - - - + + + ); } diff --git a/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx b/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx index 22afb5373b..983525db04 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx @@ -30,13 +30,17 @@ describe('', () => { test('initially renders succesfully and displays correct label', () => { wrapper = mountWithContexts( - + + + + +
); expect(wrapper.find('NotificationListItem')).toMatchSnapshot(); expect(wrapper.find('Switch').length).toBe(3); @@ -44,46 +48,55 @@ describe('', () => { test('shows approvals toggle when configured', () => { wrapper = mountWithContexts( - + + + + +
); expect(wrapper.find('Switch').length).toBe(4); }); - test('displays correct label in correct column', () => { + test('displays correct type', () => { wrapper = mountWithContexts( - + + + + +
); - const typeCell = wrapper - .find('DataListCell') - .at(1) - .find('div'); + const typeCell = wrapper.find('Td').at(1); expect(typeCell.text()).toContain('Slack'); }); test('handles approvals click when toggle is on', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification approvals"]') @@ -95,15 +108,19 @@ describe('', () => { test('handles approvals click when toggle is off', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification approvals"]') @@ -114,14 +131,18 @@ describe('', () => { test('handles started click when toggle is on', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification start"]') @@ -132,14 +153,18 @@ describe('', () => { test('handles started click when toggle is off', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification start"]') @@ -150,14 +175,18 @@ describe('', () => { test('handles success click when toggle is on', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification success"]') @@ -168,14 +197,18 @@ describe('', () => { test('handles success click when toggle is off', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification success"]') @@ -186,14 +219,18 @@ describe('', () => { test('handles error click when toggle is on', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification failure"]') @@ -204,14 +241,18 @@ describe('', () => { test('handles error click when toggle is off', () => { wrapper = mountWithContexts( - + + + + +
); wrapper .find('Switch[aria-label="Toggle notification failure"]') diff --git a/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap b/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap index 7303fdbd7d..eaf045d78d 100644 --- a/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap +++ b/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap @@ -24,398 +24,442 @@ exports[` initially renders succe } } > - -
  • - -
  • -
    + + + + +
    `; diff --git a/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx b/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx index f9c423fee3..5d4f22d12f 100644 --- a/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx +++ b/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx @@ -13,9 +13,13 @@ export default function ActionItem({ column, tooltip, visible, children }) { grid-column: ${column}; `} > - -
    {children}
    -
    + {tooltip ? ( + +
    {children}
    +
    + ) : ( + children + )} ); } diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx index cec9a984ef..a7b076da57 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx @@ -13,7 +13,12 @@ const Th = styled(PFTh)` --pf-c-table--cell--Overflow: initial; `; -export default function HeaderRow({ qsConfig, isExpandable, children }) { +export default function HeaderRow({ + qsConfig, + isExpandable, + isSelectable, + children, +}) { const location = useLocation(); const history = useHistory(); @@ -49,7 +54,7 @@ export default function HeaderRow({ qsConfig, isExpandable, children }) { {isExpandable && } - + {isSelectable && } {React.Children.map( children, child => @@ -66,6 +71,10 @@ export default function HeaderRow({ qsConfig, isExpandable, children }) { ); } +HeaderRow.defaultProps = { + isSelectable: true, +}; + export function HeaderCell({ sortKey, onSort,