From d95d3fab06c07aca8027d8be47a005b43fe21c80 Mon Sep 17 00:00:00 2001
From: Yuan Gong <gongyuan94@gmail.com>
Date: Fri, 21 Feb 2020 17:45:42 +0800
Subject: [PATCH 1/3] Let artifact details lineage view use a subpath

---
 frontend/src/components/Router.tsx     | 13 ++--
 frontend/src/pages/ArtifactDetails.tsx | 95 +++++++++++++++++---------
 2 files changed, 73 insertions(+), 35 deletions(-)

diff --git a/frontend/src/components/Router.tsx b/frontend/src/components/Router.tsx
index aecdb16a4cf..2c12df73078 100644
--- 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/pages/ArtifactDetails.tsx b/frontend/src/pages/ArtifactDetails.tsx
index 1024da3e732..bdfbd3aa633 100644
--- a/frontend/src/pages/ArtifactDetails.tsx
+++ b/frontend/src/pages/ArtifactDetails.tsx
@@ -26,7 +26,7 @@ import {
   LineageResource,
 } from '@kubeflow/frontend';
 import * as React from 'react';
-import { Page } from './Page';
+import { Page, PageProps } from './Page';
 import { ToolbarProps } from '../components/Toolbar';
 import { RoutePage, RoutePageFactory, RouteParams } from '../components/Router';
 import { classes } from 'typestyle';
@@ -34,7 +34,8 @@ 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 { serviceErrorToString, logger } from '../lib/Utils';
+import { Route, Switch } from 'react-router-dom';
 
 export enum ArtifactDetailsTab {
   OVERVIEW = 0,
@@ -52,10 +53,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 +85,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 +95,49 @@ 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>
+          <Route path={`${this.props.match.path}/`} exact>
+            <>
+              <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`}>
+            <>
+              <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 +191,24 @@ 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(
+          `${RoutePageFactory.artifactDetails(this.fullTypeName, this.id)}/lineage`,
+        );
+      case ArtifactDetailsTab.OVERVIEW:
+        return this.props.history.push(
+          `${RoutePageFactory.artifactDetails(this.fullTypeName, this.id)}`,
+        );
+      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;

From d8f856428716193cb6034d8964dbd9bd0ac75087 Mon Sep 17 00:00:00 2001
From: Yuan Gong <gongyuan94@gmail.com>
Date: Fri, 21 Feb 2020 17:53:22 +0800
Subject: [PATCH 2/3] Organize imports

---
 frontend/src/pages/ArtifactDetails.tsx | 38 ++++++++++++++------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/frontend/src/pages/ArtifactDetails.tsx b/frontend/src/pages/ArtifactDetails.tsx
index bdfbd3aa633..cff102fe586 100644
--- a/frontend/src/pages/ArtifactDetails.tsx
+++ b/frontend/src/pages/ArtifactDetails.tsx
@@ -20,28 +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, PageProps } 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, logger } from '../lib/Utils';
-import { Route, Switch } from 'react-router-dom';
+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' },
@@ -104,7 +106,11 @@ class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
     return (
       <div className={classes(commonCss.page)}>
         <Switch>
-          <Route path={`${this.props.match.path}/`} exact>
+          {/*
+           ** 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
@@ -122,7 +128,7 @@ class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
               </div>
             </>
           </Route>
-          <Route path={`${this.props.match.path}/lineage`}>
+          <Route path={`${this.props.match.path}/${LINEAGE_PATH}`} exact={true}>
             <>
               <div className={classes(padding(20, 't'))}>
                 <MD2Tabs
@@ -193,13 +199,9 @@ class ArtifactDetails extends Page<{}, ArtifactDetailsState> {
   private switchTab = (selectedTab: number) => {
     switch (selectedTab) {
       case ArtifactDetailsTab.LINEAGE_EXPLORER:
-        return this.props.history.push(
-          `${RoutePageFactory.artifactDetails(this.fullTypeName, this.id)}/lineage`,
-        );
+        return this.props.history.push(`${this.props.match.url}/${LINEAGE_PATH}`);
       case ArtifactDetailsTab.OVERVIEW:
-        return this.props.history.push(
-          `${RoutePageFactory.artifactDetails(this.fullTypeName, this.id)}`,
-        );
+        return this.props.history.push(this.props.match.url);
       default:
         logger.error(`Unknown selected tab ${selectedTab}.`);
     }

From cfb7b4e1335594446d470108e05974c5e1ced52b Mon Sep 17 00:00:00 2001
From: Yuan Gong <gongyuan94@gmail.com>
Date: Fri, 21 Feb 2020 18:15:57 +0800
Subject: [PATCH 3/3] Update snapshot

---
 frontend/src/components/__snapshots__/Router.test.tsx.snap | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frontend/src/components/__snapshots__/Router.test.tsx.snap b/frontend/src/components/__snapshots__/Router.test.tsx.snap
index 24a04eed400..ab74eababf4 100644
--- 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]}