From da1558e7a2d2eef61c8fd79ff583bb46a66285cb Mon Sep 17 00:00:00 2001
From: Andrej Ocenas <mr.ocenas@gmail.com>
Date: Wed, 19 May 2021 19:41:54 +0200
Subject: [PATCH] Explore: UI improvements to trace view (#34276)

* Various ui fixes

* Remove unused classes

* Fix opening of collapsible

* Fix pointer issue

* Update e2e test
---
 e2e/suite1/specs/trace-view-scrolling.spec.ts |  4 +--
 .../src/components/Collapse/Collapse.tsx      | 24 ++++------------
 .../src/TracePageHeader/TracePageHeader.tsx   |  2 +-
 .../src/TraceTimelineViewer/SpanBarRow.tsx    |  1 -
 .../src/TraceTimelineViewer/SpanDetailRow.tsx |  4 +--
 .../TraceTimelineViewer/SpanTreeOffset.tsx    |  1 -
 public/app/features/explore/Explore.tsx       |  4 +--
 .../features/explore/NodeGraphContainer.tsx   | 28 +++++++++++--------
 .../explore/TraceView/TraceViewContainer.tsx  | 20 +++++++++++++
 public/app/plugins/panel/nodeGraph/Legend.tsx |  6 ++++
 .../app/plugins/panel/nodeGraph/NodeGraph.tsx |  1 +
 .../plugins/panel/nodeGraph/ViewControls.tsx  | 15 ++++++++--
 12 files changed, 68 insertions(+), 42 deletions(-)
 create mode 100644 public/app/features/explore/TraceView/TraceViewContainer.tsx

diff --git a/e2e/suite1/specs/trace-view-scrolling.spec.ts b/e2e/suite1/specs/trace-view-scrolling.spec.ts
index 4f0f7f5026b..ecabfd5ed8b 100644
--- a/e2e/suite1/specs/trace-view-scrolling.spec.ts
+++ b/e2e/suite1/specs/trace-view-scrolling.spec.ts
@@ -29,7 +29,7 @@ describe('Trace view', () => {
 
     e2e.pages.Explore.General.scrollBar().scrollTo('center');
 
-    // After scrolling we should have 140 spans instead of the first 100
-    e2e.components.TraceViewer.spanBar().should('have.length', 140);
+    // After scrolling we should have 139 spans instead of the first 100
+    e2e.components.TraceViewer.spanBar().should('have.length', 139);
   });
 });
diff --git a/packages/grafana-ui/src/components/Collapse/Collapse.tsx b/packages/grafana-ui/src/components/Collapse/Collapse.tsx
index 727270b419e..aa4554984c7 100644
--- a/packages/grafana-ui/src/components/Collapse/Collapse.tsx
+++ b/packages/grafana-ui/src/components/Collapse/Collapse.tsx
@@ -13,6 +13,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
   collapseBody: css`
     label: collapse__body;
     padding: ${theme.spacing(theme.components.panel.padding)};
+    padding-top: 0;
     flex: 1;
     overflow: hidden;
     display: flex;
@@ -59,7 +60,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
   `,
   header: css`
     label: collapse__header;
-    padding: ${theme.spacing(1, 2)};
+    padding: ${theme.spacing(1, 2, 0.5, 2)};
     display: flex;
     cursor: inherit;
     transition: all 0.1s linear;
@@ -67,20 +68,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
   `,
   headerCollapsed: css`
     label: collapse__header--collapsed;
-    cursor: pointer;
-    padding: ${theme.spacing(1, 2)};
-  `,
-  headerButtons: css`
-    label: collapse__header-buttons;
-    margin-right: ${theme.spacing(1)};
-    margin-top: ${theme.spacing(0.25)};
-    font-size: ${theme.typography.size.lg};
-    line-height: ${theme.typography.h6.lineHeight};
-    display: inherit;
-  `,
-  headerButtonsCollapsed: css`
-    label: collapse__header-buttons--collapsed;
-    display: none;
+    padding: ${theme.spacing(1, 2, 0.5, 2)};
   `,
   headerLabel: css`
     label: collapse__header-label;
@@ -110,6 +98,7 @@ export const ControlledCollapse: FunctionComponent<Props> = ({ isOpen, onToggle,
   return (
     <Collapse
       isOpen={open}
+      collapsible
       {...otherProps}
       onToggle={() => {
         setOpen(!open);
@@ -140,14 +129,11 @@ export const Collapse: FunctionComponent<Props> = ({
   const panelClass = cx([style.collapse, 'panel-container', className]);
   const loaderClass = loading ? cx([style.loader, style.loaderActive]) : cx([style.loader]);
   const headerClass = collapsible ? cx([style.header]) : cx([style.headerCollapsed]);
-  const headerButtonsClass = collapsible ? cx([style.headerButtons]) : cx([style.headerButtonsCollapsed]);
 
   return (
     <div className={panelClass}>
       <div className={headerClass} onClick={onClickToggle}>
-        <div className={headerButtonsClass}>
-          <Icon name={isOpen ? 'angle-up' : 'angle-down'} />
-        </div>
+        {collapsible && <Icon name={isOpen ? 'angle-up' : 'angle-down'} />}
         <div className={cx([style.headerLabel])}>{label}</div>
       </div>
       {isOpen && (
diff --git a/packages/jaeger-ui-components/src/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui-components/src/TracePageHeader/TracePageHeader.tsx
index 38009c59791..224d13d6f88 100644
--- a/packages/jaeger-ui-components/src/TracePageHeader/TracePageHeader.tsx
+++ b/packages/jaeger-ui-components/src/TracePageHeader/TracePageHeader.tsx
@@ -100,7 +100,7 @@ const getStyles = createStyle((theme: Theme) => {
       font-size: 1.7em;
       line-height: 1em;
       margin: 0 0 0 0.5em;
-      padding: 0.5em 0;
+      padding-bottom: 0.5em;
     `,
     TracePageHeaderTitleCollapsible: css`
       label: TracePageHeaderTitleCollapsible;
diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.tsx
index 3950da242e9..ed18d978cb9 100644
--- a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.tsx
+++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.tsx
@@ -42,7 +42,6 @@ const getStyles = createStyle((theme: Theme) => {
   return {
     nameWrapper: css`
       label: nameWrapper;
