From 351d9cc5ba306a6da409666de386deef9f94970c Mon Sep 17 00:00:00 2001 From: Yasser Elsayed Date: Mon, 3 Dec 2018 11:46:05 -0800 Subject: [PATCH] Add loading spinner to custom table while loading items (#405) * add loading spinner to cusom table * tests * pr comments --- frontend/src/Css.tsx | 10 + frontend/src/components/CustomTable.test.tsx | 56 ++-- frontend/src/components/CustomTable.tsx | 46 +++- .../__snapshots__/CustomTable.test.tsx.snap | 142 +++++++--- frontend/src/pages/RunList.test.tsx | 1 + .../RecurringRunsManager.test.tsx.snap | 256 +++++++++--------- .../pages/__snapshots__/RunList.test.tsx.snap | 54 +--- 7 files changed, 333 insertions(+), 232 deletions(-) diff --git a/frontend/src/Css.tsx b/frontend/src/Css.tsx index e9eaa3046a7..3218e776fab 100644 --- a/frontend/src/Css.tsx +++ b/frontend/src/Css.tsx @@ -176,6 +176,15 @@ export const commonCss = stylesheet({ position: 'absolute', top: 'calc(50% - 15px)', }, + busyOverlay: { + backgroundColor: '#ffffffaa', + bottom: 0, + left: 0, + position: 'absolute', + right: 0, + top: 0, + zIndex: 1, + }, buttonAction: { $nest: { '&:disabled': { @@ -254,6 +263,7 @@ export const commonCss = stylesheet({ backgroundRepeat: 'no-repeat', backgroundSize: '100% 40px, 100% 40px, 100% 2px, 100% 2px', overflow: 'auto', + position: 'relative', }, textField: { display: 'flex', diff --git a/frontend/src/components/CustomTable.test.tsx b/frontend/src/components/CustomTable.test.tsx index c79a78662f2..aebd2669396 100644 --- a/frontend/src/components/CustomTable.test.tsx +++ b/frontend/src/components/CustomTable.test.tsx @@ -16,6 +16,7 @@ import * as React from 'react'; import CustomTable, { Column, Row, css, ExpandState } from './CustomTable'; +import TestUtils from '../TestUtils'; import { shallow } from 'enzyme'; const props = { @@ -63,34 +64,39 @@ describe('CustomTable', () => { console.error = consoleErrorBackup; }); - it('renders without rows or columns', () => { + it('renders without rows or columns', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders empty message on no rows', () => { + it('renders empty message on no rows', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders some columns with equal widths without rows', () => { + it('renders some columns with equal widths without rows', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders without the checkboxes if disableSelection is true', () => { + it('renders without the checkboxes if disableSelection is true', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders some columns with descending sort order on first column', () => { + it('renders some columns with descending sort order on first column', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders columns with specified widths', () => { + it('renders columns with specified widths', async () => { const testcolumns = [{ flex: 3, label: 'col1', @@ -99,6 +105,7 @@ describe('CustomTable', () => { label: 'col2', }]; const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); @@ -182,21 +189,24 @@ describe('CustomTable', () => { 'Rows must have the same number of cells defined in columns'); }); - it('renders some rows', () => { + it('renders some rows', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders some rows using a custom renderer', () => { + it('renders some rows using a custom renderer', async () => { columns[0].customRenderer = () => (this is custom output) as any; const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); columns[0].customRenderer = undefined; }); - it('displays warning icon with tooltip if row has error', () => { + it('displays warning icon with tooltip if row has error', async () => { rows[0].error = 'dummy error'; const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); rows[0].error = undefined; }); @@ -276,7 +286,7 @@ describe('CustomTable', () => { const reloadResult = Promise.resolve(''); const spy = () => reloadResult; const tree = shallow(); - await reloadResult; + await TestUtils.flushPromises(); expect(tree.state()).toHaveProperty('maxPageIndex', 0); expect(tree.find('WithStyles(IconButton)').at(0).prop('disabled')).toBeTruthy(); expect(tree.find('WithStyles(IconButton)').at(1).prop('disabled')).toBeTruthy(); @@ -296,7 +306,7 @@ describe('CustomTable', () => { const reloadResult = Promise.resolve('some token'); const spy = jest.fn(() => reloadResult); const tree = shallow(); - await reloadResult; + await TestUtils.flushPromises(); tree.find('WithStyles(IconButton)').at(1).simulate('click'); expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token', pageSize: 10, orderAscending: false, sortBy: '' }); @@ -306,10 +316,10 @@ describe('CustomTable', () => { const reloadResult = Promise.resolve('some token'); const spy = jest.fn(() => reloadResult); const tree = shallow(); - await reloadResult; + await TestUtils.flushPromises(); tree.find('WithStyles(IconButton)').at(1).simulate('click'); - await reloadResult; + await TestUtils.flushPromises(); expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token', pageSize: 10, sortBy: '', orderAscending: false }); expect(tree.state()).toHaveProperty('currentPage', 1); tree.setProps({ rows: [rows[1]] }); @@ -327,11 +337,12 @@ describe('CustomTable', () => { await reloadResult; tree.find('WithStyles(IconButton)').at(0).simulate('click'); - await reloadResult; + await TestUtils.flushPromises(); expect(spy).toHaveBeenLastCalledWith({ pageToken: '', orderAscending: false, sortBy: '', pageSize: 10 }); tree.setProps({ rows }); expect(tree.find('WithStyles(IconButton)').at(0).prop('disabled')).toBeTruthy(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); @@ -341,7 +352,7 @@ describe('CustomTable', () => { const tree = shallow(); tree.find('.' + css.rowsPerPage).simulate('change', { target: { value: 1234 } }); - await reloadResult; + await TestUtils.flushPromises(); expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '', orderAscending: false, sortBy: '' }); expect(tree.state()).toHaveProperty('tokenList', ['', 'some token']); }); @@ -357,34 +368,38 @@ describe('CustomTable', () => { expect(tree.state()).toHaveProperty('tokenList', ['']); }); - it('renders a collapsed row', () => { + it('renders a collapsed row', async () => { const row = { ...rows[0] }; row.expandState = ExpandState.COLLAPSED; const tree = shallow( null} />); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders a collapsed row when selection is disabled', () => { + it('renders a collapsed row when selection is disabled', async () => { const row = { ...rows[0] }; row.expandState = ExpandState.COLLAPSED; const tree = shallow( null} disableSelection={true} />); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders an expanded row', () => { + it('renders an expanded row', async () => { const row = { ...rows[0] }; row.expandState = ExpandState.EXPANDED; const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); - it('renders an expanded row with expanded component below it', () => { + it('renders an expanded row with expanded component below it', async () => { const row = { ...rows[0] }; row.expandState = ExpandState.EXPANDED; const tree = shallow( Hello World} />); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); @@ -400,8 +415,9 @@ describe('CustomTable', () => { expect(stopPropagationSpy).toHaveBeenCalledWith(); }); - it('renders a table with sorting disabled', () => { + it('renders a table with sorting disabled', async () => { const tree = shallow(); + await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); }); }); diff --git a/frontend/src/components/CustomTable.tsx b/frontend/src/components/CustomTable.tsx index 0816a0f03b6..692b9d0e86b 100644 --- a/frontend/src/components/CustomTable.tsx +++ b/frontend/src/components/CustomTable.tsx @@ -19,6 +19,7 @@ import ArrowRight from '@material-ui/icons/ArrowRight'; import Checkbox, { CheckboxProps } from '@material-ui/core/Checkbox'; import ChevronLeft from '@material-ui/icons/ChevronLeft'; import ChevronRight from '@material-ui/icons/ChevronRight'; +import CircularProgress from '@material-ui/core/CircularProgress'; import IconButton from '@material-ui/core/IconButton'; import MenuItem from '@material-ui/core/MenuItem'; import Radio from '@material-ui/core/Radio'; @@ -92,6 +93,9 @@ export const css = stylesheet({ expandButtonExpanded: { transform: 'rotate(90deg)', }, + expandableContainer: { + transition: 'margin 0.2s', + }, expandedContainer: { borderRadius: 10, boxShadow: '0 1px 2px 0 rgba(60,64,67,0.30), 0 1px 3px 1px rgba(60,64,67,0.15)', @@ -167,6 +171,7 @@ interface CustomTableProps { interface CustomTableState { currentPage: number; + isBusy: boolean; maxPageIndex: number; sortOrder: 'asc' | 'desc'; pageSize: number; @@ -182,6 +187,7 @@ export default class CustomTable extends React.Component {/* Body */} -
+
+ {/* Busy experience */} + {this.state.isBusy && ( +
+ + )} + {/* Empty experience */} - {this.props.rows.length === 0 && !!this.props.emptyMessage && ( + {this.props.rows.length === 0 && !!this.props.emptyMessage && !this.state.isBusy && (
{this.props.emptyMessage}
)} {this.props.rows.map((row, i) => { @@ -293,7 +305,8 @@ export default class CustomTable extends React.Component + return (
{ + public async reload(loadRequest?: ListRequest): Promise { // Override the current state with incoming request const request: ListRequest = Object.assign({ orderAscending: this.state.sortOrder === 'asc', @@ -373,17 +386,24 @@ export default class CustomTable extends React.Component, cb?: () => void): void { diff --git a/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap b/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap index 97df21859f0..fc8980cfe01 100644 --- a/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap +++ b/frontend/src/components/__snapshots__/CustomTable.test.tsx.snap @@ -64,9 +64,14 @@ exports[`CustomTable displays warning icon with tooltip if row has error 1`] = `
@@ -287,9 +292,14 @@ exports[`CustomTable renders a collapsed row 1`] = `
@@ -457,9 +467,14 @@ exports[`CustomTable renders a collapsed row when selection is disabled 1`] = `
@@ -611,9 +626,14 @@ exports[`CustomTable renders a table with sorting disabled 1`] = `
@@ -823,9 +843,14 @@ exports[`CustomTable renders an expanded row 1`] = `
@@ -997,9 +1022,14 @@ exports[`CustomTable renders an expanded row with expanded component below it 1`
@@ -1180,6 +1210,11 @@ exports[`CustomTable renders columns with specified widths 1`] = `
@@ -1263,6 +1298,11 @@ exports[`CustomTable renders empty message on no rows 1`] = `
@@ -1396,9 +1436,14 @@ exports[`CustomTable renders new rows after clicking next page, and enables prev
@@ -1906,6 +1961,11 @@ exports[`CustomTable renders some columns with equal widths without rows 1`] = `
@@ -2033,9 +2093,14 @@ exports[`CustomTable renders some rows 1`] = `
@@ -2245,9 +2310,14 @@ exports[`CustomTable renders some rows using a custom renderer 1`] = `
@@ -2417,6 +2487,11 @@ exports[`CustomTable renders without rows or columns 1`] = `
@@ -2534,9 +2609,14 @@ exports[`CustomTable renders without the checkboxes if disableSelection is true
diff --git a/frontend/src/pages/RunList.test.tsx b/frontend/src/pages/RunList.test.tsx index 5da07425c36..824c1142e65 100644 --- a/frontend/src/pages/RunList.test.tsx +++ b/frontend/src/pages/RunList.test.tsx @@ -89,6 +89,7 @@ describe('RunList', () => { const props = generateProps(); const tree = TestUtils.mountWithRouter(); await (tree.instance() as RunList).refresh(); + tree.update(); expect(Apis.runServiceApi.listRuns).toHaveBeenCalledTimes(2); expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith('', 10, RunSortKeys.CREATED_AT + ' desc', undefined, undefined); expect(props.onError).not.toHaveBeenCalled(); diff --git a/frontend/src/pages/__snapshots__/RecurringRunsManager.test.tsx.snap b/frontend/src/pages/__snapshots__/RecurringRunsManager.test.tsx.snap index 0ced353e598..a9c6eb117f6 100644 --- a/frontend/src/pages/__snapshots__/RecurringRunsManager.test.tsx.snap +++ b/frontend/src/pages/__snapshots__/RecurringRunsManager.test.tsx.snap @@ -74,32 +74,32 @@ exports[`RecurringRunsManager reloads the list of runs after enable/disabling 1` className="root" classes={ Object { - "colorInherit": "MuiButton-colorInherit-104", - "contained": "MuiButton-contained-94", - "containedPrimary": "MuiButton-containedPrimary-95", - "containedSecondary": "MuiButton-containedSecondary-96", - "disabled": "MuiButton-disabled-103", - "extendedFab": "MuiButton-extendedFab-101", - "fab": "MuiButton-fab-100", - "flat": "MuiButton-flat-88", - "flatPrimary": "MuiButton-flatPrimary-89", - "flatSecondary": "MuiButton-flatSecondary-90", - "focusVisible": "MuiButton-focusVisible-102", - "fullWidth": "MuiButton-fullWidth-108", - "label": "MuiButton-label-84", - "mini": "MuiButton-mini-105", - "outlined": "MuiButton-outlined-91", - "outlinedPrimary": "MuiButton-outlinedPrimary-92", - "outlinedSecondary": "MuiButton-outlinedSecondary-93", - "raised": "MuiButton-raised-97", - "raisedPrimary": "MuiButton-raisedPrimary-98", - "raisedSecondary": "MuiButton-raisedSecondary-99", - "root": "MuiButton-root-83", - "sizeLarge": "MuiButton-sizeLarge-107", - "sizeSmall": "MuiButton-sizeSmall-106", - "text": "MuiButton-text-85", - "textPrimary": "MuiButton-textPrimary-86", - "textSecondary": "MuiButton-textSecondary-87", + "colorInherit": "MuiButton-colorInherit-113", + "contained": "MuiButton-contained-103", + "containedPrimary": "MuiButton-containedPrimary-104", + "containedSecondary": "MuiButton-containedSecondary-105", + "disabled": "MuiButton-disabled-112", + "extendedFab": "MuiButton-extendedFab-110", + "fab": "MuiButton-fab-109", + "flat": "MuiButton-flat-97", + "flatPrimary": "MuiButton-flatPrimary-98", + "flatSecondary": "MuiButton-flatSecondary-99", + "focusVisible": "MuiButton-focusVisible-111", + "fullWidth": "MuiButton-fullWidth-117", + "label": "MuiButton-label-93", + "mini": "MuiButton-mini-114", + "outlined": "MuiButton-outlined-100", + "outlinedPrimary": "MuiButton-outlinedPrimary-101", + "outlinedSecondary": "MuiButton-outlinedSecondary-102", + "raised": "MuiButton-raised-106", + "raisedPrimary": "MuiButton-raisedPrimary-107", + "raisedSecondary": "MuiButton-raisedSecondary-108", + "root": "MuiButton-root-92", + "sizeLarge": "MuiButton-sizeLarge-116", + "sizeSmall": "MuiButton-sizeSmall-115", + "text": "MuiButton-text-94", + "textPrimary": "MuiButton-textPrimary-95", + "textSecondary": "MuiButton-textSecondary-96", } } color="primary" @@ -114,17 +114,17 @@ exports[`RecurringRunsManager reloads the list of runs after enable/disabling 1` variant="text" >