Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push pagination control to CustomTable #31

Merged
merged 6 commits into from
Nov 4, 2018
Merged

Push pagination control to CustomTable #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ec5a506
initial, moving paging into custom table, tests disabled
Nov 3, 2018
bce08bb
wip, rest of custom table occurrences, run list broken
Nov 3, 2018
0d08bf6
fix run list and others
Nov 3, 2018
13dc178
tests
Nov 3, 2018
1fbbd99
cleanup
Nov 3, 2018
a53b313
pr comments
Nov 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions frontend/src/components/CustomTable.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ describe('CustomTable', () => {
});

it('renders some columns with descending sort order on first column', () => {
const tree = shallow(<CustomTable {...props} orderAscending={false}
const tree = shallow(<CustomTable {...props} initialSortOrder='desc'
columns={[{ label: 'col1', sortKey: 'col1sortkey' }, { label: 'col2' }]} />);
expect(tree).toMatchSnapshot();
});
Expand All @@ -105,7 +105,7 @@ describe('CustomTable', () => {
it('calls reload function with an empty page token to get rows', () => {
const reload = jest.fn();
shallow(<CustomTable {...props} reload={reload} />);
expect(reload).toHaveBeenLastCalledWith({ pageToken: '' });
expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, orderAscending: false, sortBy: '' });
});

it('calls reload function with sort key of clicked column, while keeping same page', () => {
Expand All @@ -120,11 +120,11 @@ describe('CustomTable', () => {
}];
const reload = jest.fn();
const tree = shallow(<CustomTable {...props} reload={reload} columns={testcolumns} />);
expect(reload).toHaveBeenLastCalledWith({ pageToken: '' });
expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, sortBy: 'col1sortkey desc', orderAscending: false });

tree.find('WithStyles(TableSortLabel)').at(0).simulate('click');
tree.find('WithStyles(TableSortLabel)').at(1).simulate('click');
expect(reload).toHaveBeenLastCalledWith({
orderAscending: true, pageToken: '', sortBy: 'col1sortkey'
orderAscending: true, pageSize: 10, pageToken: '', sortBy: 'col2sortkey',
});
});

Expand All @@ -140,16 +140,16 @@ describe('CustomTable', () => {
}];
const reload = jest.fn();
const tree = shallow(<CustomTable {...props} reload={reload} columns={testcolumns} />);
expect(reload).toHaveBeenLastCalledWith({ pageToken: '' });
expect(reload).toHaveBeenLastCalledWith({ pageToken: '', orderAscending: false, pageSize: 10, sortBy: 'col1sortkey desc' });

tree.find('WithStyles(TableSortLabel)').at(0).simulate('click');
expect(reload).toHaveBeenLastCalledWith({
orderAscending: true, pageToken: '', sortBy: 'col1sortkey'
tree.find('WithStyles(TableSortLabel)').at(1).simulate('click');
expect(reload).toHaveBeenLastCalledWith({
orderAscending: true, pageSize: 10, pageToken: '', sortBy: 'col2sortkey',
});
tree.setProps({ sortBy: 'col1sortkey' });
tree.find('WithStyles(TableSortLabel)').at(0).simulate('click');
tree.find('WithStyles(TableSortLabel)').at(1).simulate('click');
expect(reload).toHaveBeenLastCalledWith({
orderAscending: false, pageToken: '', sortBy: 'col1sortkey'
orderAscending: false, pageSize: 10, pageToken: '', sortBy: 'col2sortkey desc'
});
});

Expand All @@ -163,10 +163,10 @@ describe('CustomTable', () => {
}];
const reload = jest.fn();
const tree = shallow(<CustomTable {...props} reload={reload} columns={testcolumns} />);
expect(reload).toHaveBeenLastCalledWith({ pageToken: '' });
expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, orderAscending: false, sortBy: '' });

tree.find('WithStyles(TableSortLabel)').at(0).simulate('click');
expect(reload).toHaveBeenLastCalledWith({ pageToken: '' });
expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, orderAscending: false, sortBy: '' });
});