-      background: ${autoColor(theme, '#f8f8f8')};
       line-height: 27px;
       overflow: hidden;
       display: flex;
diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetailRow.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetailRow.tsx
index f541f1ac4f8..661e1fc988d 100644
--- a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetailRow.tsx
+++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetailRow.tsx
@@ -58,7 +58,7 @@ const getStyles = createStyle((theme: Theme) => {
       }
     `,
     infoWrapper: css`
-      background: ${autoColor(theme, '#f5f5f5')};
+      label: infoWrapper;
       border: 1px solid ${autoColor(theme, '#d3d3d3')};
       border-top: 3px solid;
       padding: 0.75rem;
@@ -123,7 +123,7 @@ export class UnthemedSpanDetailRow extends React.PureComponent<SpanDetailRowProp
     const styles = getStyles(theme);
     return (
       <TimelineRow>
-        <TimelineRow.Cell width={columnDivision}>
+        <TimelineRow.Cell width={columnDivision} style={{ overflow: 'hidden' }}>
           <SpanTreeOffset
             span={span}
             showChildrenIcon={false}
diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanTreeOffset.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanTreeOffset.tsx
index 5d80492cedc..313a4640820 100644
--- a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanTreeOffset.tsx
+++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanTreeOffset.tsx
@@ -34,7 +34,6 @@ export const getStyles = createStyle((theme: Theme) => {
     SpanTreeOffsetParent: css`
       label: SpanTreeOffsetParent;
       &:hover {
-        background-color: ${autoColor(theme, '#e8e8e8')};
         cursor: pointer;
       }
     `,
diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx
index d5f8bed7498..7c49109f840 100644
--- a/public/app/features/explore/Explore.tsx
+++ b/public/app/features/explore/Explore.tsx
@@ -34,12 +34,12 @@ import { StoreState } from 'app/types';
 import { ExploreToolbar } from './ExploreToolbar';
 import { NoDataSourceCallToAction } from './NoDataSourceCallToAction';
 import { getTimeZone } from '../profile/state/selectors';
-import { TraceView } from './TraceView/TraceView';
 import { SecondaryActions } from './SecondaryActions';
 import { FILTER_FOR_OPERATOR, FILTER_OUT_OPERATOR, FilterItem } from '@grafana/ui/src/components/Table/types';
 import { ExploreGraphNGPanel } from './ExploreGraphNGPanel';
 import { NodeGraphContainer } from './NodeGraphContainer';
 import { ResponseErrorContainer } from './ResponseErrorContainer';
+import { TraceViewContainer } from './TraceView/TraceViewContainer';
 
 const getStyles = stylesFactory((theme: GrafanaTheme) => {
   return {
@@ -290,7 +290,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
     return (
       // If there is no data (like 404) we show a separate error so no need to show anything here
-      dataFrames.length && <TraceView exploreId={exploreId} dataFrames={dataFrames} splitOpenFn={splitOpen} />
+      dataFrames.length && <TraceViewContainer exploreId={exploreId} dataFrames={dataFrames} splitOpenFn={splitOpen} />
     );
   }
 
diff --git a/public/app/features/explore/NodeGraphContainer.tsx b/public/app/features/explore/NodeGraphContainer.tsx
index 07a295b706f..a15d27e8213 100644
--- a/public/app/features/explore/NodeGraphContainer.tsx
+++ b/public/app/features/explore/NodeGraphContainer.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useState } from 'react';
 import { Badge, Collapse } from '@grafana/ui';
 import { DataFrame, TimeRange } from '@grafana/data';
 import { ExploreId, StoreState } from '../../types';
@@ -19,19 +19,23 @@ export function UnconnectedNodeGraphContainer(props: Props & ConnectedProps<type
   const { dataFrames, range, splitOpen, short } = props;
   const getLinks = useLinks(range, splitOpen);
 
+  const [open, setOpen] = useState(true);
+
   return (
-    <div style={{ height: short ? 300 : 600 }}>
-      <Collapse
-        label={
-          <span>
-            Node graph <Badge text={'Beta'} color={'blue'} icon={'rocket'} tooltip={'This visualization is in beta'} />
-          </span>
-        }
-        isOpen
-      >
+    <Collapse
+      label={
+        <span>
+          Node graph <Badge text={'Beta'} color={'blue'} icon={'rocket'} tooltip={'This visualization is in beta'} />
+        </span>
+      }
+      collapsible={short}
+      isOpen={short ? open : true}
+      onToggle={short ? () => setOpen(!open) : undefined}
+    >
+      <div style={{ height: short ? 300 : 600 }}>
         <NodeGraph dataFrames={dataFrames} getLinks={getLinks} />
-      </Collapse>
-    </div>
+      </div>
+    </Collapse>
   );
 }
 
diff --git a/public/app/features/explore/TraceView/TraceViewContainer.tsx b/public/app/features/explore/TraceView/TraceViewContainer.tsx
new file mode 100644
index 00000000000..d65cc042a71
--- /dev/null
+++ b/public/app/features/explore/TraceView/TraceViewContainer.tsx
@@ -0,0 +1,20 @@
+import React from 'react';
+import { Collapse } from '@grafana/ui';
+import { DataFrame } from '@grafana/data';
+import { TraceView } from './TraceView';
+import { ExploreId, SplitOpen } from '../../../types';
+
+interface Props {
+  dataFrames: DataFrame[];
+  splitOpenFn: SplitOpen;
+  exploreId: ExploreId;
+}
+export function TraceViewContainer(props: Props) {
+  const { dataFrames, splitOpenFn, exploreId } = props;
+
+  return (
+    <Collapse label="Trace View" isOpen>
+      <TraceView exploreId={exploreId} dataFrames={dataFrames} splitOpenFn={splitOpenFn} />
+    </Collapse>
+  );
+}
diff --git a/public/app/plugins/panel/nodeGraph/Legend.tsx b/public/app/plugins/panel/nodeGraph/Legend.tsx
index 37f435a8d17..0294e8d141f 100644
--- a/public/app/plugins/panel/nodeGraph/Legend.tsx
+++ b/public/app/plugins/panel/nodeGraph/Legend.tsx
@@ -12,6 +12,11 @@ function getStyles() {
       label: LegendItem;
       flex-grow: 0;
     `,
+
+    legend: css`
+      label: Legend;
+      pointer-events: all;
+    `,
   };
 }
 
@@ -41,6 +46,7 @@ export const Legend = function Legend(props: Props) {
 
   return (
     <VizLegend<ItemData>
+      className={styles.legend}
       displayMode={LegendDisplayMode.List}
       placement={'bottom'}
       items={colorItems}
diff --git a/public/app/plugins/panel/nodeGraph/NodeGraph.tsx b/public/app/plugins/panel/nodeGraph/NodeGraph.tsx
index 57a43d8857e..dd6a5c361a4 100644
--- a/public/app/plugins/panel/nodeGraph/NodeGraph.tsx
+++ b/public/app/plugins/panel/nodeGraph/NodeGraph.tsx
@@ -58,6 +58,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
     display: flex;
     align-items: flex-end;
     justify-content: space-between;
+    pointer-events: none;
   `,
   legend: css`
     label: legend;
diff --git a/public/app/plugins/panel/nodeGraph/ViewControls.tsx b/public/app/plugins/panel/nodeGraph/ViewControls.tsx
index 4654186a4ab..abcd9484304 100644
--- a/public/app/plugins/panel/nodeGraph/ViewControls.tsx
+++ b/public/app/plugins/panel/nodeGraph/ViewControls.tsx
@@ -1,5 +1,15 @@
 import React, { useState } from 'react';
-import { Button, HorizontalGroup, VerticalGroup } from '@grafana/ui';
+import { Button, HorizontalGroup, useStyles, VerticalGroup } from '@grafana/ui';
+import { css } from '@emotion/css';
+
+function getStyles() {
+  return {
+    wrapper: css`
+      label: wrapper;
+      pointer-events: all;
+    `,
+  };
+}
 
 interface Props<Config> {
   config: Config;
@@ -20,9 +30,10 @@ export function ViewControls<Config extends Record<string, any>>(props: Props<Co
 
   // For debugging the layout, should be removed here and maybe moved to panel config later on
   const allowConfiguration = false;
+  const styles = useStyles(getStyles);
 
   return (
-    <div>
+    <div className={styles.wrapper}>
       <VerticalGroup spacing="sm">
         <HorizontalGroup spacing="xs">
           <Button