[ARVADOS-WORKBENCH2] created: 2.4.0-22-g9858ad74
Git user
git at public.arvados.org
Thu Apr 14 13:26:36 UTC 2022
at 9858ad74ab6f81c55c06c8f05b20ccd0ec67dd7a (commit)
commit 9858ad74ab6f81c55c06c8f05b20ccd0ec67dd7a
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Thu Apr 14 10:24:33 2022 -0300
18972: Improves implementation on DataExplorer's "loading..." indicator.
Instead of using globals, I think this solution is more elegant, as it's
based on local component state.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/src/components/data-explorer/data-explorer.tsx b/src/components/data-explorer/data-explorer.tsx
index 051f5d34..0363d333 100644
--- a/src/components/data-explorer/data-explorer.tsx
+++ b/src/components/data-explorer/data-explorer.tsx
@@ -66,6 +66,8 @@ interface DataExplorerDataProps<T> {
contextMenuColumn: boolean;
dataTableDefaultView?: React.ReactNode;
working?: boolean;
+ currentRefresh?: string;
+ currentRoute?: string;
hideColumnSelector?: boolean;
paperProps?: PaperProps;
actions?: React.ReactNode;
@@ -96,16 +98,55 @@ type DataExplorerProps<T> = DataExplorerDataProps<T> &
export const DataExplorer = withStyles(styles)(
class DataExplorerGeneric<T> extends React.Component<DataExplorerProps<T>> {
+ state = {
+ showLoading: false,
+ prevRefresh: '',
+ prevRoute: '',
+ };
+
+ componentDidUpdate(prevProps: DataExplorerProps<T>) {
+ const currentRefresh = this.props.currentRefresh || '';
+ const currentRoute = this.props.currentRoute || '';
+
+ if (currentRoute !== this.state.prevRoute) {
+ // Component already mounted, but the user comes from a route change,
+ // like browsing through a project hierarchy.
+ this.setState({
+ showLoading: this.props.working,
+ prevRoute: currentRoute,
+ });
+ }
+
+ if (currentRefresh !== this.state.prevRefresh) {
+ // Component already mounted, but the user just clicked the
+ // refresh button.
+ this.setState({
+ showLoading: this.props.working,
+ prevRefresh: currentRefresh,
+ });
+ }
+ if (this.state.showLoading && !this.props.working) {
+ this.setState({
+ showLoading: false,
+ });
+ }
+ }
componentDidMount() {
if (this.props.onSetColumns) {
this.props.onSetColumns(this.props.columns);
}
+ // Component just mounted, so we need to show the loading indicator.
+ this.setState({
+ showLoading: this.props.working,
+ prevRefresh: this.props.currentRefresh || '',
+ prevRoute: this.props.currentRoute || '',
+ });
}
render() {
const {
- columns, onContextMenu, onFiltersChange, onSortToggle, working, extractKey,
+ columns, onContextMenu, onFiltersChange, onSortToggle, extractKey,
rowsPerPage, rowsPerPageOptions, onColumnToggle, searchLabel, searchValue, onSearch,
items, itemsAvailable, onRowClick, onRowDoubleClick, classes,
dataTableDefaultView, hideColumnSelector, actions, paperProps, hideSearchInput,
@@ -113,8 +154,7 @@ export const DataExplorer = withStyles(styles)(
doHidePanel, doMaximizePanel, panelName, panelMaximized, elementPath
} = this.props;
- const dataCy = this.props["data-cy"];
- return <Paper className={classes.root} {...paperProps} key={paperKey} data-cy={dataCy}>
+ return <Paper className={classes.root} {...paperProps} key={paperKey} data-cy={this.props["data-cy"]}>
<Grid container direction="column" wrap="nowrap" className={classes.container}>
<div>
{title && <Grid item xs className={classes.title}>{title}</Grid>}
@@ -156,7 +196,7 @@ export const DataExplorer = withStyles(styles)(
onFiltersChange={onFiltersChange}
onSortToggle={onSortToggle}
extractKey={extractKey}
- working={working}
+ working={this.state.showLoading}
defaultView={dataTableDefaultView}
currentItemUuid={currentItemUuid}
currentRoute={paperKey} /></Grid>
diff --git a/src/views-components/data-explorer/data-explorer.tsx b/src/views-components/data-explorer/data-explorer.tsx
index 97e20b0d..06d97038 100644
--- a/src/views-components/data-explorer/data-explorer.tsx
+++ b/src/views-components/data-explorer/data-explorer.tsx
@@ -21,11 +21,6 @@ interface Props {
extractKey?: (item: any) => React.Key;
}
-let prevRoute = '';
-let prevRefresh = '';
-let routeChanged = false;
-let isWorking = false;
-
const mapStateToProps = (state: RootState, { id }: Props) => {
const progress = state.progressIndicator.find(p => p.id === id);
const dataExplorerState = getDataExplorer(state.dataExplorer, id);
@@ -33,23 +28,14 @@ const mapStateToProps = (state: RootState, { id }: Props) => {
const currentRefresh = localStorage.getItem(LAST_REFRESH_TIMESTAMP) || '';
const currentItemUuid = currentRoute === '/workflows' ? state.properties.workflowPanelDetailsUuid : state.detailsPanel.resourceUuid;
- if (currentRoute !== prevRoute || currentRefresh !== prevRefresh) {
- routeChanged = true;
- prevRoute = currentRoute;
- prevRefresh = currentRefresh;
- }
- if (progress?.working) {
- isWorking = true;
- }
-
- const working = routeChanged && isWorking;
-
- if (working && !progress?.working) {
- routeChanged = false;
- isWorking = false;
- }
-
- return { ...dataExplorerState, working, paperKey: currentRoute, currentItemUuid };
+ return {
+ ...dataExplorerState,
+ working: !!progress?.working,
+ currentRefresh: currentRefresh,
+ currentRoute: currentRoute,
+ paperKey: currentRoute,
+ currentItemUuid
+ };
};
const mapDispatchToProps = () => {
commit 139421ac9ab6a7f60ad7762d1787d79533ce6831
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Tue Apr 12 11:32:41 2022 -0300
18972: Records the last refresh click on localStorage for others to act on.
Adding timestamps on the store isn't recommended as the reducers should be
only pure functions so I opted to use the localStorage for this application.
The DataExplorer code now also checks if the last Refresh button click
timestamp changed to decide if it should show the "loading, please wait"
message.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/src/components/refresh-button/refresh-button.test.tsx b/src/components/refresh-button/refresh-button.test.tsx
index f9fa32d2..3a9292e6 100644
--- a/src/components/refresh-button/refresh-button.test.tsx
+++ b/src/components/refresh-button/refresh-button.test.tsx
@@ -6,7 +6,7 @@ import React from "react";
import { Button } from "@material-ui/core";
import { shallow, configure } from "enzyme";
import Adapter from "enzyme-adapter-react-16";
-import { RefreshButton } from './refresh-button';
+import { LAST_REFRESH_TIMESTAMP, RefreshButton } from './refresh-button';
configure({ adapter: new Adapter() });
@@ -31,6 +31,7 @@ describe('<RefreshButton />', () => {
});
it('should pass window location to router', () => {
+ expect(localStorage.getItem(LAST_REFRESH_TIMESTAMP)).toBeFalsy();
// setup
const wrapper = shallow(<RefreshButton {...props} />);
@@ -39,5 +40,6 @@ describe('<RefreshButton />', () => {
// then
expect(props.history.replace).toHaveBeenCalledWith('/');
+ expect(localStorage.getItem(LAST_REFRESH_TIMESTAMP)).not.toBeFalsy();
});
});
diff --git a/src/components/refresh-button/refresh-button.tsx b/src/components/refresh-button/refresh-button.tsx
index 9971547b..e2fe5484 100644
--- a/src/components/refresh-button/refresh-button.tsx
+++ b/src/components/refresh-button/refresh-button.tsx
@@ -26,12 +26,17 @@ interface RefreshButtonProps {
onClick?: () => void;
}
+export const LAST_REFRESH_TIMESTAMP = 'lastRefreshTimestamp';
+
export const RefreshButton = ({ history, classes, onClick }: RouteComponentProps & WithStyles<CssRules> & RefreshButtonProps) =>
<Button
color="primary"
size="small"
variant="contained"
onClick={() => {
+ // Notify interested parties that the refresh button was clicked.
+ const now = (new Date()).getTime();
+ localStorage.setItem(LAST_REFRESH_TIMESTAMP, now.toString());
history.replace(window.location.pathname);
if (onClick) {
onClick();
diff --git a/src/views-components/data-explorer/data-explorer.tsx b/src/views-components/data-explorer/data-explorer.tsx
index 8db551e8..97e20b0d 100644
--- a/src/views-components/data-explorer/data-explorer.tsx
+++ b/src/views-components/data-explorer/data-explorer.tsx
@@ -11,6 +11,7 @@ import { dataExplorerActions } from "store/data-explorer/data-explorer-action";
import { DataColumn } from "components/data-table/data-column";
import { DataColumns } from "components/data-table/data-table";
import { DataTableFilters } from 'components/data-table-filters/data-table-filters-tree';
+import { LAST_REFRESH_TIMESTAMP } from "components/refresh-button/refresh-button";
interface Props {
id: string;
@@ -18,10 +19,10 @@ interface Props {
onContextMenu?: (event: React.MouseEvent<HTMLElement>, item: any, isAdmin?: boolean) => void;
onRowDoubleClick: (item: any) => void;
extractKey?: (item: any) => React.Key;
- working?: boolean;
}
let prevRoute = '';
+let prevRefresh = '';
let routeChanged = false;
let isWorking = false;
@@ -29,11 +30,13 @@ const mapStateToProps = (state: RootState, { id }: Props) => {
const progress = state.progressIndicator.find(p => p.id === id);
const dataExplorerState = getDataExplorer(state.dataExplorer, id);
const currentRoute = state.router.location ? state.router.location.pathname : '';
+ const currentRefresh = localStorage.getItem(LAST_REFRESH_TIMESTAMP) || '';
const currentItemUuid = currentRoute === '/workflows' ? state.properties.workflowPanelDetailsUuid : state.detailsPanel.resourceUuid;
- if (currentRoute !== prevRoute) {
+ if (currentRoute !== prevRoute || currentRefresh !== prevRefresh) {
routeChanged = true;
prevRoute = currentRoute;
+ prevRefresh = currentRefresh;
}
if (progress?.working) {
isWorking = true;
diff --git a/src/views-components/main-content-bar/main-content-bar.tsx b/src/views-components/main-content-bar/main-content-bar.tsx
index a460a518..271d77c1 100644
--- a/src/views-components/main-content-bar/main-content-bar.tsx
+++ b/src/views-components/main-content-bar/main-content-bar.tsx
@@ -13,6 +13,7 @@ import * as Routes from 'routes/routes';
import { toggleDetailsPanel } from 'store/details-panel/details-panel-action';
import RefreshButton from "components/refresh-button/refresh-button";
import { loadSidePanelTreeProjects } from "store/side-panel-tree/side-panel-tree-actions";
+import { Dispatch } from "redux";
type CssRules = "infoTooltip";
@@ -44,46 +45,40 @@ const isButtonVisible = ({ router }: RootState) => {
Routes.matchAllProcessesRoute(pathname) ||
Routes.matchTrashRoute(pathname) ||
Routes.matchFavoritesRoute(pathname);
-
- /* return !Routes.matchWorkflowRoute(pathname) && !Routes.matchUserVirtualMachineRoute(pathname) &&
- * !Routes.matchAdminVirtualMachineRoute(pathname) && !Routes.matchRepositoriesRoute(pathname) &&
- * !Routes.matchSshKeysAdminRoute(pathname) && !Routes.matchSshKeysUserRoute(pathname) &&
- * !Routes.matchSiteManagerRoute(pathname) &&
- * !Routes.matchKeepServicesRoute(pathname) && !Routes.matchComputeNodesRoute(pathname) &&
- * !Routes.matchApiClientAuthorizationsRoute(pathname) && !Routes.matchUsersRoute(pathname) &&
- * !Routes.matchMyAccountRoute(pathname) && !Routes.matchLinksRoute(pathname); */
};
-export const MainContentBar =
- connect((state: RootState) => ({
- buttonVisible: isButtonVisible(state),
- projectUuid: state.detailsPanel.resourceUuid,
- }), (dispatch) => ({
- onDetailsPanelToggle: () => dispatch<any>(toggleDetailsPanel()),
- onRefreshButtonClick: (id) => {
- dispatch<any>(loadSidePanelTreeProjects(id));
- }
- }))(
- withStyles(styles)(
- (props: MainContentBarProps & WithStyles<CssRules> & any) =>
- <Toolbar>
- <Grid container>
- <Grid container item xs alignItems="center">
- <Breadcrumbs />
- </Grid>
- <Grid item>
- <RefreshButton onClick={() => {
- props.onRefreshButtonClick(props.projectUuid);
- }} />
- </Grid>
- <Grid item>
- {props.buttonVisible && <Tooltip title="Additional Info">
- <IconButton data-cy="additional-info-icon" color="inherit" className={props.classes.infoTooltip} onClick={props.onDetailsPanelToggle}>
- <DetailsIcon />
- </IconButton>
- </Tooltip>}
- </Grid>
- </Grid>
- </Toolbar>
- )
- );
+const mapStateToProps = (state: RootState) => ({
+ buttonVisible: isButtonVisible(state),
+ projectUuid: state.detailsPanel.resourceUuid,
+});
+
+const mapDispatchToProps = () => (dispatch: Dispatch) => ({
+ onDetailsPanelToggle: () => dispatch<any>(toggleDetailsPanel()),
+ onRefreshButtonClick: (id) => {
+ dispatch<any>(loadSidePanelTreeProjects(id));
+ }
+});
+
+export const MainContentBar = connect(mapStateToProps, mapDispatchToProps)(withStyles(styles)(
+ (props: MainContentBarProps & WithStyles<CssRules> & any) =>
+ <Toolbar><Grid container>
+ <Grid container item xs alignItems="center">
+ <Breadcrumbs />
+ </Grid>
+ <Grid item>
+ <RefreshButton onClick={() => {
+ props.onRefreshButtonClick(props.projectUuid);
+ }} />
+ </Grid>
+ <Grid item>
+ {props.buttonVisible && <Tooltip title="Additional Info">
+ <IconButton data-cy="additional-info-icon"
+ color="inherit"
+ className={props.classes.infoTooltip}
+ onClick={props.onDetailsPanelToggle}>
+ <DetailsIcon />
+ </IconButton>
+ </Tooltip>}
+ </Grid>
+ </Grid></Toolbar>
+));
commit 3dd4b3bc31fea7f614290131d0ade5c0e0ffc595
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Fri Apr 8 16:58:05 2022 -0300
18972: Avoids data table flickering when reloading data on the same route.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/src/components/data-table/data-table.tsx b/src/components/data-table/data-table.tsx
index e8a6ce69..14dfdaca 100644
--- a/src/components/data-table/data-table.tsx
+++ b/src/components/data-table/data-table.tsx
@@ -86,7 +86,7 @@ type DataTableProps<T> = DataTableDataProps<T> & WithStyles<CssRules>;
export const DataTable = withStyles(styles)(
class Component<T> extends React.Component<DataTableProps<T>> {
render() {
- const { items, classes } = this.props;
+ const { items, classes, working } = this.props;
return <div className={classes.root}>
<div className={classes.content}>
<Table>
@@ -96,16 +96,16 @@ export const DataTable = withStyles(styles)(
</TableRow>
</TableHead>
<TableBody className={classes.tableBody}>
- { this.props.working !== undefined && !this.props.working && items.map(this.renderBodyRow) }
+ { !working && items.map(this.renderBodyRow) }
</TableBody>
</Table>
- { this.props.working &&
+ { !!working &&
<div className={classes.loader}>
<DataTableDefaultView
icon={PendingIcon}
messages={['Loading data, please wait.']} />
</div> }
- {items.length === 0 && this.props.working !== undefined && !this.props.working && this.renderNoItemsPlaceholder()}
+ {items.length === 0 && !working && this.renderNoItemsPlaceholder()}
</div>
</div>;
}
diff --git a/src/views-components/data-explorer/data-explorer.tsx b/src/views-components/data-explorer/data-explorer.tsx
index 900ab94e..8db551e8 100644
--- a/src/views-components/data-explorer/data-explorer.tsx
+++ b/src/views-components/data-explorer/data-explorer.tsx
@@ -21,8 +21,9 @@ interface Props {
working?: boolean;
}
-let data: any[] = [];
-let href: string = '';
+let prevRoute = '';
+let routeChanged = false;
+let isWorking = false;
const mapStateToProps = (state: RootState, { id }: Props) => {
const progress = state.progressIndicator.find(p => p.id === id);
@@ -30,16 +31,20 @@ const mapStateToProps = (state: RootState, { id }: Props) => {
const currentRoute = state.router.location ? state.router.location.pathname : '';
const currentItemUuid = currentRoute === '/workflows' ? state.properties.workflowPanelDetailsUuid : state.detailsPanel.resourceUuid;
- let loading = false;
-
- if (dataExplorerState.items.length > 0 && data === dataExplorerState.items && href !== window.location.href) {
- loading = true
- } else {
- href = window.location.href;
- data = dataExplorerState.items;
+ if (currentRoute !== prevRoute) {
+ routeChanged = true;
+ prevRoute = currentRoute;
+ }
+ if (progress?.working) {
+ isWorking = true;
}
- const working = (progress && progress.working) || loading;
+ const working = routeChanged && isWorking;
+
+ if (working && !progress?.working) {
+ routeChanged = false;
+ isWorking = false;
+ }
return { ...dataExplorerState, working, paperKey: currentRoute, currentItemUuid };
};
@@ -86,5 +91,5 @@ const mapDispatchToProps = () => {
});
};
-export const DataExplorer = connect(mapStateToProps, mapDispatchToProps())(DataExplorerComponent);
+export const DataExplorer = connect(mapStateToProps, mapDispatchToProps)(DataExplorerComponent);
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list