it('logs error if row has more cells than columns', () => {
Expand Down Expand Up @@ -299,7 +299,7 @@ describe('CustomTable', () => {
await reloadResult;

tree.find('WithStyles(IconButton)').at(1).simulate('click');
expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token' });
expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token', pageSize: 10, orderAscending: false, sortBy: '' });
});

it('renders new rows after clicking next page, and enables previous page button', async () => {
Expand All @@ -310,7 +310,7 @@ describe('CustomTable', () => {

tree.find('WithStyles(IconButton)').at(1).simulate('click');
await reloadResult;
expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token' });
expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token', pageSize: 10, sortBy: '', orderAscending: false });
expect(tree.state()).toHaveProperty('currentPage', 1);
tree.setProps({ rows: [rows[1]] });
expect(tree).toMatchSnapshot();
Expand All @@ -328,7 +328,7 @@ describe('CustomTable', () => {

tree.find('WithStyles(IconButton)').at(0).simulate('click');
await reloadResult;
expect(spy).toHaveBeenLastCalledWith({ pageToken: '' });
expect(spy).toHaveBeenLastCalledWith({ pageToken: '', orderAscending: false, sortBy: '', pageSize: 10 });

tree.setProps({ rows });
expect(tree.find('WithStyles(IconButton)').at(0).prop('disabled')).toBeTruthy();
Expand All @@ -342,7 +342,7 @@ describe('CustomTable', () => {

tree.find('.' + css.rowsPerPage).simulate('change', { target: { value: 1234 } });
await reloadResult;
expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '' });
expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '', orderAscending: false, sortBy: '' });
expect(tree.state()).toHaveProperty('tokenList', ['', 'some token']);
});

Expand All @@ -353,7 +353,7 @@ describe('CustomTable', () => {

tree.find('.' + css.rowsPerPage).simulate('change', { target: { value: 1234 } });
await reloadResult;
expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '' });
expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '', orderAscending: false, sortBy: '' });
expect(tree.state()).toHaveProperty('tokenList', ['']);
});

