review included

jaegertracing · Feb 26, 2020 · ec89569 · ec89569
1 parent fb08bd7
commit ec89569
Show file tree

Hide file tree

Showing 12 changed files with 93 additions and 111 deletions.
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/AltViewOptions.test.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/AltViewOptions.test.js
@@ -14,10 +14,11 @@
 
 import * as React from 'react';
 import { shallow } from 'enzyme';
-import { Button, Dropdown } from 'antd';
+import { Dropdown } from 'antd';
 import { Link } from 'react-router-dom';
 import AltViewOptions from './AltViewOptions';
 import * as track from './TracePageHeader.track';
+import { ETraceViewType } from '../types';
 
 describe('AltViewOptions', () => {
   let trackGanttView;
@@ -43,9 +44,9 @@ describe('AltViewOptions', () => {
   };
 
   const props = {
-    selectedTraceView: 0,
+    viewType: ETraceViewType.TraceTimelineViewer,
     traceID: 'test trace ID',
-    onTraceGraphViewClicked: jest.fn(),
+    onTraceViewChange: jest.fn(),
   };
 
   beforeAll(() => {
@@ -81,50 +82,30 @@ describe('AltViewOptions', () => {
 
   it('track dropdown menu', () => {
     expect(trackGraphView).not.toHaveBeenCalled();
-    expect(props.onTraceGraphViewClicked).not.toHaveBeenCalled();
+    expect(props.onTraceViewChange).not.toHaveBeenCalled();
     getLink('Trace Graph').simulate('click');
     expect(trackGraphView).toHaveBeenCalledTimes(1);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(1);
+    expect(props.onTraceViewChange).toHaveBeenCalledTimes(1);
     expect(trackStatisticsView).not.toHaveBeenCalled();
     getLink('Trace Statistics').simulate('click');
     expect(trackStatisticsView).toHaveBeenCalledTimes(1);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(2);
+    expect(props.onTraceViewChange).toHaveBeenCalledTimes(2);
 
-    wrapper.setProps({ selectedTraceView: 1 });
+    wrapper.setProps({ viewType: ETraceViewType.TraceGraph });
     expect(trackGanttView).not.toHaveBeenCalled();
     getLink('Trace Timeline').simulate('click');
     expect(trackGanttView).toHaveBeenCalledTimes(1);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(3);
+    expect(props.onTraceViewChange).toHaveBeenCalledTimes(3);
     getLink('Trace Statistics').simulate('click');
     expect(trackStatisticsView).toHaveBeenCalledTimes(2);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(4);
+    expect(props.onTraceViewChange).toHaveBeenCalledTimes(4);
 
-    wrapper.setProps({ selectedTraceView: 2 });
+    wrapper.setProps({ viewType: 2 });
     getLink('Trace Timeline').simulate('click');
     expect(trackGanttView).toHaveBeenCalledTimes(2);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(5);
+    expect(props.onTraceViewChange).toHaveBeenCalledTimes(5);
     getLink('Trace Graph').simulate('click');
     expect(trackGraphView).toHaveBeenCalledTimes(2);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(6);
-  });
-
-  it('toggles and track toggle', () => {
-    wrapper.setProps({ selectedTraceView: 0 });
-    expect(trackGraphView).not.toHaveBeenCalled();
-    wrapper.find(Button).simulate('click');
-    expect(trackGraphView).toHaveBeenCalledTimes(1);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(1);
-
-    wrapper.setProps({ selectedTraceView: 1 });
-    expect(trackStatisticsView).not.toHaveBeenCalled();
-    wrapper.find(Button).simulate('click');
-    expect(trackStatisticsView).toHaveBeenCalledTimes(1);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(2);
-
-    wrapper.setProps({ selectedTraceView: 2 });
-    expect(trackGanttView).not.toHaveBeenCalled();
-    wrapper.find(Button).simulate('click');
-    expect(trackGanttView).toHaveBeenCalledTimes(1);
-    expect(props.onTraceGraphViewClicked).toHaveBeenCalledTimes(3);
+    expect(props.onTraceViewChange).toHaveBeenCalledTimes(6);
   });
 });
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/AltViewOptions.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/AltViewOptions.tsx
@@ -24,50 +24,50 @@ import {
   trackRawJsonView,
 } from './TracePageHeader.track';
 import prefixUrl from '../../../utils/prefix-url';
+import { ETraceViewType } from '../types';
 
 type Props = {
-  onTraceGraphViewClicked: (index: number) => void;
+  onTraceViewChange: (actualViewType: ETraceViewType) => void;
   traceID: string;
-  selectedTraceView: number;
+  viewType: ETraceViewType;
 };
 
-export default function AltViewOptions(props: Props) {
-  const { onTraceGraphViewClicked, selectedTraceView, traceID } = props;
-  const menuItems = ['Trace Timeline', 'Trace Graph', 'Trace Statistics'];
+const menuItems = [
+  {
+    viewType: ETraceViewType.TraceTimelineViewer,
+    label: 'Trace Timeline',
+  },
+  {
+    viewType: ETraceViewType.TraceGraph,
+    label: 'Trace Graph',
+  },
+  {
+    viewType: ETraceViewType.TraceStatistics,
+    label: 'Trace Statistics',
+  },
+];
 
-  const handleToggleView = (index: number, item: string) => {
-    if (item === menuItems[0]) {
-      trackGanttView();
-    } else if (item === menuItems[1]) {
-      trackGraphView();
-    } else if (item === menuItems[2]) {
-      trackStatisticsView();
-    }
-    onTraceGraphViewClicked(index);
-  };
+export default function AltViewOptions(props: Props) {
+  const { onTraceViewChange, viewType, traceID } = props;
 
-  const toggle = () => {
-    let nextIndex = selectedTraceView + 1;
-    if (nextIndex > 2) {
-      nextIndex = 0;
-    }
-    if (nextIndex === 0) {
+  const handleToggleView = (item: ETraceViewType) => {
+    if (item === ETraceViewType.TraceTimelineViewer) {
       trackGanttView();
-    } else if (nextIndex === 1) {
+    } else if (item === ETraceViewType.TraceGraph) {
       trackGraphView();
-    } else if (nextIndex === 2) {
+    } else if (item === ETraceViewType.TraceStatistics) {
       trackStatisticsView();
     }
-    onTraceGraphViewClicked(nextIndex);
+    onTraceViewChange(item);
   };
 
   const menu = (
     <Menu>
-      {menuItems.map((item, index) =>
-        index === selectedTraceView ? null : (
-          <Menu.Item key={item}>
-            <a onClick={() => handleToggleView(index, item)} role="button">
-              {item}
+      {menuItems.map(item =>
+        item.viewType === viewType ? null : (
+          <Menu.Item key={item.label}>
+            <a onClick={() => handleToggleView(item.viewType)} role="button">
+              {item.label}
             </a>
           </Menu.Item>
         )
@@ -97,8 +97,11 @@ export default function AltViewOptions(props: Props) {
 
   return (
     <Dropdown overlay={menu}>
-      <Button onClick={toggle} className="ub-mr2" htmlType="button">
-        Alternate Views <Icon type="down" />
+      <Button className="ub-mr2" htmlType="button">
+        {menuItems.find(test => test.viewType === viewType) !== undefined
+          ? menuItems.find(test => test.viewType === viewType)!.label
+          : ''}
+        <Icon type="down" />
       </Button>
     </Dropdown>
   );

diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
@@ -26,7 +26,7 @@ import AltViewOptions from './AltViewOptions';
 import KeyboardShortcutsHelp from './KeyboardShortcutsHelp';
 import SpanGraph from './SpanGraph';
 import TracePageSearchBar from './TracePageSearchBar';
-import { TUpdateViewRangeTimeFunction, IViewRange, ViewRangeTimeUpdate } from '../types';
+import { TUpdateViewRangeTimeFunction, IViewRange, ViewRangeTimeUpdate, ETraceViewType } from '../types';
 import LabeledList from '../../common/LabeledList';
 import NewWindowIcon from '../../common/NewWindowIcon';
 import TraceName from '../../common/TraceName';
@@ -49,7 +49,7 @@ type TracePageHeaderEmbedProps = {
   nextResult: () => void;
   onArchiveClicked: () => void;
   onSlimViewClicked: () => void;
-  onTraceGraphViewClicked: (index: number) => void;
+  onTraceViewChange: (actualViewType: ETraceViewType) => void;
   prevResult: () => void;
   resultCount: number;
   showArchiveButton: boolean;
@@ -60,7 +60,7 @@ type TracePageHeaderEmbedProps = {
   textFilter: string | TNil;
   toSearch: string | null;
   trace: Trace;
-  selectedTraceView: number;
+  viewType: ETraceViewType;
   updateNextViewRangeTime: (update: ViewRangeTimeUpdate) => void;
   updateViewRangeTime: TUpdateViewRangeTimeFunction;
   viewRange: IViewRange;
@@ -117,7 +117,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
     nextResult,
     onArchiveClicked,
     onSlimViewClicked,
-    onTraceGraphViewClicked,
+    onTraceViewChange,
     prevResult,
     resultCount,
     showArchiveButton,
@@ -128,8 +128,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
     textFilter,
     toSearch,
     trace,
-
-    selectedTraceView,
+    viewType,
     updateNextViewRangeTime,
     updateViewRangeTime,
     viewRange,
@@ -188,15 +187,11 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
           ref={forwardedRef}
           resultCount={resultCount}
           textFilter={textFilter}
-          navigable={selectedTraceView === 0}
+          navigable={viewType === ETraceViewType.TraceTimelineViewer}
         />
         {showShortcutsHelp && <KeyboardShortcutsHelp className="ub-m2" />}
         {showViewOptions && (
-          <AltViewOptions
-            onTraceGraphViewClicked={onTraceGraphViewClicked}
-            traceID={trace.traceID}
-            selectedTraceView={selectedTraceView}
-          />
+          <AltViewOptions onTraceViewChange={onTraceViewChange} traceID={trace.traceID} viewType={viewType} />
         )}
         {showArchiveButton && (
           <Button className="ub-mr2 ub-flex ub-items-center" htmlType="button" onClick={onArchiveClicked}>

diff --git a/...ger-ui/src/components/TracePage/TracePageHeader/__snapshots__/AltViewOptions.test.js.snap b/...ger-ui/src/components/TracePage/TracePageHeader/__snapshots__/AltViewOptions.test.js.snap
@@ -60,10 +60,9 @@ exports[`AltViewOptions renders correctly 1`] = `
     ghost={false}
     htmlType="button"
     loading={false}
-    onClick={[Function]}
     prefixCls="ant-btn"
   >
-    Alternate Views 
+    Trace Timeline
     <Icon
       type="down"
     />

diff --git a/...ts/TracePage/TraceStatistics/PopupSQL.css → ...ts/TracePage/TraceStatistics/PopupSql.css b/...ts/TracePage/TraceStatistics/PopupSQL.css → ...ts/TracePage/TraceStatistics/PopupSql.css
diff --git a/...racePage/TraceStatistics/PopupSQL.test.js → ...racePage/TraceStatistics/PopupSql.test.js b/...racePage/TraceStatistics/PopupSQL.test.js → ...racePage/TraceStatistics/PopupSql.test.js
@@ -14,7 +14,7 @@
 
 import React from 'react';
 import { shallow } from 'enzyme';
-import PopupSQL from './PopupSQL';
+import PopupSql from './PopupSql';
 
 describe('<PopupSQL', () => {
   let wrapper;
@@ -26,7 +26,7 @@ describe('<PopupSQL', () => {
       popupContent:
         'select specialtie0_.vet_id as vet_id1_1_0_, specialtie0_.specialty_id as specialt2_1_0_, specialty1_.id as id1_0_1_, specialty1_.name as name2_0_1_ from vet_specialties specialtie0_ inner join specialties specialty1_ on specialtie0_.specialty_id=specialty1_.id where specialtie0_.vet_id=?',
     };
-    wrapper = shallow(<PopupSQL {...props} />);
+    wrapper = shallow(<PopupSql {...props} />);
   });
 
   it('does not explode', () => {

diff --git a/...ts/TracePage/TraceStatistics/PopupSQL.tsx → ...ts/TracePage/TraceStatistics/PopupSql.tsx b/...ts/TracePage/TraceStatistics/PopupSQL.tsx → ...ts/TracePage/TraceStatistics/PopupSql.tsx
@@ -14,7 +14,7 @@
 
 import React from 'react';
 import { Button } from 'antd';
-import './PopupSQL.css';
+import './PopupSql.css';
 
 type Props = {
   closePopup: (popupContent: string) => void;
@@ -24,7 +24,7 @@ type Props = {
 /**
  * Render the popup that is needed for sql.
  */
-export default function PopupSQL(props: Props) {
+export default function PopupSql(props: Props) {
   const value = `"${props.popupContent}"`;
   return (
     <div className="PopupSQL">

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceStatistics/index.test.js b/packages/jaeger-ui/src/components/TracePage/TraceStatistics/index.test.js
@@ -19,7 +19,7 @@ import MainTableData from './MainTableData';
 import DetailTableData from './DetailTableData';
 import HeaderTable from './HeaderTable';
 import TraceStatisticsHeader from './TraceStatisticsHeader';
-import PopupSQL from './PopupSQL';
+import PopupSql from './PopupSql';
 import transformTraceData from '../../../model/transform-trace-data';
 import { getColumnValues, getColumnValuesSecondDropdown } from './tableValues';
 
@@ -54,7 +54,7 @@ describe('<TraceTagOverview>', () => {
     expect(wrapper.state('valueNameSelector2')).toBe('No Item selected');
     expect(wrapper.find(MainTableData).length).toBe(2);
     expect(wrapper.find(DetailTableData).length).toBe(0);
-    expect(wrapper.find(PopupSQL).length).toBe(0);
+    expect(wrapper.find(PopupSql).length).toBe(0);
   });
 
   it('check search', () => {

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceStatistics/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceStatistics/index.tsx
@@ -23,7 +23,7 @@ import TraceStatisticsHeader from './TraceStatisticsHeader';
 import { ITableSpan } from './types';
 import sortTable from './sortTable';
 import { TNil } from '../../../types';
-import PopupSQL from './PopupSQL';
+import PopupSQL from './PopupSql';
 
 type Props = {
   trace: Trace;

diff --git a/packages/jaeger-ui/src/components/TracePage/index.test.js b/packages/jaeger-ui/src/components/TracePage/index.test.js
@@ -55,6 +55,7 @@ import traceGenerator from '../../demo/trace-generators';
 import transformTraceData from '../../model/transform-trace-data';
 import filterSpansSpy from '../../utils/filter-spans';
 import updateUiFindSpy from '../../utils/update-ui-find';
+import { ETraceViewType } from './types';
 
 describe('makeShortcutCallbacks()', () => {
   let adjRange;
@@ -347,7 +348,7 @@ describe('<TracePage>', () => {
     describe('calculates hideMap correctly', () => {
       it('is true if on traceGraphView', () => {
         wrapper.instance().traceDagEV = { vertices: [], nodes: [] };
-        wrapper.setState({ selectedTraceView: 1 });
+        wrapper.setState({ viewType: ETraceViewType.TraceGraph });
         expect(wrapper.find(TracePageHeader).prop('hideMap')).toBe(true);
       });
 
@@ -432,7 +433,7 @@ describe('<TracePage>', () => {
 
         const size = 30;
         getUiFindVertexKeysSpy.mockReturnValueOnce({ size });
-        wrapper.setState({ selectedTraceView: 1 });
+        wrapper.setState({ viewType: ETraceViewType.TraceGraph });
         wrapper.setProps({ uiFind: 'new ui find to bust memo' });
         expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(size);
       });
@@ -628,16 +629,16 @@ describe('<TracePage>', () => {
     });
 
     it('propagates traceGraphView changes', () => {
-      const { onTraceGraphViewClicked } = header.props();
-      expect(header.prop('selectedTraceView')).toBe(0);
-      onTraceGraphViewClicked();
+      const { onTraceViewChange } = header.props();
+      expect(header.prop('viewType')).toBe(ETraceViewType.TraceTimelineViewer);
+      onTraceViewChange();
       wrapper.update();
       refreshWrappers();
-      expect(header.prop('selectedTraceView')).toBe(undefined);
+      expect(header.prop('viewType')).toBe(undefined);
       expect(calculateTraceDagEVSpy).toHaveBeenCalledWith(defaultProps.trace.data);
 
       wrapper.setProps({ trace: {} });
-      onTraceGraphViewClicked();
+      onTraceViewChange();
       expect(calculateTraceDagEVSpy).toHaveBeenCalledTimes(1);
     });