From d0071a97fd056041f8e16e964241e46bd7d9e015 Mon Sep 17 00:00:00 2001
From: tacigar <ig.lhound@gmail.com>
Date: Fri, 23 Dec 2022 16:49:33 +0900
Subject: [PATCH] Adjust styling of Dependencies page

---
 .../DependenciesPage/DependenciesGraph.tsx    | 169 ++++++++----------
 .../DependenciesPage/DependenciesPage.tsx     |  77 ++++----
 .../DependenciesPage/NodeDetailData.tsx       | 163 ++++++-----------
 .../DependenciesPage/{Edge.ts => types.ts}    |   6 +-
 .../DiscoverPage/DiscoverPageContent.tsx      |  20 ++-
 .../src/components/common/ExplainBox.tsx      |   8 +-
 6 files changed, 178 insertions(+), 265 deletions(-)
 rename zipkin-lens/src/components/DependenciesPage/{Edge.ts => types.ts} (88%)

diff --git a/zipkin-lens/src/components/DependenciesPage/DependenciesGraph.tsx b/zipkin-lens/src/components/DependenciesPage/DependenciesGraph.tsx
index f91d27f937a..29cc1ae3dc2 100644
--- a/zipkin-lens/src/components/DependenciesPage/DependenciesGraph.tsx
+++ b/zipkin-lens/src/components/DependenciesPage/DependenciesGraph.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2020 The OpenZipkin Authors
+ * Copyright 2015-2022 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -16,7 +16,6 @@
 
 import {
   Box,
-  Grid,
   Theme,
   createStyles,
   makeStyles,
@@ -25,12 +24,10 @@ import {
 import moment from 'moment';
 import React, { CSSProperties, useState, useCallback, useMemo } from 'react';
 import ReactSelect, { ValueType, ActionMeta } from 'react-select';
-import { AutoSizer } from 'react-virtualized';
-
-import Edge from './Edge';
+import Dependencies from '../../models/Dependencies';
 import NodeDetailData from './NodeDetailData';
+import { Edge } from './types';
 import VizceralWrapper from './VizceralWrapper';
-import Dependencies from '../../models/Dependencies';
 
 // These filter functions use any type because they are passed directly to untyped JS code.
 const filterConnections = (object: any, value: any) => {
@@ -108,22 +105,10 @@ const reactSelectStyles = {
 
 const useStyles = makeStyles((theme: Theme) =>
   createStyles({
-    containerGrid: {
-      height: '100%',
-    },
-    graphItemGrid: {
-      height: '100%',
-    },
-    nodeDetailDataItemGrid: {
-      height: '100%',
-      boxShadow: theme.shadows[10],
-      zIndex: 1,
-    },
-    vizceralWrapper: {
-      // In Firefox, 100% height does not work...
-      '& .vizceral': {
-        height: '99%',
-      },
+    detailWrapper: {
+      flex: '0 0 420px',
+      width: 420,
+      borderLeft: `1px solid ${theme.palette.divider}`,
     },
   }),
 );
@@ -238,82 +223,70 @@ const DependenciesGraph: React.FC<DependenciesGraphProps> = ({
   );
 
   return (
-    <Box width="100%" height="100%" data-testid="dependencies-graph">
-      <Grid container className={classes.containerGrid}>
-        <Grid
-          item
-          xs={focusedNodeName ? 7 : 12}
-          className={classes.graphItemGrid}
-        >
-          <AutoSizer>
-            {({ width, height }) => (
-              <Box
-                width={width}
-                height={height}
-                position="relative"
-                className={classes.vizceralWrapper}
-              >
-                <VizceralWrapper
-                  allowDraggingOfNodes
-                  targetFramerate={30}
-                  traffic={{
-                    renderer: 'region',
-                    layout: 'ltrTree',
-                    name: 'dependencies-graph',
-                    maxVolume: maxVolume * 50,
-                    nodes,
-                    connections: edges,
-                    updated: createdTs,
-                  }}
-                  objectHighlighted={handleObjectHighlight}
-                  styles={vizStyle}
-                  key={
-                    // Normally, when updating filters, Vizsceral will show and hide nodes without relaying
-                    // them out. For a large dependency graph, this often won't let us see well the
-                    // information about the filtered nodes, so we prefer to force a relayout any time we
-                    // update the filter. Changing the key based on the filter like this causes react to
-                    // destroy and reconstruct the component from scratch, which will have a layout zooming
-                    // in on the filtered nodes.
-                    filter
-                  }
-                  filters={[
-                    {
-                      name: 'shownConnections',
-                      type: 'connection',
-                      passes: filterConnections,
-                      value: filter,
-                    },
-                    {
-                      name: 'shownNodes',
-                      type: 'node',
-                      passes: filterNodes,
-                      value: filter,
-                    },
-                  ]}
-                />
-              </Box>
-            )}
-          </AutoSizer>
-          <Box position="absolute" left={30} top={30}>
-            <ReactSelect
-              isClearable
-              options={filterOptions}
-              onChange={handleFilterChange}
-              value={!filter ? undefined : { value: filter, label: filter }}
-              styles={reactSelectStyles}
-            />
-          </Box>
-        </Grid>
-        {focusedNodeName ? (
-          <Grid item xs={5} className={classes.nodeDetailDataItemGrid}>
-            <NodeDetailData
-              serviceName={focusedNodeName}
-              targetEdges={targetEdges}
-              sourceEdges={sourceEdges}
-            />
-          </Grid>
-        ) : null}
-      </Grid>
+    <Box
+      width="100%"
+      height="100%"
+      data-testid="dependencies-graph"
+      display="flex"
+    >
+      <Box flex="1 1" position="relative">
+        <VizceralWrapper
+          allowDraggingOfNodes
+          targetFramerate={30}
+          traffic={{
+            renderer: 'region',
+            layout: 'ltrTree',
+            name: 'dependencies-graph',
+            maxVolume: maxVolume * 50,
+            nodes,
+            connections: edges,
+            updated: createdTs,
+          }}
+          objectHighlighted={handleObjectHighlight}
+          styles={vizStyle}
+          key={
+            // Normally, when updating filters, Vizsceral will show and hide nodes without relaying
+            // them out. For a large dependency graph, this often won't let us see well the
+            // information about the filtered nodes, so we prefer to force a relayout any time we
+            // update the filter. Changing the key based on the filter like this causes react to
+            // destroy and reconstruct the component from scratch, which will have a layout zooming
+            // in on the filtered nodes.
+            filter
+          }
+          filters={[
+            {
+              name: 'shownConnections',
+              type: 'connection',
+              passes: filterConnections,
+              value: filter,
+            },
+            {
+              name: 'shownNodes',
+              type: 'node',
+              passes: filterNodes,
+              value: filter,
+            },
+          ]}
+        />
+        <Box position="absolute" left={20} top={20}>
+          <ReactSelect
+            isClearable
+            options={filterOptions}
+            onChange={handleFilterChange}
+            value={!filter ? undefined : { value: filter, label: filter }}
+            styles={reactSelectStyles}
+          />
+        </Box>
+      </Box>
+      {focusedNodeName ? (
+        <Box className={classes.detailWrapper}>
+          <NodeDetailData
+            serviceName={focusedNodeName}
+            targetEdges={targetEdges}
+            sourceEdges={sourceEdges}
+          />
+        </Box>
+      ) : null}
     </Box>
   );
 };
diff --git a/zipkin-lens/src/components/DependenciesPage/DependenciesPage.tsx b/zipkin-lens/src/components/DependenciesPage/DependenciesPage.tsx
index 3384f61371f..885933a3e3f 100644
--- a/zipkin-lens/src/components/DependenciesPage/DependenciesPage.tsx
+++ b/zipkin-lens/src/components/DependenciesPage/DependenciesPage.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2020 The OpenZipkin Authors
+ * Copyright 2015-2022 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -12,7 +12,7 @@
  * the License.
  */
 
-import { faSearch, faProjectDiagram } from '@fortawesome/free-solid-svg-icons';
+import { faProjectDiagram, faSync } from '@fortawesome/free-solid-svg-icons';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { t, Trans } from '@lingui/macro';
 import { useLingui } from '@lingui/react';
@@ -30,35 +30,28 @@ import moment from 'moment';
 import React, { useEffect, useState, useCallback, useMemo } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { withRouter, RouteComponentProps } from 'react-router-dom';
-
-import DependenciesGraph from './DependenciesGraph';
-import { clearAlert, setAlert } from '../App/slice';
-import ExplainBox from '../common/ExplainBox';
 import {
   clearDependencies,
   loadDependencies,
 } from '../../slices/dependenciesSlice';
 import { RootState } from '../../store';
+import { clearAlert, setAlert } from '../App/slice';
+import ExplainBox from '../common/ExplainBox';
 import { LoadingIndicator } from '../common/LoadingIndicator';
+import DependenciesGraph from './DependenciesGraph';
 
 const useStyles = makeStyles((theme: Theme) =>
   createStyles({
+    searchWrapper: {
+      backgroundColor: theme.palette.background.paper,
+      padding: theme.spacing(2),
+      flex: '0 0',
+      borderBottom: `1px solid ${theme.palette.divider}`,
+    },
     dateTimePicker: {
       marginLeft: theme.spacing(1),
       marginRight: theme.spacing(1),
     },
-    dateTimePickerInput: {
-      fontSize: '1rem',
-      height: '1.8rem',
-      padding: `${theme.spacing(0.4)}px ${theme.spacing(0.6)}px`,
-    },
-    searchButton: {
-      fontSize: '1.2rem',
-      padding: theme.spacing(1),
-      minWidth: 0,
-      width: 32,
-      height: 32,
-    },
   }),
 );
 
@@ -209,39 +202,41 @@ const DependenciesPageImpl: React.FC<DependenciesPageProps> = ({
       display="flex"
       flexDirection="column"
     >
-      <Box bgcolor="background.paper" boxShadow={3} p={3}>
+      <Box className={classes.searchWrapper}>
         <Box display="flex" justifyContent="center" alignItems="center">
-          <KeyboardDateTimePicker
-            label={i18n._(t`Start Time`)}
-            inputVariant="outlined"
-            value={tempTimeRange.startTime}
-            onChange={handleStartTimeChange}
-            format="MM/DD/YYYY HH:mm:ss"
-            className={classes.dateTimePicker}
-            InputProps={{ classes: { input: classes.dateTimePickerInput } }}
-          />
-          -
-          <KeyboardDateTimePicker
-            label={i18n._(t`End Time`)}
-            inputVariant="outlined"
-            value={tempTimeRange.endTime}
-            onChange={handleEndTimeChange}
-            format="MM/DD/YYYY HH:mm:ss"
-            className={classes.dateTimePicker}
-            InputProps={{ classes: { input: classes.dateTimePickerInput } }}
-          />
+          <Box display="flex" mr={0.5} alignItems="center">
+            <KeyboardDateTimePicker
+              label={i18n._(t`Start Time`)}
+              inputVariant="outlined"
+              value={tempTimeRange.startTime}
+              onChange={handleStartTimeChange}
+              format="MM/DD/YYYY HH:mm:ss"
+              className={classes.dateTimePicker}
+              size="small"
+            />
+            -
+            <KeyboardDateTimePicker
+              label={i18n._(t`End Time`)}
+              inputVariant="outlined"
+              value={tempTimeRange.endTime}
+              onChange={handleEndTimeChange}
+              format="MM/DD/YYYY HH:mm:ss"
+              className={classes.dateTimePicker}
+              size="small"
+            />
+          </Box>
           <Button
             color="primary"
             variant="contained"
             onClick={handleSearchButtonClick}
-            className={classes.searchButton}
             data-testid="search-button"
+            startIcon={<FontAwesomeIcon icon={faSync} />}
           >
-            <FontAwesomeIcon icon={faSearch} />
+            <Trans>Run Query</Trans>
           </Button>
         </Box>
       </Box>
-      <Box m={4} flexGrow={1}>
+      <Box flex="1 1" bgcolor="background.paper" overflow="hidden">
         {content}
       </Box>
     </Box>
diff --git a/zipkin-lens/src/components/DependenciesPage/NodeDetailData.tsx b/zipkin-lens/src/components/DependenciesPage/NodeDetailData.tsx
index 03d18a74fc4..2ff44486bbd 100644
--- a/zipkin-lens/src/components/DependenciesPage/NodeDetailData.tsx
+++ b/zipkin-lens/src/components/DependenciesPage/NodeDetailData.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2020 The OpenZipkin Authors
+ * Copyright 2015-2022 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -17,11 +17,9 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import {
   Box,
   Button,
-  Paper,
   Table,
   TableBody,
   TableCell,
-  TableHead,
   TableRow,
   Theme,
   Typography,
@@ -30,41 +28,36 @@ import {
 } from '@material-ui/core';
 import React, { useCallback } from 'react';
 import { withRouter, RouteComponentProps } from 'react-router-dom';
-import { PieChart, Pie, Cell, ResponsiveContainer } from 'recharts';
-
-import Edge from './Edge';
+import { PieChart, Pie, Cell, ResponsiveContainer, Tooltip } from 'recharts';
+import { Edge } from './types';
 import { selectServiceColor } from '../../constants/color';
 
 const useStyles = makeStyles((theme: Theme) =>
   createStyles({
     title: {
-      overflowWrap: 'break-word',
-      flexGrow: 1,
-      minWidth: 10, // break-word in flex-box
-    },
-    searchTracesButton: {
-      display: 'flex',
-    },
-    noDataMessage: {
-      opacity: 0.8,
+      wordWrap: 'break-word',
+      flex: '1 1',
+      minWidth: 0,
     },
-    tablePaper: {
-      marginRight: theme.spacing(2),
-      marginLeft: theme.spacing(2),
-      maxHeight: 180,
-      overflowY: 'auto',
-      borderRadius: 3,
+    table: {
+      tableLayout: 'fixed',
     },
-    tableHeadCell: {
-      top: 0,
-      position: 'sticky',
-      padding: `${theme.spacing(1)}px ${theme.spacing(2)}px`,
-      backgroundColor: theme.palette.grey[200],
-      zIndex: 1,
+    tableRow: {
+      '&:first-child > *': {
+        borderTop: `1px solid ${theme.palette.divider}`,
+      },
     },
     tableCell: {
-      wordBreak: 'break-all',
-      padding: `${theme.spacing(1)}px ${theme.spacing(2)}px`,
+      wordWrap: 'break-word',
+    },
+    contentWrapper: {
+      flex: '1 1',
+      borderTop: `1px solid ${theme.palette.divider}`,
+      overflow: 'auto',
+      '& > :not(:last-child)': {
+        marginBottom: theme.spacing(2),
+      },
+      paddingBottom: theme.spacing(2),
     },
   }),
 );
@@ -121,69 +114,34 @@ const NodeDetailDataImpl: React.FC<NodeDetailDataProps> = ({
 
   return (
     <Box height="100%" display="flex" flexDirection="column">
-      <Box
-        pt={2}
-        pb={1}
-        pr={2}
-        pl={2}
-        bgcolor="grey.200"
-        color="text.secondary"
-        display="flex"
-        justifyContent="space-between"
-      >
-        <Box display="flex" alignItems="center">
-          <Box mr={0.75}>
-            <FontAwesomeIcon
-              icon={faSquare}
-              size="lg"
-              color={selectServiceColor(serviceName)}
-            />
-          </Box>
-          <Typography variant="h5" className={classes.title}>
-            {serviceName}
-          </Typography>
-        </Box>
-        <Button
-          variant="outlined"
-          onClick={handleSearchTracesButtonClick}
-          data-testid="search-traces-button"
-        >
-          <FontAwesomeIcon icon={faSearch} />
-          <Box component="span" ml={0.75}>
+      <Box px={2} py={1} display="flex" flex="0 0" alignItems="center">
+        <Typography variant="h6" className={classes.title}>
+          {serviceName}
+        </Typography>
+        <Box flex="0 0" ml={0.5}>
+          <Button
+            variant="outlined"
+            onClick={handleSearchTracesButtonClick}
+            data-testid="search-traces-button"
+            startIcon={<FontAwesomeIcon icon={faSearch} />}
+          >
             Traces
-          </Box>
-        </Button>
+          </Button>
+        </Box>
       </Box>
-      <Box
-        height={100}
-        flexGrow={1}
-        bgcolor="background.paper"
-        borderColor="divider"
-        borderTop={1}
-        paddingBottom={2}
-        overflow="auto"
-      >
+      <Box className={classes.contentWrapper}>
         {shownDataList.map((d) => (
-          <Box display="flex" flexDirection="column">
-            <Box height={180} position="relative">
-              <Box
-                position="absolute"
-                top={15}
-                left={15}
-                display="flex"
-                alignItems="center"
-              >
-                <Box color="text.secondary">
-                  <Typography variant="h6">{d.title}</Typography>
-                </Box>
-                <Box color="text.hint" ml={1}>
-                  (traced requests)
-                </Box>
+          <Box key={d.title}>
+            <Box height={240} position="relative">
+              <Box position="absolute" top={15} left={15}>
+                <Typography variant="body1">
+                  {d.title} (traced requests)
+                </Typography>
               </Box>
-              <Box position="absolute" bottom={10} right={15}>
-                <Box color="text.hint" ml={1}>
+              <Box position="absolute" bottom={15} right={15}>
+                <Typography variant="body2" color="textSecondary">
                   {d.formatter(d.edges.length)}
-                </Box>
+                </Typography>
               </Box>
               <ResponsiveContainer>
                 <PieChart>
@@ -194,7 +152,7 @@ const NodeDetailDataImpl: React.FC<NodeDetailDataProps> = ({
                     }))}
                     nameKey="name"
                     dataKey="value"
-                    outerRadius={60}
+                    outerRadius={70}
                   >
                     {d.edges.map((edge) => (
                       <Cell
@@ -203,27 +161,18 @@ const NodeDetailDataImpl: React.FC<NodeDetailDataProps> = ({
                       />
                     ))}
                   </Pie>
+                  <Tooltip />
                 </PieChart>
               </ResponsiveContainer>
             </Box>
-            <Paper className={classes.tablePaper}>
-              <Table>
-                <TableHead>
-                  <TableRow>
-                    <TableCell className={classes.tableHeadCell}>
-                      Service Name
-                    </TableCell>
-                    <TableCell className={classes.tableHeadCell}>
-                      Call
-                    </TableCell>
-                    <TableCell className={classes.tableHeadCell}>
-                      Error
-                    </TableCell>
-                  </TableRow>
-                </TableHead>
+            <Box px={2}>
+              <Table size="small" className={classes.table}>
                 <TableBody>
                   {d.edges.map((edge) => (
-                    <TableRow>
+                    <TableRow
+                      key={`${edge.source}---${edge.target}`}
+                      className={classes.tableRow}
+                    >
                       <TableCell className={classes.tableCell}>
                         <FontAwesomeIcon
                           icon={faSquare}
@@ -233,17 +182,17 @@ const NodeDetailDataImpl: React.FC<NodeDetailDataProps> = ({
                           {d.selectNodeName(edge)}
                         </Box>
                       </TableCell>
-                      <TableCell className={classes.tableCell}>
+                      <TableCell className={classes.tableCell} align="right">
                         {edge.metrics.normal}
                       </TableCell>
-                      <TableCell className={classes.tableCell}>
+                      <TableCell className={classes.tableCell} align="right">
                         {edge.metrics.danger}
                       </TableCell>
                     </TableRow>
                   ))}
                 </TableBody>
               </Table>
-            </Paper>
+            </Box>
           </Box>
         ))}
       </Box>
diff --git a/zipkin-lens/src/components/DependenciesPage/Edge.ts b/zipkin-lens/src/components/DependenciesPage/types.ts
similarity index 88%
rename from zipkin-lens/src/components/DependenciesPage/Edge.ts
rename to zipkin-lens/src/components/DependenciesPage/types.ts
index dabb7f03aed..b8349bb3572 100644
--- a/zipkin-lens/src/components/DependenciesPage/Edge.ts
+++ b/zipkin-lens/src/components/DependenciesPage/types.ts
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2020 The OpenZipkin Authors
+ * Copyright 2015-2022 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -12,7 +12,7 @@
  * the License.
  */
 
-type Edge = {
+export type Edge = {
   source: string;
   target: string;
   metrics: {
@@ -20,5 +20,3 @@ type Edge = {
     danger: number;
   };
 };
-
-export default Edge;
diff --git a/zipkin-lens/src/components/DiscoverPage/DiscoverPageContent.tsx b/zipkin-lens/src/components/DiscoverPage/DiscoverPageContent.tsx
index 84d8198edbe..6e08f996076 100644
--- a/zipkin-lens/src/components/DiscoverPage/DiscoverPageContent.tsx
+++ b/zipkin-lens/src/components/DiscoverPage/DiscoverPageContent.tsx
@@ -559,14 +559,16 @@ const DiscoverPageContent: React.FC<DiscoverPageContentProps> = ({
     );
   } else {
     content = (
-      <Paper elevation={3}>
-        <TraceSummaryTable
-          traceSummaries={filteredTraceSummaries}
-          toggleFilter={toggleFilter}
-          traceSummaryOpenMap={traceSummaryOpenMap}
-          toggleTraceSummaryOpen={toggleTraceSummaryOpen}
-        />
-      </Paper>
+      <Container>
+        <Paper elevation={3}>
+          <TraceSummaryTable
+            traceSummaries={filteredTraceSummaries}
+            toggleFilter={toggleFilter}
+            traceSummaryOpenMap={traceSummaryOpenMap}
+            toggleTraceSummaryOpen={toggleTraceSummaryOpen}
+          />
+        </Paper>
+      </Container>
     );
   }
 
@@ -694,7 +696,7 @@ const DiscoverPageContent: React.FC<DiscoverPageContentProps> = ({
         )}
       </Box>
       <Box flexGrow={1} overflow="auto" pt={3} pb={3}>
-        <Container>{content}</Container>
+        {content}
       </Box>
     </Box>
   );
diff --git a/zipkin-lens/src/components/common/ExplainBox.tsx b/zipkin-lens/src/components/common/ExplainBox.tsx
index 45b251fbcaf..818772f7162 100644
--- a/zipkin-lens/src/components/common/ExplainBox.tsx
+++ b/zipkin-lens/src/components/common/ExplainBox.tsx
@@ -26,17 +26,13 @@ interface ExplainBoxProps {
 const ExplainBox = React.memo<ExplainBoxProps>(({ icon, headerText, text }) => {
   return (
     <Box
-      top={64}
-      left={0}
-      right={0}
-      bottom={0}
-      position="fixed"
+      height="100%"
+      width="100%"
       display="flex"
       alignItems="center"
       justifyContent="center"
       flexDirection="column"
       color="text.secondary"
-      zIndex={-1}
     >
       <FontAwesomeIcon icon={icon} size="10x" />
       <Box mt={3} mb={2}>