Expand Down
68 changes: 51 additions & 17 deletions frontend/src/components/CustomTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import TableSortLabel from '@material-ui/core/TableSortLabel';
import TextField from '@material-ui/core/TextField';
import Tooltip from '@material-ui/core/Tooltip';
import WarningIcon from '@material-ui/icons/WarningRounded';
import { BaseListRequest } from '../lib/Apis';
import { ListRequest } from '../lib/Apis';
import Checkbox, { CheckboxProps } from '@material-ui/core/Checkbox';
import { TextFieldProps } from '@material-ui/core/TextField';
import { classes, stylesheet } from 'typestyle';
Expand Down Expand Up @@ -145,14 +145,13 @@ interface CustomTableProps {
disablePaging?: boolean;
disableSelection?: boolean;
disableSorting?: boolean;
getExpandComponent?: (index: number) => React.ReactNode;
emptyMessage?: string;
orderAscending: boolean;
pageSize: number;
reload: (request: BaseListRequest) => Promise<string>;
getExpandComponent?: (index: number) => React.ReactNode;
initialSortColumn?: string;
initialSortOrder?: 'asc' | 'desc';
reload: (request: ListRequest) => Promise<string>;
rows: Row[];
selectedIds?: string[];
sortBy: string;
toggleExpansion?: (rowId: number) => void;
updateSelection?: (selectedIds: string[]) => void;
useRadioButtons?: boolean;
Expand All @@ -161,16 +160,23 @@ interface CustomTableProps {
interface CustomTableState {
currentPage: number;
maxPageIndex: number;
sortOrder: 'asc' | 'desc';
pageSize: number;
sortBy: string;
tokenList: string[];
}

export default class CustomTable extends React.Component<CustomTableProps, CustomTableState> {
constructor(props: any) {
constructor(props: CustomTableProps) {
super(props);

this.state = {
currentPage: 0,
maxPageIndex: Number.MAX_SAFE_INTEGER,
pageSize: 10,
sortBy: props.initialSortColumn ||
(props.columns.length ? props.columns[0].sortKey || '' : ''),
sortOrder: props.initialSortOrder || 'desc',
tokenList: [''],
};
}
Expand Down Expand Up @@ -218,7 +224,7 @@ export default class CustomTable extends React.Component<CustomTableProps, Custo
}

public render() {
const { disableSorting, orderAscending, sortBy, pageSize } = this.props;
const { pageSize, sortBy, sortOrder } = this.state;
const numSelected = (this.props.selectedIds || []).length;
const totalFlex = this.props.columns.reduce((total, c) => total += (c.flex || 1), 0);
const widths = this.props.columns.map(c => (c.flex || 1) / totalFlex * 100);
Expand All @@ -228,7 +234,7 @@ export default class CustomTable extends React.Component<CustomTableProps, Custo

{/* Header */}
<div className={classes(
css.header, (this.props.disableSelection || this.props.useRadioButtons) && padding(20, 'l'))}>
css.header, (this.props.disableSelection || this.props.useRadioButtons) && padding(20, 'l'))}>
{(this.props.disableSelection !== true && this.props.useRadioButtons !== true) && (
<div className={classes(css.columnName, css.cell, css.selectionToggle)}>
<Checkbox indeterminate={!!numSelected && numSelected < this.props.rows.length}
Expand All @@ -242,12 +248,12 @@ export default class CustomTable extends React.Component<CustomTableProps, Custo
return (
<div key={i} style={{ width: widths[i] + '%' }}
className={css.columnName}>
{disableSorting === true && <div>{col.label}</div>}
{!disableSorting && (
{this.props.disableSorting === true && <div>{col.label}</div>}
{!this.props.disableSorting && (
<Tooltip title={isColumnSortable ? 'Sort' : 'Cannot sort by this column'}
enterDelay={300}>
<TableSortLabel active={isCurrentSortColumn} className={commonCss.ellipsis}
direction={isColumnSortable ? (orderAscending ? 'asc' : 'desc') : undefined}
direction={isColumnSortable ? sortOrder : undefined}
onClick={() => this._requestSort(this.props.columns[i].sortKey)}>
{col.label}
</TableSortLabel>
Expand Down Expand Up @@ -340,10 +346,38 @@ export default class CustomTable extends React.Component<CustomTableProps, Custo
);
}

private async _requestSort(sortBy?: string) {
public reload(loadRequest?: ListRequest): Promise<string> {
// Override the current state with incoming request
const request: ListRequest = Object.assign({
orderAscending: this.state.sortOrder === 'asc',
pageSize: this.state.pageSize,
pageToken: this.state.tokenList[this.state.currentPage],
sortBy: this.state.sortBy,
}, loadRequest);

this.setState({
pageSize: request.pageSize!,
sortBy: request.sortBy!,
sortOrder: request.orderAscending ? 'asc' : 'desc',
});

if (request.sortBy && !request.orderAscending) {
request.sortBy += ' desc';
}

return this.props.reload(request);
}

private _requestSort(sortBy?: string) {
if (sortBy) {
const orderAscending = this.props.sortBy === sortBy ? !this.props.orderAscending : true;
this._resetToFirstPage(await this.props.reload({ pageToken: '', orderAscending, sortBy }));
// Set the sort column to the provided column if it's different, and
// invert the sort order it if it's the same column
const sortOrder = this.state.sortBy === sortBy ?
(this.state.sortOrder === 'asc' ? 'desc' : 'asc') : 'asc';
this.setState({ sortOrder, sortBy }, async () => {
this._resetToFirstPage(
await this.reload({ pageToken: '', orderAscending: sortOrder === 'asc', sortBy }));
});
}
}

Expand All @@ -353,7 +387,7 @@ export default class CustomTable extends React.Component<CustomTableProps, Custo
newCurrentPage = Math.max(0, newCurrentPage);
newCurrentPage = Math.min(this.state.maxPageIndex, newCurrentPage);

const newPageToken = await this.props.reload({
const newPageToken = await this.reload({
pageToken: this.state.tokenList[newCurrentPage],
});

Expand All @@ -376,7 +410,7 @@ export default class CustomTable extends React.Component<CustomTableProps, Custo
private async _requestRowsPerPage(event: React.ChangeEvent) {
const pageSize = (event.target as TextFieldProps).value as number;

this._resetToFirstPage(await this.props.reload({ pageSize, pageToken: '' }));
this._resetToFirstPage(await this.reload({ pageSize, pageToken: '' }));
}

private _resetToFirstPage(newPageToken?: string) {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/__snapshots__/CustomTable.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1723,7 +1723,7 @@ exports[`CustomTable renders some columns with descending sort order on first co
title="Sort"
>
<WithStyles(TableSortLabel)
active={false}
active={true}
className="ellipsis"
direction="desc"
onClick={[Function]}
Expand Down
19 changes: 1 addition & 18 deletions frontend/src/lib/Apis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { StoragePath } from './WorkflowParser';

const v1beta1Prefix = 'apis/v1beta1';

export interface BaseListRequest {
export interface ListRequest {
filterBy?: string;
orderAscending?: boolean;
pageSize?: number;
Expand Down Expand Up @@ -168,23 +168,6 @@ export class Apis {
}
}

// tslint:disable-next-line:no-empty-interface
export interface ListExperimentsRequest extends BaseListRequest {
}

// tslint:disable-next-line:no-empty-interface
export interface ListJobsRequest extends BaseListRequest {
experimentId?: string;
}

// tslint:disable-next-line:no-empty-interface
export interface ListPipelinesRequest extends BaseListRequest {
}

export interface ListRunsRequest extends BaseListRequest {
experimentId?: string;
}

// Valid sortKeys as specified by the backend.
// Note that '' and 'created_at' are considered equivalent.
export enum RunSortKeys {
Expand Down
Loading