[MLMD][Lineage] Navigate to ArtifactDetails Overview on row click [l…

…ong term] (#3141) * Let artifact details lineage view use a subpath * Organize imports * Update snapshot
kubeflow · Feb 25, 2020 · 2e39867 · 2e39867
1 parent 968e583
commit 2e39867
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 43 deletions.
diff --git a/frontend/src/components/Router.tsx b/frontend/src/components/Router.tsx
@@ -46,7 +46,12 @@ import NewPipelineVersion from '../pages/NewPipelineVersion';
 import { GettingStarted } from '../pages/GettingStarted';
 import { KFP_FLAGS, Deployments } from '../lib/Flags';
 
-export type RouteConfig = { path: string; Component: React.ComponentType<any>; view?: any };
+export type RouteConfig = {
+  path: string;
+  Component: React.ComponentType<any>;
+  view?: any;
+  notExact?: boolean;
+};
 
 const css = stylesheet({
   dialog: {
@@ -154,7 +159,7 @@ const Router: React.FC<RouterProps> = ({ configs }) => {
     { path: RoutePage.START, Component: GettingStarted },
     { path: RoutePage.ARCHIVE, Component: Archive },
     { path: RoutePage.ARTIFACTS, Component: ArtifactList },
-    { path: RoutePage.ARTIFACT_DETAILS, Component: ArtifactDetails },
+    { path: RoutePage.ARTIFACT_DETAILS, Component: ArtifactDetails, notExact: true },
     { path: RoutePage.EXECUTIONS, Component: ExecutionList },
     { path: RoutePage.EXECUTION_DETAILS, Component: ExecutionDetails },
     {
@@ -194,7 +199,7 @@ const Router: React.FC<RouterProps> = ({ configs }) => {
             // network response handlers.
             <Route
               key={i}
-              exact={true}
+              exact={!route.notExact}
               path={path}
               render={props => <RoutedPage key={props.location.key} route={route} />}
             />
@@ -251,7 +256,7 @@ class RoutedPage extends React.Component<{ route?: RouteConfig }, RouteComponent
               const { path, Component, ...otherProps } = { ...route };
               return (
                 <Route
-                  exact={true}
+                  exact={!route.notExact}
                   path={path}
                   render={({ ...props }) => (
                     <Component {...props} {...childProps} {...otherProps} />

diff --git a/frontend/src/components/__snapshots__/Router.test.tsx.snap b/frontend/src/components/__snapshots__/Router.test.tsx.snap
@@ -27,7 +27,7 @@ exports[`Router initial render 1`] = `
       render={[Function]}
     />
     <Route
-      exact={true}
+      exact={false}
       key="3"
       path="/artifact_types/:artifactType+/artifacts/:id"
       render={[Function]}

diff --git a/frontend/src/pages/ArtifactDetails.tsx b/frontend/src/pages/ArtifactDetails.tsx
@@ -20,27 +20,30 @@ import {
   ArtifactCustomProperties,
   ArtifactProperties,
   GetArtifactsByIDRequest,
-  LineageView,
   getResourceProperty,
-  titleCase,
   LineageResource,
+  LineageView,
+  titleCase,
 } from '@kubeflow/frontend';
+import { CircularProgress } from '@material-ui/core';
 import * as React from 'react';
-import { Page } from './Page';
-import { ToolbarProps } from '../components/Toolbar';
-import { RoutePage, RoutePageFactory, RouteParams } from '../components/Router';
+import { Route, Switch } from 'react-router-dom';
 import { classes } from 'typestyle';
-import { commonCss, padding } from '../Css';
-import { CircularProgress } from '@material-ui/core';
-import { ResourceInfo, ResourceType } from '../components/ResourceInfo';
 import MD2Tabs from '../atoms/MD2Tabs';
-import { serviceErrorToString } from '../lib/Utils';
+import { ResourceInfo, ResourceType } from '../components/ResourceInfo';
+import { RoutePage, RoutePageFactory, RouteParams } from '../components/Router';
+import { ToolbarProps } from '../components/Toolbar';
+import { commonCss, padding } from '../Css';
+import { logger, serviceErrorToString } from '../lib/Utils';
+import { Page, PageProps } from './Page';
 
 export enum ArtifactDetailsTab {
   OVERVIEW = 0,
   LINEAGE_EXPLORER = 1,
 }
 
+const LINEAGE_PATH = 'lineage';
+
 const TABS = {
   [ArtifactDetailsTab.OVERVIEW]: { name: 'Overview' },
   [ArtifactDetailsTab.LINEAGE_EXPLORER]: { name: 'Lineage Explorer' },
@@ -52,10 +55,9 @@ const TAB_NAMES = [ArtifactDetailsTab.OVERVIEW, ArtifactDetailsTab.LINEAGE_EXPLO
 
 interface ArtifactDetailsState {
   artifact?: Artifact;
-  selectedTab: ArtifactDetailsTab;
 }
 
-export default class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
+class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
   private get fullTypeName(): string {
     return this.props.match.params[RouteParams.ARTIFACT_TYPE] || '';
   }
@@ -85,9 +87,7 @@ export default class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
     return route.replace(`:${RouteParams.ID}`, String(resource.getId()));
   }
 
-  public state: ArtifactDetailsState = {
-    selectedTab: ArtifactDetailsTab.OVERVIEW,
-  };
+  public state: ArtifactDetailsState = {};
 
   private api = Api.getInstance();
 
@@ -97,32 +97,53 @@ export default class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
 
   public render(): JSX.Element {
     if (!this.state.artifact) {
-      return <CircularProgress />;
+      return (
+        <div className={commonCss.page}>
+          <CircularProgress className={commonCss.absoluteCenter} />
+        </div>
+      );
     }
     return (
       <div className={classes(commonCss.page)}>
-        <div className={classes(padding(20, 't'))}>
-          <MD2Tabs
-            tabs={TAB_NAMES}
-            selectedTab={this.state.selectedTab}
-            onSwitch={this.switchTab}
-          />
-        </div>
-        {this.state.selectedTab === ArtifactDetailsTab.OVERVIEW && (
-          <div className={classes(padding(20, 'lr'))}>
-            <ResourceInfo
-              resourceType={ResourceType.ARTIFACT}
-              typeName={this.properTypeName}
-              resource={this.state.artifact}
-            />
-          </div>
-        )}
-        {this.state.selectedTab === ArtifactDetailsTab.LINEAGE_EXPLORER && (
-          <LineageView
-            target={this.state.artifact}
-            buildResourceDetailsPageRoute={ArtifactDetails.buildResourceDetailsPageRoute}
-          />
-        )}
+        <Switch>
+          {/*
+           ** This is react-router's nested route feature.
+           ** reference: https://reacttraining.com/react-router/web/example/nesting
+           */}
+          <Route path={this.props.match.path} exact={true}>
+            <>
+              <div className={classes(padding(20, 't'))}>
+                <MD2Tabs
+                  tabs={TAB_NAMES}
+                  selectedTab={ArtifactDetailsTab.OVERVIEW}
+                  onSwitch={this.switchTab}
+                />
+              </div>
+              <div className={classes(padding(20, 'lr'))}>
+                <ResourceInfo
+                  resourceType={ResourceType.ARTIFACT}
+                  typeName={this.properTypeName}
+                  resource={this.state.artifact}
+                />
+              </div>
+            </>
+          </Route>
+          <Route path={`${this.props.match.path}/${LINEAGE_PATH}`} exact={true}>
+            <>
+              <div className={classes(padding(20, 't'))}>
+                <MD2Tabs
+                  tabs={TAB_NAMES}
+                  selectedTab={ArtifactDetailsTab.LINEAGE_EXPLORER}
+                  onSwitch={this.switchTab}
+                />
+              </div>
+              <LineageView
+                target={this.state.artifact}
+                buildResourceDetailsPageRoute={ArtifactDetails.buildResourceDetailsPageRoute}
+              />
+            </>
+          </Route>
+        </Switch>
       </div>
     );
   }
@@ -176,6 +197,20 @@ export default class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
   };
 
   private switchTab = (selectedTab: number) => {
-    this.setState({ selectedTab });
+    switch (selectedTab) {
+      case ArtifactDetailsTab.LINEAGE_EXPLORER:
+        return this.props.history.push(`${this.props.match.url}/${LINEAGE_PATH}`);
+      case ArtifactDetailsTab.OVERVIEW:
+        return this.props.history.push(this.props.match.url);
+      default:
+        logger.error(`Unknown selected tab ${selectedTab}.`);
+    }
   };
 }
+
+// This guarantees that each artifact renders a different <ArtifactDetails /> instance.
+const EnhancedArtifactDetails = (props: PageProps) => {
+  return <ArtifactDetails {...props} key={props.match.params[RouteParams.ID]} />;
+};
+
+export default EnhancedArtifactDetails;