From cdb37800cc639f7c0291bca54882ca9d95f80d8e Mon Sep 17 00:00:00 2001
From: Alan Guo <aguo@anyscale.com>
Date: Sun, 22 Jan 2023 16:23:39 -0800
Subject: [PATCH] Polish the new IA for dashboard (#31770)

Signed-off-by: Alan Guo <aguo@anyscale.com>

Update to more closely match the design spec.
---
 dashboard/client/src/App.tsx                  | 11 ++++-
 .../client/src/common/CollapsibleSection.tsx  | 48 +++++++++++++++----
 dashboard/client/src/pages/job/JobDetail.tsx  |  8 +++-
 .../src/pages/job/JobDetailActorPage.tsx      | 37 ++++++++++++++
 .../client/src/pages/job/JobDetailLayout.tsx  | 12 ++++-
 .../client/src/pages/layout/MainNavLayout.tsx | 19 +++++---
 dashboard/client/src/pages/log/Logs.tsx       |  2 +-
 .../client/src/pages/metrics/Metrics.tsx      | 32 +++++++++----
 .../src/pages/overview/OverviewPage.tsx       | 15 ++++--
 .../pages/overview/cards/NodeCountCard.tsx    |  1 +
 .../src/pages/overview/cards/OverviewCard.tsx |  5 +-
 .../cards/RecentJobsCard.component.test.tsx   |  2 +-
 .../pages/overview/cards/RecentJobsCard.tsx   | 12 +++--
 dashboard/client/src/theme.ts                 | 16 +++++++
 .../metrics/grafana_dashboard_base.json       |  2 +-
 15 files changed, 180 insertions(+), 42 deletions(-)
 create mode 100644 dashboard/client/src/pages/job/JobDetailActorPage.tsx

diff --git a/dashboard/client/src/App.tsx b/dashboard/client/src/App.tsx
index c48be1456e9f8..302a9afe3985c 100644
--- a/dashboard/client/src/App.tsx
+++ b/dashboard/client/src/App.tsx
@@ -8,6 +8,7 @@ import Events from "./pages/event/Events";
 import Loading from "./pages/exception/Loading";
 import JobList, { NewIAJobsPage } from "./pages/job";
 import { JobDetailChartsPage } from "./pages/job/JobDetail";
+import { JobDetailActorsPage } from "./pages/job/JobDetailActorPage";
 import { JobDetailInfoPage } from "./pages/job/JobDetailInfoPage";
 import { JobDetailLayout } from "./pages/job/JobDetailLayout";
 import { DEFAULT_VALUE, MainNavContext } from "./pages/layout/mainNavContext";
@@ -205,11 +206,19 @@ const App = () => {
                       <Route
                         element={
                           <SideTabPage tabId="charts">
-                            <JobDetailChartsPage />
+                            <JobDetailChartsPage newIA />
                           </SideTabPage>
                         }
                         path=""
                       />
+                      <Route
+                        element={
+                          <SideTabPage tabId="actors">
+                            <JobDetailActorsPage />
+                          </SideTabPage>
+                        }
+                        path="actors"
+                      />
                     </Route>
                   </Route>
                   <Route element={<NewIALogsPage />} path="logs">
diff --git a/dashboard/client/src/common/CollapsibleSection.tsx b/dashboard/client/src/common/CollapsibleSection.tsx
index 879ce6bf692c0..2f42d82ff7d6e 100644
--- a/dashboard/client/src/common/CollapsibleSection.tsx
+++ b/dashboard/client/src/common/CollapsibleSection.tsx
@@ -1,6 +1,8 @@
 import { createStyles, makeStyles, Typography } from "@material-ui/core";
-import React, { PropsWithChildren, useState } from "react";
-import { RiArrowDownSLine, RiArrowUpSLine } from "react-icons/ri";
+import classNames from "classnames";
+import React, { PropsWithChildren, useEffect, useState } from "react";
+import { RiArrowDownSLine, RiArrowRightSLine } from "react-icons/ri";
+import { ClassNameProps } from "./props";
 
 const useStyles = makeStyles((theme) =>
   createStyles({
@@ -10,6 +12,7 @@ const useStyles = makeStyles((theme) =>
       flexWrap: "nowrap",
       alignItems: "center",
       fontWeight: 500,
+      cursor: "pointer",
     },
     icon: {
       marginRight: theme.spacing(1),
@@ -17,25 +20,42 @@ const useStyles = makeStyles((theme) =>
       height: 24,
     },
     body: {
-      marginTop: theme.spacing(3),
+      marginTop: theme.spacing(1),
+    },
+    bodyHidden: {
+      display: "none",
     },
   }),
 );
 
-type CollapsibleSectionProps = PropsWithChildren<{
-  title: string;
-  startExpanded?: boolean;
-  className?: string;
-}>;
+type CollapsibleSectionProps = PropsWithChildren<
+  {
+    title: string;
+    startExpanded?: boolean;
+    /**
+     * An optimization to not avoid re-rendering the contents of the collapsible section.
+     * When enabled, we will keep the content around when collapsing but hide it via css.
+     */
+    keepRendered?: boolean;
+  } & ClassNameProps
+>;
 
 export const CollapsibleSection = ({
   title,
   startExpanded = false,
   className,
   children,
+  keepRendered,
 }: CollapsibleSectionProps) => {
   const classes = useStyles();
   const [expanded, setExpanded] = useState(startExpanded);
+  const [rendered, setRendered] = useState(expanded);
+
+  useEffect(() => {
+    if (expanded) {
+      setRendered(true);
+    }
+  }, [expanded]);
 
   const handleExpandClick = () => {
     setExpanded(!expanded);
@@ -51,11 +71,19 @@ export const CollapsibleSection = ({
         {expanded ? (
           <RiArrowDownSLine className={classes.icon} />
         ) : (
-          <RiArrowUpSLine className={classes.icon} />
+          <RiArrowRightSLine className={classes.icon} />
         )}
         {title}
       </Typography>
-      {expanded && <div className={classes.body}>{children}</div>}
+      {(expanded || (keepRendered && rendered)) && (
+        <div
+          className={classNames(classes.body, {
+            [classes.bodyHidden]: !expanded,
+          })}
+        >
+          {children}
+        </div>
+      )}
     </div>
   );
 };
diff --git a/dashboard/client/src/pages/job/JobDetail.tsx b/dashboard/client/src/pages/job/JobDetail.tsx
index cddca7c6761c9..8f0b6cdf1c21d 100644
--- a/dashboard/client/src/pages/job/JobDetail.tsx
+++ b/dashboard/client/src/pages/job/JobDetail.tsx
@@ -25,7 +25,13 @@ const useStyle = makeStyles((theme) => ({
   },
 }));
 
-export const JobDetailChartsPage = () => {
+type JobDetailChartsPageProps = {
+  newIA?: boolean;
+};
+
+export const JobDetailChartsPage = ({
+  newIA = false,
+}: JobDetailChartsPageProps) => {
   const classes = useStyle();
   const { job, msg, params } = useJobDetail();
   const jobId = params.id;
diff --git a/dashboard/client/src/pages/job/JobDetailActorPage.tsx b/dashboard/client/src/pages/job/JobDetailActorPage.tsx
new file mode 100644
index 0000000000000..c0fa8735850cb
--- /dev/null
+++ b/dashboard/client/src/pages/job/JobDetailActorPage.tsx
@@ -0,0 +1,37 @@
+import { makeStyles } from "@material-ui/core";
+import React from "react";
+
+import TitleCard from "../../components/TitleCard";
+import ActorList from "../actor/ActorList";
+import { MainNavPageInfo } from "../layout/mainNavContext";
+import { useJobDetail } from "./hook/useJobDetail";
+
+const useStyle = makeStyles((theme) => ({
+  root: {
+    padding: theme.spacing(2),
+  },
+}));
+
+export const JobDetailActorsPage = () => {
+  const classes = useStyle();
+  const { job, params } = useJobDetail();
+
+  const pageInfo = job
+    ? {
+        title: "Actors",
+        id: "actors",
+        path: job.job_id ? `/new/jobs/${job.job_id}/actors` : undefined,
+      }
+    : {
+        title: "Actors",
+        id: "actors",
+        path: undefined,
+      };
+
+  return (
+    <div className={classes.root}>
+      <MainNavPageInfo pageInfo={pageInfo} />
+      <TitleCard title="Actors">{<ActorList jobId={params.id} />}</TitleCard>
+    </div>
+  );
+};
diff --git a/dashboard/client/src/pages/job/JobDetailLayout.tsx b/dashboard/client/src/pages/job/JobDetailLayout.tsx
index 3c65ccc6b42a6..f87915e451308 100644
--- a/dashboard/client/src/pages/job/JobDetailLayout.tsx
+++ b/dashboard/client/src/pages/job/JobDetailLayout.tsx
@@ -1,5 +1,9 @@
 import React from "react";
-import { RiInformationLine, RiLineChartLine } from "react-icons/ri";
+import {
+  RiGradienterLine,
+  RiInformationLine,
+  RiLineChartLine,
+} from "react-icons/ri";
 import { MainNavPageInfo } from "../layout/mainNavContext";
 import { SideTabLayout, SideTabRouteLink } from "../layout/SideTabLayout";
 import { useJobDetail } from "./hook/useJobDetail";
@@ -29,6 +33,12 @@ export const JobDetailLayout = () => {
         title="Charts"
         Icon={RiLineChartLine}
       />
+      <SideTabRouteLink
+        to="actors"
+        tabId="actors"
+        title="Actors"
+        Icon={RiGradienterLine}
+      />
     </SideTabLayout>
   );
 };
diff --git a/dashboard/client/src/pages/layout/MainNavLayout.tsx b/dashboard/client/src/pages/layout/MainNavLayout.tsx
index 6a001a29eb3b7..85dabc6d97a35 100644
--- a/dashboard/client/src/pages/layout/MainNavLayout.tsx
+++ b/dashboard/client/src/pages/layout/MainNavLayout.tsx
@@ -88,13 +88,14 @@ const useMainNavBarStyles = makeStyles((theme) =>
       boxShadow: "0px 1px 0px #D2DCE6",
     },
     logo: {
-      width: 60,
       display: "flex",
       justifyContent: "center",
+      marginLeft: theme.spacing(2),
+      marginRight: theme.spacing(3),
     },
     navItem: {
-      marginRight: theme.spacing(2),
-      fontSize: "1em",
+      marginRight: theme.spacing(6),
+      fontSize: "1rem",
       fontWeight: 500,
       color: "black",
       textDecoration: "none",
@@ -211,15 +212,21 @@ const MainNavBreadcrumbs = () => {
         );
         if (index === 0) {
           return (
-            <Typography key={id} className={classes.breadcrumbItem}>
+            <Typography
+              key={id}
+              className={classes.breadcrumbItem}
+              variant="body2"
+            >
               {linkOrText}
             </Typography>
           );
         } else {
           return (
             <React.Fragment key={id}>
-              <Typography className={classes.breadcrumbItem}>{"/"}</Typography>
-              <Typography className={classes.breadcrumbItem}>
+              <Typography className={classes.breadcrumbItem} variant="body2">
+                {"/"}
+              </Typography>
+              <Typography className={classes.breadcrumbItem} variant="body2">
                 {linkOrText}
               </Typography>
             </React.Fragment>
diff --git a/dashboard/client/src/pages/log/Logs.tsx b/dashboard/client/src/pages/log/Logs.tsx
index 6501a0c8b7786..7a36a1e23c64c 100644
--- a/dashboard/client/src/pages/log/Logs.tsx
+++ b/dashboard/client/src/pages/log/Logs.tsx
@@ -126,7 +126,7 @@ const Logs = (props: LogsProps) => {
     setEnd,
   } = useLogs(props);
   const { newIA } = props;
-  let href = newIA ? "#/new/log/" : "#/log/";
+  let href = newIA ? "#/new/logs/" : "#/log/";
 
   if (origin) {
     if (path) {
diff --git a/dashboard/client/src/pages/metrics/Metrics.tsx b/dashboard/client/src/pages/metrics/Metrics.tsx
index 554990b3250ef..8610f6961622e 100644
--- a/dashboard/client/src/pages/metrics/Metrics.tsx
+++ b/dashboard/client/src/pages/metrics/Metrics.tsx
@@ -20,9 +20,21 @@ const useStyles = makeStyles((theme) =>
       display: "flex",
       flexDirection: "row",
       flexWrap: "wrap",
+      gap: theme.spacing(3),
+    },
+    chart: {
+      flex: "1 0 448px",
+      maxWidth: "100%",
+      height: 300,
+      overflow: "hidden",
+      [theme.breakpoints.up("md")]: {
+        // Calculate max width based on 1/3 of the total width minus padding between cards
+        maxWidth: `calc((100% - ${theme.spacing(3)}px * 2) / 3)`,
+      },
     },
     grafanaEmbed: {
-      margin: theme.spacing(1),
+      width: "100%",
+      height: "100%",
     },
     topBar: {
       position: "sticky",
@@ -213,15 +225,15 @@ export const Metrics = () => {
           </Alert>
           <div className={classes.grafanaEmbedsContainer}>
             {METRICS_CONFIG.map(({ title, path }) => (
-              <iframe
-                key={title}
-                className={classes.grafanaEmbed}
-                title={title}
-                src={`${grafanaHost}${path}&refresh${timeRangeParams}&var-SessionName=${sessionName}`}
-                width="450"
-                height="400"
-                frameBorder="0"
-              />
+              <Paper className={classes.chart} elevation={1} variant="outlined">
+                <iframe
+                  key={title}
+                  title={title}
+                  className={classes.grafanaEmbed}
+                  src={`${grafanaHost}${path}&refresh${timeRangeParams}&var-SessionName=${sessionName}`}
+                  frameBorder="0"
+                />
+              </Paper>
             ))}
           </div>
         </div>
diff --git a/dashboard/client/src/pages/overview/OverviewPage.tsx b/dashboard/client/src/pages/overview/OverviewPage.tsx
index e5faf12dc7e9c..91f347b627743 100644
--- a/dashboard/client/src/pages/overview/OverviewPage.tsx
+++ b/dashboard/client/src/pages/overview/OverviewPage.tsx
@@ -17,17 +17,23 @@ const useStyles = makeStyles((theme) =>
     overviewCardsContainer: {
       display: "flex",
       flexDirection: "row",
-      flexWrap: "nowrap",
+      flexWrap: "wrap",
       marginBottom: theme.spacing(4),
       gap: theme.spacing(3),
+      [theme.breakpoints.up("md")]: {
+        flexWrap: "nowrap",
+      },
     },
     overviewCard: {
       flex: "1 0 448px",
-      // Calculate max width based on 1/3 of the total width minus padding between cards
-      maxWidth: `calc((100% - ${theme.spacing(3)}px * 2) / 3)`,
+      maxWidth: "100%",
+      [theme.breakpoints.up("md")]: {
+        // Calculate max width based on 1/3 of the total width minus padding between cards
+        maxWidth: `calc((100% - ${theme.spacing(3)}px * 2) / 3)`,
+      },
     },
     section: {
-      marginTop: theme.spacing(2),
+      marginTop: theme.spacing(4),
     },
   }),
 );
@@ -59,6 +65,7 @@ export const OverviewPage = () => {
         className={classes.section}
         title="Node metrics"
         startExpanded
+        keepRendered
       >
         <Metrics />
       </CollapsibleSection>
diff --git a/dashboard/client/src/pages/overview/cards/NodeCountCard.tsx b/dashboard/client/src/pages/overview/cards/NodeCountCard.tsx
index 854d08afa3376..ab53ebc46bbff 100644
--- a/dashboard/client/src/pages/overview/cards/NodeCountCard.tsx
+++ b/dashboard/client/src/pages/overview/cards/NodeCountCard.tsx
@@ -18,6 +18,7 @@ const useStyles = makeStyles((theme) =>
       display: "flex",
       flexDirection: "row",
       flexWrap: "nowrap",
+      margin: theme.spacing(0, 3, 2),
     },
   }),
 );
diff --git a/dashboard/client/src/pages/overview/cards/OverviewCard.tsx b/dashboard/client/src/pages/overview/cards/OverviewCard.tsx
index bdec81b33c7ee..8edae6882702c 100644
--- a/dashboard/client/src/pages/overview/cards/OverviewCard.tsx
+++ b/dashboard/client/src/pages/overview/cards/OverviewCard.tsx
@@ -7,9 +7,8 @@ import { Link } from "react-router-dom";
 const useStyles = makeStyles((theme) =>
   createStyles({
     root: {
-      padding: theme.spacing(2, 3),
-      height: 270,
-      borderRadius: 8,
+      height: 294,
+      overflow: "hidden",
     },
   }),
 );
diff --git a/dashboard/client/src/pages/overview/cards/RecentJobsCard.component.test.tsx b/dashboard/client/src/pages/overview/cards/RecentJobsCard.component.test.tsx
index 5d1da1c2474d6..e1eed0a76fe3e 100644
--- a/dashboard/client/src/pages/overview/cards/RecentJobsCard.component.test.tsx
+++ b/dashboard/client/src/pages/overview/cards/RecentJobsCard.component.test.tsx
@@ -46,6 +46,6 @@ describe("RecentJobsCard", () => {
     expect(screen.getByText("02000000")).toBeVisible();
     expect(screen.getByText("raysubmit_23456")).toBeVisible();
     expect(screen.getByText("04000000")).toBeVisible();
-    expect(screen.getByText("05000000")).toBeVisible();
+    expect(screen.queryByText("05000000")).toBeNull();
   });
 });
diff --git a/dashboard/client/src/pages/overview/cards/RecentJobsCard.tsx b/dashboard/client/src/pages/overview/cards/RecentJobsCard.tsx
index 71c60735cd12b..e79526caf4386 100644
--- a/dashboard/client/src/pages/overview/cards/RecentJobsCard.tsx
+++ b/dashboard/client/src/pages/overview/cards/RecentJobsCard.tsx
@@ -17,6 +17,7 @@ const useStyles = makeStyles((theme) =>
     root: {
       display: "flex",
       flexDirection: "column",
+      padding: theme.spacing(2, 3),
     },
     listContainer: {
       marginTop: theme.spacing(2),
@@ -39,7 +40,7 @@ export const RecentJobsCard = ({ className }: RecentJobsCardProps) => {
   const classes = useStyles();
 
   const { jobList } = useJobList();
-  const sortedJobs = _.orderBy(jobList, ["startTime"], ["desc"]).slice(0, 5);
+  const sortedJobs = _.orderBy(jobList, ["startTime"], ["desc"]).slice(0, 4);
 
   return (
     <OverviewCard className={classNames(classes.root, className)}>
@@ -108,6 +109,7 @@ const useRecentJobListItemStyles = makeStyles((theme) =>
       overflow: "hidden",
       textOverflow: "ellipsis",
       whiteSpace: "nowrap",
+      color: "#5F6469",
     },
   }),
 );
@@ -148,10 +150,14 @@ const RecentJobListItem = ({ job, className }: RecentJobListItemProps) => {
       <Link className={classes.root} to={`/new/jobs/${job.job_id}`}>
         {icon}
         <div className={classes.textContainer}>
-          <Typography className={classes.title} variant="h4">
+          <Typography className={classes.title} variant="body2">
             {job.job_id ?? job.submission_id}
           </Typography>
-          <Typography className={classes.entrypoint} title={job.entrypoint}>
+          <Typography
+            className={classes.entrypoint}
+            title={job.entrypoint}
+            variant="caption"
+          >
             {job.entrypoint}
           </Typography>
         </div>
diff --git a/dashboard/client/src/theme.ts b/dashboard/client/src/theme.ts
index 2f5021a5fbc10..6259f9b65b221 100644
--- a/dashboard/client/src/theme.ts
+++ b/dashboard/client/src/theme.ts
@@ -32,6 +32,14 @@ const basicTheme: ThemeOptions = {
     body1: {
       fontSize: "0.75rem",
     },
+    body2: {
+      fontSize: "14px",
+      lineHeight: "20px",
+    },
+    caption: {
+      fontSize: "0.75rem",
+      lineHeight: "16px",
+    },
   },
   props: {
     MuiPaper: {
@@ -39,6 +47,13 @@ const basicTheme: ThemeOptions = {
     },
   },
   overrides: {
+    MuiCssBaseline: {
+      "@global": {
+        a: {
+          color: "#036DCF",
+        },
+      },
+    },
     MuiTooltip: {
       tooltip: {
         fontSize: "0.75rem",
@@ -56,6 +71,7 @@ const basicTheme: ThemeOptions = {
     MuiPaper: {
       outlined: {
         borderColor: "#D2DCE6",
+        borderRadius: 8,
       },
     },
   },
diff --git a/dashboard/modules/metrics/grafana_dashboard_base.json b/dashboard/modules/metrics/grafana_dashboard_base.json
index ac5583e8216e0..6edde3d5b8d6c 100644
--- a/dashboard/modules/metrics/grafana_dashboard_base.json
+++ b/dashboard/modules/metrics/grafana_dashboard_base.json
@@ -55,7 +55,7 @@
         "useTags": false
       },
       {
-        "allValue": null,
+        "allValue": ".+",
         "current": {
           "selected": true,
           "text": [