From a0e296a230cd814a6f987c7c5487dfa0a511119f Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Tue, 2 Apr 2019 17:21:09 -0400
Subject: [PATCH 01/13] WIP - add uiFind icons, scroll to first match

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../components/TracePage/ScrollManager.tsx    | 15 ++++-
 .../TracePageHeader/TracePageHeader.tsx       |  3 +
 .../TracePageHeader/TracePageSearchBar.tsx    | 10 ++-
 .../TraceTimelineViewer/SpanDetail/index.tsx  | 12 ++--
 .../VirtualizedTraceView.tsx                  |  5 ++
 .../TracePage/TraceTimelineViewer/duck.tsx    | 66 ++++++++++++-------
 .../TracePage/TraceTimelineViewer/index.tsx   |  1 +
 .../src/components/TracePage/index.tsx        | 19 ++++--
 8 files changed, 90 insertions(+), 41 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
index 96c4afee03..d8b28fea4f 100644
--- a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
@@ -12,6 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import _get from 'lodash/get';
+import _findIndex from 'lodash/findIndex';
+import _isUndefined from 'lodash/isUndefined';
+
 import { TNil } from '../../types';
 import { Span, SpanReference, Trace } from '../../types/trace';
 
@@ -120,7 +124,7 @@ export default class ScrollManager {
     this._scroller.scrollTo(y);
   }
 
-  _scrollToVisibleSpan(direction: 1 | -1) {
+  _scrollToVisibleSpan(direction: 1 | -1, startRow?: number) {
     const xrs = this._accessors;
     /* istanbul ignore next */
     if (!xrs) {
@@ -131,7 +135,8 @@ export default class ScrollManager {
     }
     const { duration, spans, startTime: traceStartTime } = this._trace;
     const isUp = direction < 0;
-    const boundaryRow = isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible();
+    const boundaryRow = _isUndefined(startRow) ? (isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible()): startRow;
+    console.log(boundaryRow);
     const spanIndex = xrs.mapRowIndexToSpanIndex(boundaryRow);
     if ((spanIndex === 0 && isUp) || (spanIndex === spans.length - 1 && !isUp)) {
       return;
@@ -184,7 +189,7 @@ export default class ScrollManager {
 
       // If there are hidden children, scroll to the last visible span
       if (childrenAreHidden) {
-        let isFallbackHidden: boolean | TNil;
+        let isFallbackHidden: boolean;
         do {
           const { isHidden, parentIDs } = isSpanHidden(spans[nextSpanIndex], childrenAreHidden, spansMap);
           if (isHidden) {
@@ -255,6 +260,10 @@ export default class ScrollManager {
     this._scrollToVisibleSpan(-1);
   };
 
+  scrollToFirstVisibleSpan = () => {
+    this._scrollToVisibleSpan(1, 0);
+  };
+
   destroy() {
     this._trace = undefined;
     this._scroller = undefined as any;
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
index 3ed9cb6386..843c50db1f 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
@@ -40,6 +40,7 @@ import './TracePageHeader.css';
 type TracePageHeaderEmbedProps = {
   canCollapse: boolean;
   clearSearch: () => void;
+  focusUiFind: () => void;
   hideMap: boolean;
   hideSummary: boolean;
   linkToStandalone: string;
@@ -95,6 +96,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
   const {
     canCollapse,
     clearSearch,
+    focusUiFind,
     forwardedRef,
     hideMap,
     hideSummary,
@@ -164,6 +166,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
         {showShortcutsHelp && <KeyboardShortcutsHelp className="ub-mr2" />}
         <TracePageSearchBar
           clearSearch={clearSearch}
+          focusUiFind={focusUiFind}
           nextResult={nextResult}
           prevResult={prevResult}
           ref={forwardedRef}
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
index 56e00bf326..593b355f0b 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
@@ -28,12 +28,13 @@ type TracePageSearchBarProps = {
   prevResult: () => void;
   nextResult: () => void;
   clearSearch: () => void;
+  focusUiFind: () => void;
   resultCount: number;
   navigable: boolean;
 };
 
 export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwardedRef: React.Ref<Input> }) {
-  const { clearSearch, forwardedRef, navigable, nextResult, prevResult, resultCount, textFilter } = props;
+  const { clearSearch, focusUiFind, forwardedRef, navigable, nextResult, prevResult, resultCount, textFilter } = props;
 
   const count = textFilter ? <span className="TracePageSearchBar--count">{resultCount}</span> : null;
 
@@ -70,6 +71,13 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
           icon="down"
           onClick={nextResult}
         />
+        <Button
+          className={navigationBtnClass}
+          disabled={navigationBtnDisabled}
+          htmlType="button"
+          icon="search"
+          onClick={focusUiFind}
+        />
         <Button
           className={btnClass}
           disabled={!textFilter}
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
index 6ff2afb1cf..e22d263074 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
@@ -13,7 +13,7 @@
 // limitations under the License.
 
 import React from 'react';
-import { Divider, Tooltip } from 'antd';
+import { Button, Divider, Tooltip } from 'antd';
 
 import AccordianKeyValues from './AccordianKeyValues';
 import AccordianLogs from './AccordianLogs';
@@ -113,13 +113,11 @@ export default function SpanDetail(props: SpanDetailProps) {
               timestamp={traceStartTime}
             />
           )}
-
         <small className="SpanDetail--debugInfo">
-          <Tooltip title="Click ID to add to filter">
-            <span className="SpanDetail--debugLabel" data-label="SpanID:" />{' '}
-            <button className="SpanDetail--debugValue" type="button" onClick={() => addToUiFind(spanID)}>
-              {spanID}
-            </button>
+          <span className="SpanDetail--debugLabel" data-label="SpanID:" />{' '}
+          {spanID}
+          <Tooltip placement="topRight" title="Click to add to filter for deep linking">
+            <Button className="SpanDetail--debugValue" htmlType="button" icon="down" onClick={() => addToUiFind(spanID)}/>
           </Tooltip>
         </small>
       </div>
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index 4a2f750805..89e1379587 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -52,6 +52,7 @@ type RowState = {
 type TVirtualizedTraceViewOwnProps = {
   currentViewRangeTime: [number, number];
   findMatchesIDs: Set<string> | TNil;
+  scrollToFirstVisibleSpan: () => void;
   registerAccessors: (accesors: Accessors) => void;
   trace: Trace;
 };
@@ -158,6 +159,10 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     setTrace(trace, uiFind);
   }
 
+  componentDidMount() {
+    setTimeout(this.props.scrollToFirstVisibleSpan);
+  }
+
   componentWillUpdate(nextProps: VirtualizedTraceViewProps) {
     const { childrenHiddenIDs, detailStates, registerAccessors, trace, currentViewRangeTime } = this.props;
     const {
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
index a5dc27668b..725183ce6c 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
@@ -68,47 +68,38 @@ export const actionTypes = generateActionTypes('@jaeger-ui/trace-timeline-viewer
   'DETAIL_LOG_ITEM_TOGGLE',
   'EXPAND_ALL',
   'EXPAND_ONE',
+  'FOCUS_UI_FIND',
   'REMOVE_HOVER_INDENT_GUIDE_ID',
   'SET_SPAN_NAME_COLUMN_WIDTH',
   'SET_TRACE',
 ]);
 
 const fullActions = createActions<TActionTypes>({
-  [actionTypes.SET_TRACE]: (trace: Trace, uiFind: string | TNil) => ({ trace, uiFind }),
-  [actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: (width: number) => ({ width }),
+  [actionTypes.ADD_HOVER_INDENT_GUIDE_ID]: (spanID: string) => ({ spanID }),
   [actionTypes.CHILDREN_TOGGLE]: (spanID: string) => ({ spanID }),
-  [actionTypes.EXPAND_ALL]: () => ({}),
-  [actionTypes.EXPAND_ONE]: (spans: Span[]) => ({ spans }),
   [actionTypes.COLLAPSE_ALL]: (spans: Span[]) => ({ spans }),
   [actionTypes.COLLAPSE_ONE]: (spans: Span[]) => ({ spans }),
-  [actionTypes.DETAIL_TOGGLE]: (spanID: string) => ({ spanID }),
-  [actionTypes.DETAIL_TAGS_TOGGLE]: (spanID: string) => ({ spanID }),
-  [actionTypes.DETAIL_PROCESS_TOGGLE]: (spanID: string) => ({ spanID }),
-  [actionTypes.DETAIL_LOGS_TOGGLE]: (spanID: string) => ({ spanID }),
   [actionTypes.DETAIL_LOG_ITEM_TOGGLE]: (spanID: string, logItem: Log) => ({ logItem, spanID }),
-  [actionTypes.ADD_HOVER_INDENT_GUIDE_ID]: (spanID: string) => ({ spanID }),
+  [actionTypes.DETAIL_LOGS_TOGGLE]: (spanID: string) => ({ spanID }),
+  [actionTypes.EXPAND_ALL]: () => ({}),
+  [actionTypes.EXPAND_ONE]: (spans: Span[]) => ({ spans }),
+  [actionTypes.DETAIL_PROCESS_TOGGLE]: (spanID: string) => ({ spanID }),
+  [actionTypes.DETAIL_TAGS_TOGGLE]: (spanID: string) => ({ spanID }),
+  [actionTypes.DETAIL_TOGGLE]: (spanID: string) => ({ spanID }),
+  [actionTypes.FOCUS_UI_FIND]: (trace: Trace, uiFind: string | TNil) => ({ trace, uiFind }),
   [actionTypes.REMOVE_HOVER_INDENT_GUIDE_ID]: (spanID: string) => ({ spanID }),
+  [actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: (width: number) => ({ width }),
+  [actionTypes.SET_TRACE]: (trace: Trace, uiFind: string | TNil) => ({ trace, uiFind }),
 });
 
 export const actions = (fullActions as any).jaegerUi.traceTimelineViewer as TTimelineViewerActions;
 
-function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
-  const { traceID, spans } = trace;
-  if (traceID === state.traceID) {
-    return state;
-  }
-  const { spanNameColumnWidth } = state;
-
-  if (!uiFind) {
-    // No filter, so we're done
-    return { ...newInitialState(), spanNameColumnWidth, traceID };
-  }
-  // There is a filter, so collapse all rows except matches and their ancestors; show details for matches
+function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
   const spansMap = new Map();
   const childrenHiddenIDs = new Set();
   const detailStates = new Map();
 
-  spans.forEach((span: Span) => {
+  spans.forEach(span => {
     spansMap.set(span.spanID, span);
     childrenHiddenIDs.add(span.spanID);
   });
@@ -121,14 +112,38 @@ function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
     });
   }
   return {
-    ...newInitialState(),
-    spanNameColumnWidth,
     childrenHiddenIDs,
     detailStates,
-    traceID,
   };
 }
 
+function focusUiFind(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
+  if (!uiFind) return state;
+  return {
+    ...state,
+    ...calculateHiddenIdsAndDetailStates(uiFind, trace.spans),
+  };
+}
+
+
+function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
+  const { traceID, spans } = trace;
+  if (traceID === state.traceID) {
+    return state;
+  }
+  const { spanNameColumnWidth } = state;
+  const newState = { ...newInitialState(), spanNameColumnWidth, traceID };
+
+  if (uiFind) {
+    Object.assign(
+      newState,
+      calculateHiddenIdsAndDetailStates(uiFind, spans),
+    );
+  }
+
+  return newState;
+}
+
 function setColumnWidth(state: TTraceTimeline, { width }: TWidthValue): TTraceTimeline {
   return { ...state, spanNameColumnWidth: width };
 }
@@ -278,6 +293,7 @@ export default handleActions(
     [actionTypes.DETAIL_TOGGLE]: guardReducer(detailToggle),
     [actionTypes.EXPAND_ALL]: guardReducer(expandAll),
     [actionTypes.EXPAND_ONE]: guardReducer(expandOne),
+    [actionTypes.FOCUS_UI_FIND]: guardReducer(focusUiFind),
     [actionTypes.REMOVE_HOVER_INDENT_GUIDE_ID]: guardReducer(removeHoverIndentGuideId),
     [actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: guardReducer(setColumnWidth),
     [actionTypes.SET_TRACE]: guardReducer(setTrace),
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/index.tsx
index 1c78ca72ee..26af3b5de5 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/index.tsx
@@ -38,6 +38,7 @@ type TDispatchProps = {
 type TProps = TDispatchProps & {
   registerAccessors: (accessors: Accessors) => void;
   findMatchesIDs: Set<string> | TNil;
+  scrollToFirstVisibleSpan: () => void;
   spanNameColumnWidth: number;
   trace: Trace;
   updateNextViewRangeTime: (update: ViewRangeTimeUpdate) => void;
diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 3a2ff5874e..69a7066f16 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -40,6 +40,7 @@ import { TEv } from './TraceGraph/types';
 import { trackSlimHeaderToggle } from './TracePageHeader/TracePageHeader.track';
 import TracePageHeader from './TracePageHeader';
 import TraceTimelineViewer from './TraceTimelineViewer';
+import { actions as timelineActions } from './TraceTimelineViewer/duck';
 import { TUpdateViewRangeTimeFunction, ViewRange, ViewRangeTimeUpdate } from './types';
 import { getLocation, getUrl } from './url';
 import ErrorMessage from '../common/ErrorMessage';
@@ -49,6 +50,7 @@ import * as jaegerApiActions from '../../actions/jaeger-api';
 import { getUiFindVertexKeys } from '../TraceDiff/TraceDiffGraph/traceDiffGraphUtils';
 import { fetchedState } from '../../constants';
 import { FetchedTrace, ReduxState, TNil } from '../../types';
+import { Trace } from '../../types/trace';
 import { TraceArchive } from '../../types/archive';
 import { EmbeddedState } from '../../types/embedded';
 import filterSpans from '../../utils/filter-spans';
@@ -60,6 +62,7 @@ type TDispatchProps = {
   acknowledgeArchive: (id: string) => void;
   archiveTrace: (id: string) => void;
   fetchTrace: (id: string) => void;
+  focusUiFind: (trace: Trace, uiFind: string | TNil) => void;
 };
 
 type TOwnProps = {
@@ -307,7 +310,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
   }
 
   render() {
-    const { archiveEnabled, archiveTraceState, embedded, id, searchUrl, uiFind, trace } = this.props;
+    const { archiveEnabled, archiveTraceState, embedded, focusUiFind, id, searchUrl, uiFind, trace } = this.props;
     const { slimView, traceGraphView, headerHeight, viewRange } = this.state;
     if (!trace || trace.state === fetchedState.LOADING) {
       return <LoadingIndicator className="u-mt-vast" centered />;
@@ -317,7 +320,6 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
       return <ErrorMessage className="ub-m3" error={trace.error || 'Unknown error'} />;
     }
 
-    // $FlowIgnore because flow believes Set<string> cannot be assigned to Set<string | number>
     let findCount = 0;
     let graphFindMatches;
     let spanFindMatches;
@@ -330,10 +332,15 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
         findCount = spanFindMatches ? spanFindMatches.size : 0;
       }
     }
-    //  =  ?  : null;
-    // const spanFindMatches = !traceGraphView ? this._filterSpans(uiFind || '', _get(trace, 'data.spans')) : null;
+
     const isEmbedded = Boolean(embedded);
     const headerProps = {
+      focusUiFind: () => {
+        if (trace.data) {
+          focusUiFind(trace.data, uiFind);
+          setTimeout(this._scrollManager.scrollToFirstVisibleSpan);
+        }
+      },
       slimView,
       textFilter: uiFind,
       traceGraphView,
@@ -382,6 +389,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
             <section style={{ paddingTop: headerHeight }}>
               <TraceTimelineViewer
                 registerAccessors={this._scrollManager.setAccessors}
+                scrollToFirstVisibleSpan={this._scrollManager.scrollToFirstVisibleSpan}
                 findMatchesIDs={spanFindMatches}
                 trace={data}
                 updateNextViewRangeTime={this.updateNextViewRangeTime}
@@ -421,7 +429,8 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP
 export function mapDispatchToProps(dispatch: Dispatch<ReduxState>): TDispatchProps {
   const { fetchTrace } = bindActionCreators(jaegerApiActions, dispatch);
   const { archiveTrace, acknowledge: acknowledgeArchive } = bindActionCreators(archiveActions, dispatch);
-  return { acknowledgeArchive, archiveTrace, fetchTrace };
+  const { focusUiFind } = bindActionCreators(timelineActions, dispatch);
+  return { acknowledgeArchive, archiveTrace, fetchTrace, focusUiFind };
 }
 
 export default connect(mapStateToProps, mapDispatchToProps)(TracePageImpl);

From ebf9473b9613815e861723b77e69ad7eb7c577e2 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 11 Apr 2019 14:57:50 -0400
Subject: [PATCH 02/13] Use componentDidUpdate instead of setTimeout TODO: Fix
 & add tests

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../components/TracePage/ScrollManager.tsx    |  4 ++-
 .../TracePageHeader/TracePageSearchBar.tsx    | 11 +++++++-
 .../TraceTimelineViewer/SpanDetail/index.tsx  | 10 ++++---
 .../VirtualizedTraceView.tsx                  | 20 +++++++++++---
 .../TracePage/TraceTimelineViewer/duck.tsx    | 12 ++++-----
 .../src/components/TracePage/index.tsx        | 26 ++++++++++++++-----
 .../jaeger-ui/src/types/TTraceTimeline.tsx    |  4 +--
 7 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
index d8b28fea4f..6dde845fc1 100644
--- a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
@@ -135,7 +135,9 @@ export default class ScrollManager {
     }
     const { duration, spans, startTime: traceStartTime } = this._trace;
     const isUp = direction < 0;
-    const boundaryRow = _isUndefined(startRow) ? (isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible()): startRow;
+    const boundaryRow = _isUndefined(startRow)
+      ? isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible()
+      : startRow;
     console.log(boundaryRow);
     const spanIndex = xrs.mapRowIndexToSpanIndex(boundaryRow);
     if ((spanIndex === 0 && isUp) || (spanIndex === spans.length - 1 && !isUp)) {
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
index 593b355f0b..2ff5aefa5b 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
@@ -34,7 +34,16 @@ type TracePageSearchBarProps = {
 };
 
 export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwardedRef: React.Ref<Input> }) {
-  const { clearSearch, focusUiFind, forwardedRef, navigable, nextResult, prevResult, resultCount, textFilter } = props;
+  const {
+    clearSearch,
+    focusUiFind,
+    forwardedRef,
+    navigable,
+    nextResult,
+    prevResult,
+    resultCount,
+    textFilter,
+  } = props;
 
   const count = textFilter ? <span className="TracePageSearchBar--count">{resultCount}</span> : null;
 
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
index e22d263074..b95bbe573f 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
@@ -114,10 +114,14 @@ export default function SpanDetail(props: SpanDetailProps) {
             />
           )}
         <small className="SpanDetail--debugInfo">
-          <span className="SpanDetail--debugLabel" data-label="SpanID:" />{' '}
-          {spanID}
+          <span className="SpanDetail--debugLabel" data-label="SpanID:" /> {spanID}
           <Tooltip placement="topRight" title="Click to add to filter for deep linking">
-            <Button className="SpanDetail--debugValue" htmlType="button" icon="down" onClick={() => addToUiFind(spanID)}/>
+            <Button
+              className="SpanDetail--debugValue"
+              htmlType="button"
+              icon="down"
+              onClick={() => addToUiFind(spanID)}
+            />
           </Tooltip>
         </small>
       </div>
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index 89e1379587..f5e93fef11 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -159,10 +159,6 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     setTrace(trace, uiFind);
   }
 
-  componentDidMount() {
-    setTimeout(this.props.scrollToFirstVisibleSpan);
-  }
-
   componentWillUpdate(nextProps: VirtualizedTraceViewProps) {
     const { childrenHiddenIDs, detailStates, registerAccessors, trace, currentViewRangeTime } = this.props;
     const {
@@ -195,6 +191,22 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     }
   }
 
+  componentDidUpdate(prevProps: VirtualizedTraceViewProps) {
+    const {
+      detailStates: currDetailStates,
+      childrenHiddenIDs: currChildrenHiddenIds,
+      scrollToFirstVisibleSpan,
+    } = this.props;
+    const { detailStates: prevDetailStates, childrenHiddenIDs: prevChildrenHiddenIds } = prevProps;
+
+    if (
+      (currChildrenHiddenIds.needToScroll && currChildrenHiddenIds !== prevChildrenHiddenIds) ||
+      (currDetailStates.needToScroll && currDetailStates !== prevDetailStates)
+    ) {
+      scrollToFirstVisibleSpan();
+    }
+  }
+
   addToUiFind = (addition: string) => {
     const { uiFind, history, location } = this.props;
     if (!uiFind || !uiFind.includes(addition)) {
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
index 725183ce6c..1cba415f09 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
@@ -96,8 +96,8 @@ export const actions = (fullActions as any).jaegerUi.traceTimelineViewer as TTim
 
 function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
   const spansMap = new Map();
-  const childrenHiddenIDs = new Set();
-  const detailStates = new Map();
+  const childrenHiddenIDs: Set<string> & { needToScroll?: true } = new Set();
+  const detailStates: Map<string, DetailState> & { needToScroll?: true } = new Map();
 
   spans.forEach(span => {
     spansMap.set(span.spanID, span);
@@ -110,6 +110,8 @@ function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
       detailStates.set(spanID, new DetailState());
       spanAncestorIds(span).forEach(ancestorID => childrenHiddenIDs.delete(ancestorID));
     });
+    childrenHiddenIDs.needToScroll = true;
+    detailStates.needToScroll = true;
   }
   return {
     childrenHiddenIDs,
@@ -125,7 +127,6 @@ function focusUiFind(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue
   };
 }
 
-
 function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
   const { traceID, spans } = trace;
   if (traceID === state.traceID) {
@@ -135,10 +136,7 @@ function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
   const newState = { ...newInitialState(), spanNameColumnWidth, traceID };
 
   if (uiFind) {
-    Object.assign(
-      newState,
-      calculateHiddenIdsAndDetailStates(uiFind, spans),
-    );
+    Object.assign(newState, calculateHiddenIdsAndDetailStates(uiFind, spans));
   }
 
   return newState;
diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 69a7066f16..1813acf6b4 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -309,8 +309,25 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
     }
   }
 
+  focusUiFind = () => {
+    const { trace, focusUiFind, uiFind } = this.props;
+    if (trace && trace.data) {
+      this._scrollManager.scrollToFirstVisibleSpan();
+      focusUiFind(trace.data, uiFind);
+    }
+  };
+
   render() {
-    const { archiveEnabled, archiveTraceState, embedded, focusUiFind, id, searchUrl, uiFind, trace } = this.props;
+    const {
+      archiveEnabled,
+      archiveTraceState,
+      embedded,
+      focusUiFind,
+      id,
+      searchUrl,
+      uiFind,
+      trace,
+    } = this.props;
     const { slimView, traceGraphView, headerHeight, viewRange } = this.state;
     if (!trace || trace.state === fetchedState.LOADING) {
       return <LoadingIndicator className="u-mt-vast" centered />;
@@ -335,12 +352,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
 
     const isEmbedded = Boolean(embedded);
     const headerProps = {
-      focusUiFind: () => {
-        if (trace.data) {
-          focusUiFind(trace.data, uiFind);
-          setTimeout(this._scrollManager.scrollToFirstVisibleSpan);
-        }
-      },
+      focusUiFind: this.focusUiFind,
       slimView,
       textFilter: uiFind,
       traceGraphView,
diff --git a/packages/jaeger-ui/src/types/TTraceTimeline.tsx b/packages/jaeger-ui/src/types/TTraceTimeline.tsx
index e926aedc79..55c173c01e 100644
--- a/packages/jaeger-ui/src/types/TTraceTimeline.tsx
+++ b/packages/jaeger-ui/src/types/TTraceTimeline.tsx
@@ -16,8 +16,8 @@ import DetailState from '../components/TracePage/TraceTimelineViewer/SpanDetail/
 import TNil from './TNil';
 
 type TTraceTimeline = {
-  childrenHiddenIDs: Set<string>;
-  detailStates: Map<string, DetailState>;
+  childrenHiddenIDs: Set<string> & { needToScroll?: true };
+  detailStates: Map<string, DetailState> & { needToScroll?: true };
   hoverIndentGuideIds: Set<string>;
   spanNameColumnWidth: number;
   traceID: string | TNil;

From 37f9a20e40cea0930fd917e652b616f1dc80790d Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 11 Apr 2019 16:47:37 -0400
Subject: [PATCH 03/13] Clarify focusUiFind function name, fix existing tests,
 add TraceTimelineViewer/duck.tsx tests

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../components/TracePage/ScrollManager.tsx    |  11 +-
 .../TracePageHeader/TracePageHeader.tsx       |   6 +-
 .../TracePageSearchBar.test.js                |  16 +-
 .../TracePageHeader/TracePageSearchBar.tsx    |  53 ++---
 .../SpanDetail/index.test.js                  |   3 +-
 .../TraceTimelineViewer/duck.test.js          | 212 +++++++++++-------
 .../TracePage/TraceTimelineViewer/duck.tsx    |  20 +-
 .../src/components/TracePage/index.test.js    |   3 +-
 .../src/components/TracePage/index.tsx        |  16 +-
 9 files changed, 188 insertions(+), 152 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
index 6dde845fc1..b975555f3b 100644
--- a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
@@ -12,10 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import _get from 'lodash/get';
-import _findIndex from 'lodash/findIndex';
-import _isUndefined from 'lodash/isUndefined';
-
 import { TNil } from '../../types';
 import { Span, SpanReference, Trace } from '../../types/trace';
 
@@ -135,11 +131,8 @@ export default class ScrollManager {
     }
     const { duration, spans, startTime: traceStartTime } = this._trace;
     const isUp = direction < 0;
-    const boundaryRow = _isUndefined(startRow)
-      ? isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible()
-      : startRow;
-    console.log(boundaryRow);
-    const spanIndex = xrs.mapRowIndexToSpanIndex(boundaryRow);
+    const boundaryRow = isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible();
+    const spanIndex = xrs.mapRowIndexToSpanIndex(startRow != null ? startRow : boundaryRow);
     if ((spanIndex === 0 && isUp) || (spanIndex === spans.length - 1 && !isUp)) {
       return;
     }
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
index 843c50db1f..5d26ebc4e5 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
@@ -40,7 +40,7 @@ import './TracePageHeader.css';
 type TracePageHeaderEmbedProps = {
   canCollapse: boolean;
   clearSearch: () => void;
-  focusUiFind: () => void;
+  focusUiFindMatches: () => void;
   hideMap: boolean;
   hideSummary: boolean;
   linkToStandalone: string;
@@ -96,7 +96,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
   const {
     canCollapse,
     clearSearch,
-    focusUiFind,
+    focusUiFindMatches,
     forwardedRef,
     hideMap,
     hideSummary,
@@ -166,7 +166,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
         {showShortcutsHelp && <KeyboardShortcutsHelp className="ub-mr2" />}
         <TracePageSearchBar
           clearSearch={clearSearch}
-          focusUiFind={focusUiFind}
+          focusUiFindMatches={focusUiFindMatches}
           nextResult={nextResult}
           prevResult={prevResult}
           ref={forwardedRef}
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js
index bb3f249b96..4fcfea7cca 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js
@@ -55,7 +55,7 @@ describe('<TracePageSearchBar>', () => {
 
     it('renders buttons', () => {
       const buttons = wrapper.find('Button');
-      expect(buttons.length).toBe(3);
+      expect(buttons.length).toBe(4);
       buttons.forEach(button => {
         expect(button.hasClass('TracePageSearchBar--btn')).toBe(true);
         expect(button.hasClass('is-disabled')).toBe(false);
@@ -66,15 +66,11 @@ describe('<TracePageSearchBar>', () => {
       expect(wrapper.find('Button[icon="close"]').prop('onClick')).toBe(defaultProps.clearSearch);
     });
 
-    it('disables navigation buttons when not navigable', () => {
+    it('hides navigation buttons when not navigable', () => {
       wrapper.setProps({ navigable: false });
-      const buttons = wrapper.find('Button');
-      expect(buttons.length).toBe(3);
-      buttons.forEach((button, i) => {
-        expect(button.hasClass('TracePageSearchBar--btn')).toBe(true);
-        expect(button.hasClass('is-disabled')).toBe(i !== 2);
-        expect(button.prop('disabled')).toBe(i !== 2);
-      });
+      const button = wrapper.find('Button');
+      expect(button.length).toBe(1);
+      expect(button.prop('icon')).toBe('close');
     });
   });
 
@@ -89,7 +85,7 @@ describe('<TracePageSearchBar>', () => {
 
     it('renders buttons', () => {
       const buttons = wrapper.find('Button');
-      expect(buttons.length).toBe(3);
+      expect(buttons.length).toBe(4);
       buttons.forEach(button => {
         expect(button.hasClass('TracePageSearchBar--btn')).toBe(true);
         expect(button.hasClass('is-disabled')).toBe(true);
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
index 2ff5aefa5b..0da70d0ad8 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
@@ -28,7 +28,7 @@ type TracePageSearchBarProps = {
   prevResult: () => void;
   nextResult: () => void;
   clearSearch: () => void;
-  focusUiFind: () => void;
+  focusUiFindMatches: () => void;
   resultCount: number;
   navigable: boolean;
 };
@@ -36,7 +36,7 @@ type TracePageSearchBarProps = {
 export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwardedRef: React.Ref<Input> }) {
   const {
     clearSearch,
-    focusUiFind,
+    focusUiFindMatches,
     forwardedRef,
     navigable,
     nextResult,
@@ -47,8 +47,7 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
 
   const count = textFilter ? <span className="TracePageSearchBar--count">{resultCount}</span> : null;
 
-  const navigationBtnDisabled = !navigable || !textFilter;
-  const navigationBtnClass = cx('TracePageSearchBar--btn', { 'is-disabled': navigationBtnDisabled });
+  const navigationBtnClass = cx('TracePageSearchBar--btn', { 'is-disabled': !textFilter });
   const btnClass = cx('TracePageSearchBar--btn', { 'is-disabled': !textFilter });
   const uiFindInputInputProps = {
     'data-test': markers.IN_TRACE_SEARCH,
@@ -66,27 +65,31 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
           forwardedRef={forwardedRef}
           trackFindFunction={trackFilter}
         />
-        <Button
-          className={navigationBtnClass}
-          disabled={navigationBtnDisabled}
-          htmlType="button"
-          icon="up"
-          onClick={prevResult}
-        />
-        <Button
-          className={navigationBtnClass}
-          disabled={navigationBtnDisabled}
-          htmlType="button"
-          icon="down"
-          onClick={nextResult}
-        />
-        <Button
-          className={navigationBtnClass}
-          disabled={navigationBtnDisabled}
-          htmlType="button"
-          icon="search"
-          onClick={focusUiFind}
-        />
+        {navigable && (
+          <>
+            <Button
+              className={navigationBtnClass}
+              disabled={!textFilter}
+              htmlType="button"
+              icon="up"
+              onClick={prevResult}
+            />
+            <Button
+              className={navigationBtnClass}
+              disabled={!textFilter}
+              htmlType="button"
+              icon="down"
+              onClick={nextResult}
+            />
+            <Button
+              className={navigationBtnClass}
+              disabled={!textFilter}
+              htmlType="button"
+              icon="search"
+              onClick={focusUiFindMatches}
+            />
+          </>
+        )}
         <Button
           className={btnClass}
           disabled={!textFilter}
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
index 9740c8b20d..fa5e822fd8 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
@@ -16,6 +16,7 @@
 jest.mock('../utils');
 
 import React from 'react';
+import { Button } from 'antd';
 import { shallow } from 'enzyme';
 
 import AccordianKeyValues from './AccordianKeyValues';
@@ -122,7 +123,7 @@ describe('<SpanDetail>', () => {
   });
 
   it('calls addToUiFind when the spanID is clicked', () => {
-    const spanIDButton = wrapper.find('button');
+    const spanIDButton = wrapper.find(Button);
     spanIDButton.simulate('click');
     expect(props.addToUiFind).toHaveBeenCalledWith(props.span.spanID);
   });
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
index ec0405c612..f4208cf979 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
@@ -51,102 +51,146 @@ describe('TraceTimelineViewer/duck', () => {
     expect(width).toBe(n);
   });
 
-  describe('setTrace', () => {
-    describe('without uiFind', () => {
-      it('retains all state when setting to the same traceID', () => {
-        const action = actions.setTrace(trace);
-        store.dispatch(action);
-        const state = store.getState();
-        store.dispatch(action);
-        expect(store.getState()).toBe(state);
-      });
+  describe('focusUiFindMatches', () => {
+    const uiFind = 'uiFind';
+    const action = actions.focusUiFindMatches(trace, uiFind);
+    const uiFindMatchesArray = [trace.spans[5].spanID, trace.spans[10].spanID, trace.spans[15].spanID];
+    const uiFindMatches = new Set(uiFindMatchesArray);
+    const uiFindAncestorIdsMockSchema = {
+      [uiFindMatchesArray[0]]: [trace.spans[0].spanID, trace.spans[3].spanID, trace.spans[4].spanID],
+      [uiFindMatchesArray[1]]: [
+        trace.spans[0].spanID,
+        trace.spans[3].spanID,
+        trace.spans[4].spanID,
+        trace.spans[5].spanID,
+        trace.spans[8].spanID,
+        trace.spans[9].spanID,
+      ],
+      [uiFindMatchesArray[2]]: [
+        trace.spans[0].spanID,
+        trace.spans[3].spanID,
+        trace.spans[13].spanID,
+        trace.spans[14].spanID,
+      ],
+    };
+    const uiFindAncestorIdsSet = new Set(
+      _reduce(uiFindAncestorIdsMockSchema, (allAncestors, spanAncestors) =>
+        allAncestors.concat(spanAncestors)
+      )
+    );
+    let focusUiFindMatchesStore;
+    let state;
+
+    beforeAll(() => {
+      filterSpansSpy.mockReturnValue(uiFindMatches);
+      spanAncestorIdsSpy.mockImplementation(({ spanID }) => uiFindAncestorIdsMockSchema[spanID]);
+      focusUiFindMatchesStore = createStore(reducer, newInitialState());
+      focusUiFindMatchesStore.dispatch(action);
+      state = focusUiFindMatchesStore.getState();
+    });
 
-      it('retains only the spanNameColumnWidth when changing traceIDs', () => {
-        let action;
-        const width = 0.5;
-        const id = 'some-id';
+    it('adds a detailState for each span matching the uiFind filter', () => {
+      // Sanity check
+      expect(trace.spans).toHaveLength(30);
+      expect(uiFindMatches.size).toBe(3);
 
-        action = actions.childrenToggle(id);
-        store.dispatch(action);
-        action = actions.detailToggle(id);
-        store.dispatch(action);
-        action = actions.setSpanNameColumnWidth(width);
-        store.dispatch(action);
+      expect(filterSpansSpy).toHaveBeenCalledWith(uiFind, trace.spans);
+      trace.spans.forEach(({ spanID }) => {
+        expect(state.detailStates.has(spanID)).toBe(uiFindMatches.has(spanID));
+      });
+    });
 
-        let state = store.getState();
-        expect(state.traceID).toBe(null);
-        expect(state.childrenHiddenIDs).not.toEqual(new Set());
-        expect(state.detailStates).not.toEqual(new Map());
-        expect(state.spanNameColumnWidth).toBe(width);
+    it('hides the children of all spanIDs that are not ancestors of a span matching the uiFind filter', () => {
+      // Sanity check
+      expect(trace.spans).toHaveLength(30);
+      expect(uiFindAncestorIdsSet.size).toBe(8);
 
-        action = actions.setTrace(trace);
-        store.dispatch(action);
-        state = store.getState();
-        expect(state.traceID).toBe(trace.traceID);
-        expect(state.childrenHiddenIDs).toEqual(new Set());
-        expect(state.detailStates).toEqual(new Map());
-        expect(state.spanNameColumnWidth).toBe(width);
+      trace.spans.forEach(({ spanID }) => {
+        expect(state.childrenHiddenIDs.has(spanID)).toBe(!uiFindAncestorIdsSet.has(spanID));
       });
     });
 
-    describe('with uiFind', () => {
-      const uiFind = 'uiFind';
-      const uiFindMatchesArray = [trace.spans[5].spanID, trace.spans[10].spanID, trace.spans[15].spanID];
-      const uiFindMatches = new Set(uiFindMatchesArray);
-      const uiFindAncestorIdsMockSchema = {
-        [uiFindMatchesArray[0]]: [trace.spans[0].spanID, trace.spans[3].spanID, trace.spans[4].spanID],
-        [uiFindMatchesArray[1]]: [
-          trace.spans[0].spanID,
-          trace.spans[3].spanID,
-          trace.spans[4].spanID,
-          trace.spans[5].spanID,
-          trace.spans[8].spanID,
-          trace.spans[9].spanID,
-        ],
-        [uiFindMatchesArray[2]]: [
-          trace.spans[0].spanID,
-          trace.spans[3].spanID,
-          trace.spans[13].spanID,
-          trace.spans[14].spanID,
-        ],
-      };
-      const uiFindAncestorIdsSet = new Set(
-        _reduce(uiFindAncestorIdsMockSchema, (allAncestors, spanAncestors) =>
-          allAncestors.concat(spanAncestors)
-        )
-      );
-      let state;
-
-      beforeAll(() => {
-        filterSpansSpy.mockReturnValue(uiFindMatches);
-        spanAncestorIdsSpy.mockImplementation(({ spanID }) => uiFindAncestorIdsMockSchema[spanID]);
-        const action = actions.setTrace(trace, uiFind);
-        store = createStore(reducer, newInitialState());
-        store.dispatch(action);
-        state = store.getState();
-      });
+    it('indicates the need to scroll iff there are uiFindMatches', () => {
+      expect(state.detailStates.needToScroll).toBe(true);
+      expect(state.childrenHiddenIDs.needToScroll).toBe(true);
 
-      it('adds a detailState for each span matching the uiFind filter', () => {
-        // Sanity check
-        expect(trace.spans).toHaveLength(30);
-        expect(uiFindMatches.size).toBe(3);
+      filterSpansSpy.mockReturnValue(new Set());
+      const singleSpecStore = createStore(reducer, newInitialState());
+      singleSpecStore.dispatch(action);
+      const singleSpecState = singleSpecStore.getState();
+      expect(singleSpecState.detailStates.needToScroll).toBeUndefined();
+      expect(singleSpecState.childrenHiddenIDs.needToScroll).toBeUndefined();
+    });
 
-        expect(filterSpansSpy).toHaveBeenCalledWith(uiFind, trace.spans);
-        trace.spans.forEach(({ spanID }) => {
-          expect(state.detailStates.has(spanID)).toBe(uiFindMatches.has(spanID));
-        });
-      });
+    it('returns existing state if uiFind is falsy', () => {
+      const emptyStringAction = actions.focusUiFindMatches(trace, '');
+      focusUiFindMatchesStore.dispatch(emptyStringAction);
+      expect(focusUiFindMatchesStore.getState()).toBe(state);
 
-      it('hides the children of all spanIDs that are not ancestors of a span matching the uiFind filter', () => {
-        // Sanity check
-        expect(trace.spans).toHaveLength(30);
-        expect(uiFindAncestorIdsSet.size).toBe(8);
+      const nullAction = actions.focusUiFindMatches(trace, null);
+      focusUiFindMatchesStore.dispatch(nullAction);
+      expect(focusUiFindMatchesStore.getState()).toBe(state);
 
-        trace.spans.forEach(({ spanID }) => {
-          expect(state.childrenHiddenIDs.has(spanID)).toBe(!uiFindAncestorIdsSet.has(spanID));
-        });
-      });
+      const undefinedAction = actions.focusUiFindMatches(trace, undefined);
+      focusUiFindMatchesStore.dispatch(undefinedAction);
+      expect(focusUiFindMatchesStore.getState()).toBe(state);
+    });
+  });
+
+  describe('setTrace', () => {
+    const setTraceAction = actions.setTrace(trace);
+
+    it('retains all state when setting to the same traceID', () => {
+      store.dispatch(setTraceAction);
+      const state = store.getState();
+      store.dispatch(setTraceAction);
+      expect(store.getState()).toBe(state);
+    });
+
+    it('retains only the spanNameColumnWidth when changing traceIDs', () => {
+      let action;
+      const width = 0.5;
+      const id = 'some-id';
+
+      action = actions.childrenToggle(id);
+      store.dispatch(action);
+      action = actions.detailToggle(id);
+      store.dispatch(action);
+      action = actions.setSpanNameColumnWidth(width);
+      store.dispatch(action);
+
+      let state = store.getState();
+      expect(state.traceID).toBe(null);
+      expect(state.childrenHiddenIDs).not.toEqual(new Set());
+      expect(state.detailStates).not.toEqual(new Map());
+      expect(state.spanNameColumnWidth).toBe(width);
+
+      store.dispatch(setTraceAction);
+      state = store.getState();
+      expect(state.traceID).toBe(trace.traceID);
+      expect(state.childrenHiddenIDs).toEqual(new Set());
+      expect(state.detailStates).toEqual(new Map());
+      expect(state.spanNameColumnWidth).toBe(width);
+    });
+
+    it('calls calculateHiddenIdsAndDetailStates iff a truthy uiFind is provided', () => {
+      store.dispatch(setTraceAction);
+      let state = store.getState();
+      expect(state.childrenHiddenIDs).toEqual(new Set());
+
+      store.dispatch(actions.setTrace(trace, null));
+      state = store.getState();
+      expect(state.childrenHiddenIDs).toEqual(new Set());
+
+      store.dispatch(actions.setTrace(trace, undefined));
+      state = store.getState();
+      expect(state.childrenHiddenIDs).toEqual(new Set());
+
+      store.dispatch(actions.setTrace(trace, 'truthy uiFind string'));
+      state = store.getState();
+      expect(state.childrenHiddenIDs).toEqual(new Set());
     });
+    // describe('calls focus sometmisth', () => {
   });
 
   describe('toggles children and details', () => {
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
index 1cba415f09..ca09021b07 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
@@ -68,7 +68,7 @@ export const actionTypes = generateActionTypes('@jaeger-ui/trace-timeline-viewer
   'DETAIL_LOG_ITEM_TOGGLE',
   'EXPAND_ALL',
   'EXPAND_ONE',
-  'FOCUS_UI_FIND',
+  'FOCUS_UI_FIND_MATCHES',
   'REMOVE_HOVER_INDENT_GUIDE_ID',
   'SET_SPAN_NAME_COLUMN_WIDTH',
   'SET_TRACE',
@@ -86,7 +86,7 @@ const fullActions = createActions<TActionTypes>({
   [actionTypes.DETAIL_PROCESS_TOGGLE]: (spanID: string) => ({ spanID }),
   [actionTypes.DETAIL_TAGS_TOGGLE]: (spanID: string) => ({ spanID }),
   [actionTypes.DETAIL_TOGGLE]: (spanID: string) => ({ spanID }),
-  [actionTypes.FOCUS_UI_FIND]: (trace: Trace, uiFind: string | TNil) => ({ trace, uiFind }),
+  [actionTypes.FOCUS_UI_FIND_MATCHES]: (trace: Trace, uiFind: string | TNil) => ({ trace, uiFind }),
   [actionTypes.REMOVE_HOVER_INDENT_GUIDE_ID]: (spanID: string) => ({ spanID }),
   [actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: (width: number) => ({ width }),
   [actionTypes.SET_TRACE]: (trace: Trace, uiFind: string | TNil) => ({ trace, uiFind }),
@@ -104,7 +104,7 @@ function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
     childrenHiddenIDs.add(span.spanID);
   });
   const matchedSpanIds = filterSpans(uiFind, spans);
-  if (matchedSpanIds) {
+  if (matchedSpanIds && matchedSpanIds.size) {
     matchedSpanIds.forEach(spanID => {
       const span = spansMap.get(spanID);
       detailStates.set(spanID, new DetailState());
@@ -119,7 +119,7 @@ function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
   };
 }
 
-function focusUiFind(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
+function focusUiFindMatches(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
   if (!uiFind) return state;
   return {
     ...state,
@@ -133,13 +133,11 @@ function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
     return state;
   }
   const { spanNameColumnWidth } = state;
-  const newState = { ...newInitialState(), spanNameColumnWidth, traceID };
 
-  if (uiFind) {
-    Object.assign(newState, calculateHiddenIdsAndDetailStates(uiFind, spans));
-  }
-
-  return newState;
+  return Object.assign(
+    { ...newInitialState(), spanNameColumnWidth, traceID },
+    uiFind ? calculateHiddenIdsAndDetailStates(uiFind, spans) : null
+  );
 }
 
 function setColumnWidth(state: TTraceTimeline, { width }: TWidthValue): TTraceTimeline {
@@ -291,7 +289,7 @@ export default handleActions(
     [actionTypes.DETAIL_TOGGLE]: guardReducer(detailToggle),
     [actionTypes.EXPAND_ALL]: guardReducer(expandAll),
     [actionTypes.EXPAND_ONE]: guardReducer(expandOne),
-    [actionTypes.FOCUS_UI_FIND]: guardReducer(focusUiFind),
+    [actionTypes.FOCUS_UI_FIND_MATCHES]: guardReducer(focusUiFindMatches),
     [actionTypes.REMOVE_HOVER_INDENT_GUIDE_ID]: guardReducer(removeHoverIndentGuideId),
     [actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: guardReducer(setColumnWidth),
     [actionTypes.SET_TRACE]: guardReducer(setTrace),
diff --git a/packages/jaeger-ui/src/components/TracePage/index.test.js b/packages/jaeger-ui/src/components/TracePage/index.test.js
index 42a36d132d..485552636f 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/index.test.js
@@ -581,9 +581,10 @@ describe('<TracePage>', () => {
 describe('mapDispatchToProps()', () => {
   it('creates the actions correctly', () => {
     expect(mapDispatchToProps(() => {})).toEqual({
-      fetchTrace: expect.any(Function),
       acknowledgeArchive: expect.any(Function),
       archiveTrace: expect.any(Function),
+      fetchTrace: expect.any(Function),
+      focusUiFindMatches: expect.any(Function),
     });
   });
 });
diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 1813acf6b4..5c9fe9c3dd 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -62,7 +62,7 @@ type TDispatchProps = {
   acknowledgeArchive: (id: string) => void;
   archiveTrace: (id: string) => void;
   fetchTrace: (id: string) => void;
-  focusUiFind: (trace: Trace, uiFind: string | TNil) => void;
+  focusUiFindMatches: (trace: Trace, uiFind: string | TNil) => void;
 };
 
 type TOwnProps = {
@@ -309,11 +309,11 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
     }
   }
 
-  focusUiFind = () => {
-    const { trace, focusUiFind, uiFind } = this.props;
+  focusUiFindMatches = () => {
+    const { trace, focusUiFindMatches, uiFind } = this.props;
     if (trace && trace.data) {
       this._scrollManager.scrollToFirstVisibleSpan();
-      focusUiFind(trace.data, uiFind);
+      focusUiFindMatches(trace.data, uiFind);
     }
   };
 
@@ -322,7 +322,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
       archiveEnabled,
       archiveTraceState,
       embedded,
-      focusUiFind,
+      focusUiFindMatches,
       id,
       searchUrl,
       uiFind,
@@ -352,7 +352,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
 
     const isEmbedded = Boolean(embedded);
     const headerProps = {
-      focusUiFind: this.focusUiFind,
+      focusUiFindMatches: this.focusUiFindMatches,
       slimView,
       textFilter: uiFind,
       traceGraphView,
@@ -441,8 +441,8 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP
 export function mapDispatchToProps(dispatch: Dispatch<ReduxState>): TDispatchProps {
   const { fetchTrace } = bindActionCreators(jaegerApiActions, dispatch);
   const { archiveTrace, acknowledge: acknowledgeArchive } = bindActionCreators(archiveActions, dispatch);
-  const { focusUiFind } = bindActionCreators(timelineActions, dispatch);
-  return { acknowledgeArchive, archiveTrace, fetchTrace, focusUiFind };
+  const { focusUiFindMatches } = bindActionCreators(timelineActions, dispatch);
+  return { acknowledgeArchive, archiveTrace, fetchTrace, focusUiFindMatches };
 }
 
 export default connect(mapStateToProps, mapDispatchToProps)(TracePageImpl);

From bb66f09b63b7089ef24633c58ef90ba4468310a3 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 11 Apr 2019 17:21:30 -0400
Subject: [PATCH 04/13] Add scrollToFirstVisibleSpan tests

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../VirtualizedTraceView.test.js              | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
index 04d938929a..910f0b1d4b 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
@@ -41,6 +41,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
     detailToggle: jest.fn(),
     findMatchesIDs: null,
     registerAccessors: jest.fn(),
+    scrollToFirstVisibleSpan: jest.fn(),
     setSpanNameColumnWidth: jest.fn(),
     setTrace: jest.fn(),
     spanNameColumnWidth: 0.5,
@@ -343,4 +344,34 @@ describe('<VirtualizedTraceViewImpl>', () => {
       ).toBe(true);
     });
   });
+
+  describe('handles needToScroll', () => {
+    beforeEach(() => {
+      props.scrollToFirstVisibleSpan.mockReset();
+    });
+
+    [[Set, 'childrenHiddenIDs'], [Map, 'detailStates']].forEach(([ConstructorFn, propName]) => {
+      it(`calls props.scrollToFirstVisibleSpan if ${propName} changes and ${propName}.needToScroll is true`, () => {
+        expect(props.scrollToFirstVisibleSpan).not.toHaveBeenCalled();
+
+        const collectionThatNeedsToScroll = new ConstructorFn();
+        collectionThatNeedsToScroll.needToScroll = true;
+
+        wrapper.setProps({ [propName]: collectionThatNeedsToScroll });
+        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1);
+
+        wrapper.setProps({ [propName]: collectionThatNeedsToScroll });
+        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1);
+
+        const differentCollectionThatAlsoNeedsToScroll = new ConstructorFn();
+        differentCollectionThatAlsoNeedsToScroll.needToScroll = true;
+        wrapper.setProps({ [propName]: differentCollectionThatAlsoNeedsToScroll });
+        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(2);
+
+        const differentCollectionThatDoesNotNeedToScroll = new ConstructorFn();
+        wrapper.setProps({ [propName]: differentCollectionThatDoesNotNeedToScroll });
+        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(2);
+      });
+    });
+  });
 });

From 79f04af2e4ca89fb8bad7857fa12993bd26fe48c Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 11 Apr 2019 17:21:30 -0400
Subject: [PATCH 05/13] Add scrollToFirstVisibleSpan tests

Signed-off-by: Everett Ross <reverett@uber.com>
---
 packages/jaeger-ui/src/components/TracePage/index.tsx | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 5c9fe9c3dd..290c6b32a9 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -318,16 +318,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
   };
 
   render() {
-    const {
-      archiveEnabled,
-      archiveTraceState,
-      embedded,
-      focusUiFindMatches,
-      id,
-      searchUrl,
-      uiFind,
-      trace,
-    } = this.props;
+    const { archiveEnabled, archiveTraceState, embedded, id, searchUrl, uiFind, trace } = this.props;
     const { slimView, traceGraphView, headerHeight, viewRange } = this.state;
     if (!trace || trace.state === fetchedState.LOADING) {
       return <LoadingIndicator className="u-mt-vast" centered />;

From 79eb6df2f7a55c9879d68a4c82d803fa84330c0a Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Fri, 12 Apr 2019 15:05:21 -0400
Subject: [PATCH 06/13] Bolster test coverage for files in PR

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../TracePage/ScrollManager.test.js           | 52 ++++++++++--
 .../TracePageHeader/TracePageHeader.test.js   |  9 +++
 .../TracePageSearchBar.test.js                | 29 ++++---
 .../TraceTimelineViewer/duck.test.js          |  5 ++
 .../src/components/TracePage/index.test.js    | 79 ++++++++++++++++++-
 .../src/components/TracePage/index.tsx        | 18 ++---
 6 files changed, 161 insertions(+), 31 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/ScrollManager.test.js b/packages/jaeger-ui/src/components/TracePage/ScrollManager.test.js
index 7f676d825f..34e6b37241 100644
--- a/packages/jaeger-ui/src/components/TracePage/ScrollManager.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/ScrollManager.test.js
@@ -21,7 +21,6 @@ import ScrollManager from './ScrollManager';
 const SPAN_HEIGHT = 2;
 
 function getTrace() {
-  let nextSpanID = 0;
   const spans = [];
   const trace = {
     spans,
@@ -29,7 +28,7 @@ function getTrace() {
     startTime: 1000,
   };
   for (let i = 0; i < 10; i++) {
-    spans.push({ duration: 1, startTime: 1000, spanID: nextSpanID++ });
+    spans.push({ duration: 1, startTime: 1000, spanID: i + 1 });
   }
   return trace;
 }
@@ -106,6 +105,9 @@ describe('ScrollManager', () => {
   });
 
   describe('_scrollToVisibleSpan()', () => {
+    function getRefs(spanID) {
+      return [{ refType: 'CHILD_OF', spanID }];
+    }
     let scrollPastMock;
 
     beforeEach(() => {
@@ -142,7 +144,7 @@ describe('ScrollManager', () => {
 
     it('skips spans that are out of view', () => {
       trace.spans[4].startTime = trace.startTime + trace.duration * 0.5;
-      accessors.getViewRange = jest.fn(() => [0.4, 0.6]);
+      accessors.getViewRange = () => [0.4, 0.6];
       accessors.getTopRowIndexVisible.mockReturnValue(trace.spans.length - 1);
       accessors.getBottomRowIndexVisible.mockReturnValue(0);
       manager._scrollToVisibleSpan(1);
@@ -154,18 +156,41 @@ describe('ScrollManager', () => {
     it('skips spans that do not match the text search', () => {
       accessors.getTopRowIndexVisible.mockReturnValue(trace.spans.length - 1);
       accessors.getBottomRowIndexVisible.mockReturnValue(0);
-      accessors.getSearchedSpanIDs = jest.fn(() => new Set([trace.spans[4].spanID]));
+      accessors.getSearchedSpanIDs = () => new Set([trace.spans[4].spanID]);
       manager._scrollToVisibleSpan(1);
       expect(scrollPastMock).lastCalledWith(4, 1);
       manager._scrollToVisibleSpan(-1);
       expect(scrollPastMock).lastCalledWith(4, -1);
     });
 
-    describe('scrollToNextVisibleSpan() and scrollToPrevVisibleSpan()', () => {
-      function getRefs(spanID) {
-        return [{ refType: 'CHILD_OF', spanID }];
-      }
+    it('scrolls to boundary when scrolling away from closest spanID in findMatches', () => {
+      const closetFindMatchesSpanID = 4;
+      accessors.getTopRowIndexVisible.mockReturnValue(closetFindMatchesSpanID - 1);
+      accessors.getBottomRowIndexVisible.mockReturnValue(closetFindMatchesSpanID + 1);
+      accessors.getSearchedSpanIDs = () => new Set([trace.spans[closetFindMatchesSpanID].spanID]);
+
+      manager._scrollToVisibleSpan(1);
+      expect(scrollPastMock).lastCalledWith(trace.spans.length - 1, 1);
+
+      manager._scrollToVisibleSpan(-1);
+      expect(scrollPastMock).lastCalledWith(0, -1);
+    });
+
+    it('scrolls to last visible row when boundary is hidden', () => {
+      const parentOfLastRowWithHiddenChildrenIndex = trace.spans.length - 2;
+      accessors.getBottomRowIndexVisible.mockReturnValue(0);
+      accessors.getCollapsedChildren = () =>
+        new Set([trace.spans[parentOfLastRowWithHiddenChildrenIndex].spanID]);
+      accessors.getSearchedSpanIDs = () => new Set([trace.spans[0].spanID]);
+      trace.spans[trace.spans.length - 1].references = getRefs(
+        trace.spans[parentOfLastRowWithHiddenChildrenIndex].spanID
+      );
+
+      manager._scrollToVisibleSpan(1);
+      expect(scrollPastMock).lastCalledWith(parentOfLastRowWithHiddenChildrenIndex, 1);
+    });
 
+    describe('scrollToNextVisibleSpan() and scrollToPrevVisibleSpan()', () => {
       beforeEach(() => {
         // change spans so 0 and 4 are top-level and their children are collapsed
         const spans = trace.spans;
@@ -213,6 +238,17 @@ describe('ScrollManager', () => {
         expect(scrollPastMock).lastCalledWith(4, -1);
       });
     });
+
+    describe('scrollToFirstVisibleSpan', () => {
+      beforeEach(() => {
+        jest.spyOn(manager, '_scrollToVisibleSpan').mockImplementationOnce();
+      });
+
+      it('calls _scrollToVisibleSpan searching downwards from first span', () => {
+        manager.scrollToFirstVisibleSpan();
+        expect(manager._scrollToVisibleSpan).toHaveBeenCalledWith(1, 0);
+      });
+    });
   });
 
   describe('scrollPageDown() and scrollPageUp()', () => {
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.test.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.test.js
index 0d5d1f9de7..26bfcea491 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.test.js
@@ -14,6 +14,7 @@
 
 import React from 'react';
 import { shallow, mount } from 'enzyme';
+import { Link } from 'react-router-dom';
 
 import AltViewOptions from './AltViewOptions';
 import KeyboardShortcutsHelp from './KeyboardShortcutsHelp';
@@ -110,6 +111,14 @@ describe('<TracePageHeader>', () => {
       expect(wrapper.find(AltViewOptions).length).toBe(0);
     });
 
+    it('renders the link to search', () => {
+      expect(wrapper.find(Link).length).toBe(0);
+
+      const toSearch = 'some-link';
+      wrapper.setProps({ toSearch });
+      expect(wrapper.find({ to: toSearch }).length).toBe(1);
+    });
+
     it('toggles the standalone link', () => {
       const linkToStandalone = 'some-link';
       const props = {
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js
index 4fcfea7cca..d5dc0c0d95 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.test.js
@@ -16,20 +16,20 @@ import React from 'react';
 import { shallow } from 'enzyme';
 
 import * as markers from './TracePageSearchBar.markers';
-import { TracePageSearchBarFn as TracePageSearchBar } from './TracePageSearchBar';
+import DefaultTracePageSearchBar, { TracePageSearchBarFn as TracePageSearchBar } from './TracePageSearchBar';
 import { trackFilter } from '../index.track';
 import UiFindInput from '../../common/UiFindInput';
 
-describe('<TracePageSearchBar>', () => {
-  const defaultProps = {
-    forwardedRef: React.createRef(),
-    navigable: true,
-    nextResult: () => {},
-    prevResult: () => {},
-    resultCount: 0,
-    textFilter: 'something',
-  };
+const defaultProps = {
+  forwardedRef: React.createRef(),
+  navigable: true,
+  nextResult: () => {},
+  prevResult: () => {},
+  resultCount: 0,
+  textFilter: 'something',
+};
 
+describe('<TracePageSearchBar>', () => {
   let wrapper;
 
   beforeEach(() => {
@@ -94,3 +94,12 @@ describe('<TracePageSearchBar>', () => {
     });
   });
 });
+
+describe('<DefaultTracePageSearchBar>', () => {
+  const { forwardedRef: ref, ...propsWithoutRef } = defaultProps;
+
+  it('forwardsRef correctly', () => {
+    const wrapper = shallow(<DefaultTracePageSearchBar {...propsWithoutRef} ref={ref} />);
+    expect(wrapper.find(TracePageSearchBar).props()).toEqual(defaultProps);
+  });
+});
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
index f4208cf979..3842861d5d 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
@@ -392,13 +392,18 @@ describe('TraceTimelineViewer/duck', () => {
   it('toggles a log item', () => {
     const logItem = 'hello-log-item';
     const id = trace.spans[0].spanID;
+    const secondID = trace.spans[1].spanID;
     const baseDetail = new DetailState();
     const toggledDetail = baseDetail.toggleLogItem(logItem);
 
     store.dispatch(actions.detailToggle(id));
+    store.dispatch(actions.detailToggle(secondID));
+    const secondDetail = store.getState().detailStates.get(secondID);
     expect(store.getState().detailStates.get(id)).toEqual(baseDetail);
+
     store.dispatch(actions.detailLogItemToggle(id, logItem));
     expect(store.getState().detailStates.get(id)).toEqual(toggledDetail);
+    expect(store.getState().detailStates.get(secondID)).toBe(secondDetail);
   });
 
   describe('hoverIndentGuideIds', () => {
diff --git a/packages/jaeger-ui/src/components/TracePage/index.test.js b/packages/jaeger-ui/src/components/TracePage/index.test.js
index 485552636f..60a71f4e7e 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/index.test.js
@@ -42,12 +42,14 @@ import * as track from './index.track';
 import ArchiveNotifier from './ArchiveNotifier';
 import { reset as resetShortcuts } from './keyboard-shortcuts';
 import { cancel as cancelScroll } from './scroll-page';
+import * as calculateTraceDagEV from './TraceGraph/calculateTraceDagEV';
 import SpanGraph from './TracePageHeader/SpanGraph';
 import TracePageHeader from './TracePageHeader';
 import { trackSlimHeaderToggle } from './TracePageHeader/TracePageHeader.track';
 import TraceTimelineViewer from './TraceTimelineViewer';
 import ErrorMessage from '../common/ErrorMessage';
 import LoadingIndicator from '../common/LoadingIndicator';
+import * as getUiFindVertexKeys from '../TraceDiff/TraceDiffGraph/traceDiffGraphUtils';
 import { fetchedState } from '../../constants';
 import traceGenerator from '../../demo/trace-generators';
 import transformTraceData from '../../model/transform-trace-data';
@@ -84,6 +86,7 @@ describe('<TracePage>', () => {
   const defaultProps = {
     acknowledgeArchive: () => {},
     fetchTrace() {},
+    focusUiFindMatches: jest.fn(),
     id: trace.traceID,
     history: {
       replace: () => {},
@@ -132,6 +135,32 @@ describe('<TracePage>', () => {
     });
   });
 
+  describe('focusUiFindMatches', () => {
+    let scrollToFirstVisibleSpanSpy;
+
+    beforeEach(() => {
+      scrollToFirstVisibleSpanSpy = jest.spyOn(wrapper.instance()._scrollManager, 'scrollToFirstVisibleSpan');
+      scrollToFirstVisibleSpanSpy.mockReset();
+      defaultProps.focusUiFindMatches.mockReset();
+    });
+
+    it('calls _scrollManager.scrollToFirstVisibleSpan and props.focusUiFindMatches with props.trace.data and uiFind when props.trace.data is present', () => {
+      const uiFind = 'test ui find';
+      wrapper.setProps({ uiFind });
+      wrapper.find(TracePageHeader).prop('focusUiFindMatches')();
+      expect(defaultProps.focusUiFindMatches).toHaveBeenCalledWith(defaultProps.trace.data, uiFind);
+      expect(scrollToFirstVisibleSpanSpy).toHaveBeenCalled();
+    });
+
+    it('handles when props.trace.data is absent', () => {
+      const propFn = wrapper.find(TracePageHeader).prop('focusUiFindMatches');
+      wrapper.setProps({ trace: {} });
+      propFn();
+      expect(defaultProps.focusUiFindMatches).not.toHaveBeenCalled();
+      expect(scrollToFirstVisibleSpanSpy).not.toHaveBeenCalled();
+    });
+  });
+
   it('uses props.uiFind, props.trace.traceID, and props.trace.spans.length to create filterSpans memo cache key', () => {
     expect(filterSpansSpy).toHaveBeenCalledTimes(0);
 
@@ -331,7 +360,17 @@ describe('<TracePage>', () => {
     });
 
     describe('resultCount', () => {
-      it('is the size of findMatchesIDs when available', () => {
+      let getUiFindVertexKeysSpy;
+
+      beforeAll(() => {
+        getUiFindVertexKeysSpy = jest.spyOn(getUiFindVertexKeys, 'getUiFindVertexKeys');
+      });
+
+      beforeEach(() => {
+        getUiFindVertexKeysSpy.mockReset();
+      });
+
+      it('is the size of spanFindMatches when available', () => {
         expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(0);
 
         const size = 20;
@@ -340,9 +379,26 @@ describe('<TracePage>', () => {
         expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(size);
       });
 
+      it('is the size of graphFindMatches when available', () => {
+        expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(0);
+
+        const size = 30;
+        getUiFindVertexKeysSpy.mockReturnValueOnce({ size });
+        wrapper.setState({ traceGraphView: true });
+        wrapper.setProps({ uiFind: 'new ui find to bust memo' });
+        expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(size);
+      });
+
       it('defaults to 0', () => {
+        // falsy uiFind for base case
+        wrapper.setProps({ uiFind: '' });
+        expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(0);
+
         filterSpansSpy.mockReturnValueOnce(null);
-        wrapper.setProps({ uiFind: 'new ui find to bust memo' });
+        wrapper.setProps({ uiFind: 'truthy uiFind' });
+        expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(0);
+
+        wrapper.setState({ traceGraphView: true });
         expect(wrapper.find(TracePageHeader).prop('resultCount')).toBe(0);
       });
     });
@@ -458,6 +514,7 @@ describe('<TracePage>', () => {
     let header;
     let spanGraph;
     let timeline;
+    let calculateTraceDagEVSpy;
 
     function refreshWrappers() {
       header = wrapper.find(TracePageHeader);
@@ -465,6 +522,10 @@ describe('<TracePage>', () => {
       timeline = wrapper.find(TraceTimelineViewer);
     }
 
+    beforeAll(() => {
+      calculateTraceDagEVSpy = jest.spyOn(calculateTraceDagEV, 'default');
+    });
+
     beforeEach(() => {
       wrapper = mount(<TracePage {...defaultProps} />);
       // use the method directly because it is a `ref` prop
@@ -490,6 +551,15 @@ describe('<TracePage>', () => {
       expect(sections.length).toBe(0);
     });
 
+    it('initializes slimView correctly', () => {
+      expect(wrapper.state('slimView')).toBe(false);
+      // Empty trace avoids this spec from evaluating TracePageHeader's consumption of slimView
+      wrapper = mount(
+        <TracePage {...defaultProps} trace={{}} embedded={{ timeline: { collapseTitle: true } }} />
+      );
+      expect(wrapper.state('slimView')).toBe(true);
+    });
+
     it('propagates slimView changes', () => {
       const { onSlimViewClicked } = header.props();
       expect(header.prop('slimView')).toBe(false);
@@ -516,6 +586,11 @@ describe('<TracePage>', () => {
       wrapper.update();
       refreshWrappers();
       expect(header.prop('traceGraphView')).toBe(true);
+      expect(calculateTraceDagEVSpy).toHaveBeenCalledWith(defaultProps.trace.data);
+
+      wrapper.setProps({ trace: {} });
+      onTraceGraphViewClicked();
+      expect(calculateTraceDagEVSpy).toHaveBeenCalledTimes(1);
     });
 
     it('propagates viewRange changes', () => {
diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 290c6b32a9..22a50fc124 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -182,10 +182,8 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
   }
 
   componentWillReceiveProps(nextProps: TProps) {
-    if (this._scrollManager) {
-      const { trace } = nextProps;
-      this._scrollManager.setTrace(trace && trace.data);
-    }
+    const { trace } = nextProps;
+    this._scrollManager.setTrace(trace && trace.data);
   }
 
   componentDidUpdate({ id: prevID }: TProps) {
@@ -204,13 +202,11 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
   componentWillUnmount() {
     resetShortcuts();
     cancelScroll();
-    if (this._scrollManager) {
-      this._scrollManager.destroy();
-      this._scrollManager = new ScrollManager(undefined, {
-        scrollBy,
-        scrollTo,
-      });
-    }
+    this._scrollManager.destroy();
+    this._scrollManager = new ScrollManager(undefined, {
+      scrollBy,
+      scrollTo,
+    });
   }
 
   _adjustViewRange(startChange: number, endChange: number, trackSrc: string) {

From 8a268c2c307d8419434b599c6a9a2dd14dbaaaa2 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Mon, 15 Apr 2019 15:18:11 -0400
Subject: [PATCH 07/13] Improve TracePageSearchBar styling, choose icons

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../TracePageHeader/TracePageHeader.tsx       |  3 +--
 .../TracePageHeader/TracePageSearchBar.css    | 13 ++++++++--
 .../TracePageHeader/TracePageSearchBar.tsx    | 25 ++++++++++---------
 .../TraceTimelineViewer/SpanDetail/index.tsx  |  8 +++---
 4 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
index 5d26ebc4e5..59e185972c 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
@@ -163,7 +163,6 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
         ) : (
           title
         )}
-        {showShortcutsHelp && <KeyboardShortcutsHelp className="ub-mr2" />}
         <TracePageSearchBar
           clearSearch={clearSearch}
           focusUiFindMatches={focusUiFindMatches}
@@ -174,7 +173,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
           textFilter={textFilter}
           navigable={!traceGraphView}
         />
-
+        {showShortcutsHelp && <KeyboardShortcutsHelp className="ub-m2" />}
         {showViewOptions && (
           <AltViewOptions
             onTraceGraphViewClicked={onTraceGraphViewClicked}
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
index 0b5622db6a..044b01193a 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
@@ -15,11 +15,19 @@ limitations under the License.
 */
 
 .TracePageSearchBar {
-  max-width: 20rem;
+  width: 40%;
+}
+
+.TracePageSearchBar--InputGroup {
+  justify-content: flex-end;
 }
 
 .TracePageSearchBar--bar {
-  display: flex;
+  max-width: 100%;
+  transition: max-width 1.5s;
+}
+
+.TracePageSearchBar--bar:not(:focus-within) {
   max-width: 20rem;
 }
 
@@ -28,6 +36,7 @@ limitations under the License.
 }
 
 .TracePageSearchBar--btn {
+  border-left: none;
   transition: 0.2s;
 }
 
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
index 0da70d0ad8..b5a7aee93f 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
@@ -15,6 +15,7 @@
 import * as React from 'react';
 import { Button, Input } from 'antd';
 import cx from 'classnames';
+import IoAndroidLocate from 'react-icons/lib/io/android-locate';
 
 import * as markers from './TracePageSearchBar.markers';
 import { trackFilter } from '../index.track';
@@ -47,7 +48,6 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
 
   const count = textFilter ? <span className="TracePageSearchBar--count">{resultCount}</span> : null;
 
-  const navigationBtnClass = cx('TracePageSearchBar--btn', { 'is-disabled': !textFilter });
   const btnClass = cx('TracePageSearchBar--btn', { 'is-disabled': !textFilter });
   const uiFindInputInputProps = {
     'data-test': markers.IN_TRACE_SEARCH,
@@ -57,9 +57,9 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
   };
 
   return (
-    <div className="ub-flex-auto ub-mx2 TracePageSearchBar">
+    <div className="TracePageSearchBar">
       {/* style inline because compact overwrites the display */}
-      <Input.Group compact style={{ display: 'flex' }}>
+      <Input.Group className="TracePageSearchBar--InputGroup" compact style={{ display: 'flex' }}>
         <UiFindInput
           inputProps={uiFindInputInputProps}
           forwardedRef={forwardedRef}
@@ -68,26 +68,27 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
         {navigable && (
           <>
             <Button
-              className={navigationBtnClass}
+              className={cx(btnClass, 'ant-btn-icon-only')}
+              disabled={!textFilter}
+              htmlType="button"
+              onClick={focusUiFindMatches}
+            >
+              <IoAndroidLocate />
+            </Button>
+            <Button
+              className={btnClass}
               disabled={!textFilter}
               htmlType="button"
               icon="up"
               onClick={prevResult}
             />
             <Button
-              className={navigationBtnClass}
+              className={btnClass}
               disabled={!textFilter}
               htmlType="button"
               icon="down"
               onClick={nextResult}
             />
-            <Button
-              className={navigationBtnClass}
-              disabled={!textFilter}
-              htmlType="button"
-              icon="search"
-              onClick={focusUiFindMatches}
-            />
           </>
         )}
         <Button
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
index b95bbe573f..c949fc1d3c 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
@@ -14,6 +14,7 @@
 
 import React from 'react';
 import { Button, Divider, Tooltip } from 'antd';
+import ZoomIn from 'react-icons/lib/ti/zoom-in';
 
 import AccordianKeyValues from './AccordianKeyValues';
 import AccordianLogs from './AccordianLogs';
@@ -117,11 +118,12 @@ export default function SpanDetail(props: SpanDetailProps) {
           <span className="SpanDetail--debugLabel" data-label="SpanID:" /> {spanID}
           <Tooltip placement="topRight" title="Click to add to filter for deep linking">
             <Button
-              className="SpanDetail--debugValue"
+              className="SpanDetail--debugValue ant-btn-icon-only"
               htmlType="button"
-              icon="down"
               onClick={() => addToUiFind(spanID)}
-            />
+            >
+              <ZoomIn size="1.4em" />
+            </Button>
           </Tooltip>
         </small>
       </div>

From 5a146ef5e8631addc97092245e0a8fd1ffe1a574 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Tue, 23 Apr 2019 18:18:47 -0400
Subject: [PATCH 08/13] Move scroll indicator from set/map to standalone

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../components/TracePage/ScrollManager.tsx    | 11 +++-
 .../TracePageHeader/TracePageSearchBar.css    | 12 +++--
 .../TracePageHeader/TracePageSearchBar.tsx    |  2 +-
 .../VirtualizedTraceView.test.js              | 50 ++++++++++++-------
 .../VirtualizedTraceView.tsx                  | 37 ++++++++++----
 .../TraceTimelineViewer/duck.test.js          | 49 ++++++++++++------
 .../TracePage/TraceTimelineViewer/duck.tsx    | 22 ++++++--
 .../jaeger-ui/src/types/TTraceTimeline.tsx    |  5 +-
 8 files changed, 132 insertions(+), 56 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
index b975555f3b..807a0f25a5 100644
--- a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx
@@ -131,8 +131,15 @@ export default class ScrollManager {
     }
     const { duration, spans, startTime: traceStartTime } = this._trace;
     const isUp = direction < 0;
-    const boundaryRow = isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible();
-    const spanIndex = xrs.mapRowIndexToSpanIndex(startRow != null ? startRow : boundaryRow);
+    let boundaryRow: number;
+    if (startRow != null) {
+      boundaryRow = startRow;
+    } else if (isUp) {
+      boundaryRow = xrs.getTopRowIndexVisible();
+    } else {
+      boundaryRow = xrs.getBottomRowIndexVisible();
+    }
+    const spanIndex = xrs.mapRowIndexToSpanIndex(boundaryRow);
     if ((spanIndex === 0 && isUp) || (spanIndex === spans.length - 1 && !isUp)) {
       return;
     }
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
index 044b01193a..703e81f7a2 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
@@ -23,12 +23,12 @@ limitations under the License.
 }
 
 .TracePageSearchBar--bar {
-  max-width: 100%;
-  transition: max-width 1.5s;
+  max-width: 20rem;
+  transition: max-width 0.5s;
 }
 
-.TracePageSearchBar--bar:not(:focus-within) {
-  max-width: 20rem;
+.TracePageSearchBar--bar:focus-within {
+  max-width: 100%;
 }
 
 .TracePageSearchBar--count {
@@ -43,3 +43,7 @@ limitations under the License.
 .TracePageSearchBar--btn.is-disabled {
   opacity: 0.5;
 }
+
+.TracePageSearchBar--btn.locate-btn {
+  padding: 1px 8px 4px;
+}
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
index b5a7aee93f..bf88749497 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
@@ -68,7 +68,7 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
         {navigable && (
           <>
             <Button
-              className={cx(btnClass, 'ant-btn-icon-only')}
+              className={cx(btnClass, 'locate-btn')}
               disabled={!textFilter}
               htmlType="button"
               onClick={focusUiFindMatches}
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
index 910f0b1d4b..e70371e6a1 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
@@ -32,6 +32,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
   const props = {
     childrenHiddenIDs: new Set(),
     childrenToggle: jest.fn(),
+    clearShouldScrollToFirstUiFindMatch: jest.fn(),
     currentViewRangeTime: [0.25, 0.75],
     detailLogItemToggle: jest.fn(),
     detailLogsToggle: jest.fn(),
@@ -44,6 +45,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
     scrollToFirstVisibleSpan: jest.fn(),
     setSpanNameColumnWidth: jest.fn(),
     setTrace: jest.fn(),
+    shouldScrollToFirstUiFindMatch: false,
     spanNameColumnWidth: 0.5,
     trace,
     uiFind: 'uiFind',
@@ -345,32 +347,46 @@ describe('<VirtualizedTraceViewImpl>', () => {
     });
   });
 
-  describe('handles needToScroll', () => {
+  describe('shouldScrollToFirstUiFindMatch', () => {
+    const propsWithTruthShouldScrollToFirstUiFindMatch = { ...props, shouldScrollToFirstUiFindMatch: true };
+
     beforeEach(() => {
       props.scrollToFirstVisibleSpan.mockReset();
+      props.clearShouldScrollToFirstUiFindMatch.mockReset();
     });
 
-    [[Set, 'childrenHiddenIDs'], [Map, 'detailStates']].forEach(([ConstructorFn, propName]) => {
-      it(`calls props.scrollToFirstVisibleSpan if ${propName} changes and ${propName}.needToScroll is true`, () => {
-        expect(props.scrollToFirstVisibleSpan).not.toHaveBeenCalled();
+    it('calls props.scrollToFirstVisibleSpan if shouldScrollToFirstUiFindMatch is true', () => {
+      expect(props.scrollToFirstVisibleSpan).not.toHaveBeenCalled();
+      expect(props.clearShouldScrollToFirstUiFindMatch).not.toHaveBeenCalled();
 
-        const collectionThatNeedsToScroll = new ConstructorFn();
-        collectionThatNeedsToScroll.needToScroll = true;
+      wrapper.setProps(propsWithTruthShouldScrollToFirstUiFindMatch);
+      expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1);
+      expect(props.clearShouldScrollToFirstUiFindMatch).toHaveBeenCalledTimes(1);
+    });
 
-        wrapper.setProps({ [propName]: collectionThatNeedsToScroll });
-        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1);
+    describe('shouldComponentUpdate', () => {
+      it('returns true if props.shouldScrollToFirstUiFindMatch is unchanged', () => {
+        expect(wrapper.instance().shouldComponentUpdate(props)).toBe(true);
+      });
 
-        wrapper.setProps({ [propName]: collectionThatNeedsToScroll });
-        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1);
+      it('returns true if props.shouldScrollToFirstUiFindMatch changes to true', () => {
+        expect(wrapper.instance().shouldComponentUpdate(propsWithTruthShouldScrollToFirstUiFindMatch)).toBe(
+          true
+        );
+      });
 
-        const differentCollectionThatAlsoNeedsToScroll = new ConstructorFn();
-        differentCollectionThatAlsoNeedsToScroll.needToScroll = true;
-        wrapper.setProps({ [propName]: differentCollectionThatAlsoNeedsToScroll });
-        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(2);
+      it('returns true if props.shouldScrollToFirstUiFindMatch changes to false and another props change', () => {
+        const propsWithOtherDifferenceAndTrueshouldScrollToFirstUiFindMatch = {
+          ...propsWithTruthShouldScrollToFirstUiFindMatch,
+          clearShouldScrollToFirstUiFindMatch: () => {},
+        };
+        wrapper.setProps(propsWithOtherDifferenceAndTrueshouldScrollToFirstUiFindMatch);
+        expect(wrapper.instance().shouldComponentUpdate(props)).toBe(true);
+      });
 
-        const differentCollectionThatDoesNotNeedToScroll = new ConstructorFn();
-        wrapper.setProps({ [propName]: differentCollectionThatDoesNotNeedToScroll });
-        expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(2);
+      it('returns false if props.shouldScrollToFirstUiFindMatch changes to false and no other props change', () => {
+        wrapper.setProps(propsWithTruthShouldScrollToFirstUiFindMatch);
+        expect(wrapper.instance().shouldComponentUpdate(props)).toBe(false);
       });
     });
   });
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index f5e93fef11..7da5319438 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -59,6 +59,7 @@ type TVirtualizedTraceViewOwnProps = {
 
 type TDispatchProps = {
   childrenToggle: (spanID: string) => void;
+  clearShouldScrollToFirstUiFindMatch: () => void;
   detailLogItemToggle: (spanID: string, log: Log) => void;
   detailLogsToggle: (spanID: string) => void;
   detailProcessToggle: (spanID: string) => void;
@@ -135,6 +136,7 @@ function getCssClasses(currentViewRange: [number, number]) {
 }
 
 // export from tests
+// eslint-disable-next-line react/no-redundant-should-component-update
 export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTraceViewProps> {
   clippingCssClasses: string;
   listView: ListView | TNil;
@@ -159,6 +161,25 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     setTrace(trace, uiFind);
   }
 
+  shouldComponentUpdate(nextProps: VirtualizedTraceViewProps) {
+    if (
+      !nextProps.shouldScrollToFirstUiFindMatch &&
+      nextProps.shouldScrollToFirstUiFindMatch !== this.props.shouldScrollToFirstUiFindMatch
+    ) {
+      const nextPropKeys = Object.keys(nextProps);
+      for (let i = 0; i < nextPropKeys.length; i += 1) {
+        if (
+          nextProps[nextPropKeys[i] as keyof VirtualizedTraceViewProps] !==
+            this.props[nextPropKeys[i] as keyof VirtualizedTraceViewProps] &&
+          nextPropKeys[i] !== 'shouldScrollToFirstUiFindMatch'
+        )
+          return true;
+      }
+      return false;
+    }
+    return true;
+  }
+
   componentWillUpdate(nextProps: VirtualizedTraceViewProps) {
     const { childrenHiddenIDs, detailStates, registerAccessors, trace, currentViewRangeTime } = this.props;
     const {
@@ -191,19 +212,15 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     }
   }
 
-  componentDidUpdate(prevProps: VirtualizedTraceViewProps) {
+  componentDidUpdate() {
     const {
-      detailStates: currDetailStates,
-      childrenHiddenIDs: currChildrenHiddenIds,
+      shouldScrollToFirstUiFindMatch,
+      clearShouldScrollToFirstUiFindMatch,
       scrollToFirstVisibleSpan,
     } = this.props;
-    const { detailStates: prevDetailStates, childrenHiddenIDs: prevChildrenHiddenIds } = prevProps;
-
-    if (
-      (currChildrenHiddenIds.needToScroll && currChildrenHiddenIds !== prevChildrenHiddenIds) ||
-      (currDetailStates.needToScroll && currDetailStates !== prevDetailStates)
-    ) {
+    if (shouldScrollToFirstUiFindMatch) {
       scrollToFirstVisibleSpan();
+      clearShouldScrollToFirstUiFindMatch();
     }
   }
 
@@ -431,7 +448,7 @@ function mapStateToProps(state: ReduxState): TTraceTimeline & TExtractUiFindFrom
 
 /* istanbul ignore next */
 function mapDispatchToProps(dispatch: Dispatch<ReduxState>): TDispatchProps {
-  return bindActionCreators(actions, dispatch) as any;
+  return (bindActionCreators(actions, dispatch) as any) as TDispatchProps;
 }
 
 export default withRouter(
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
index 3842861d5d..f25066d929 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js
@@ -111,15 +111,13 @@ describe('TraceTimelineViewer/duck', () => {
     });
 
     it('indicates the need to scroll iff there are uiFindMatches', () => {
-      expect(state.detailStates.needToScroll).toBe(true);
-      expect(state.childrenHiddenIDs.needToScroll).toBe(true);
+      expect(state.shouldScrollToFirstUiFindMatch).toBe(true);
 
       filterSpansSpy.mockReturnValue(new Set());
       const singleSpecStore = createStore(reducer, newInitialState());
       singleSpecStore.dispatch(action);
       const singleSpecState = singleSpecStore.getState();
-      expect(singleSpecState.detailStates.needToScroll).toBeUndefined();
-      expect(singleSpecState.childrenHiddenIDs.needToScroll).toBeUndefined();
+      expect(singleSpecState.shouldScrollToFirstUiFindMatch).toBe(false);
     });
 
     it('returns existing state if uiFind is falsy', () => {
@@ -138,6 +136,10 @@ describe('TraceTimelineViewer/duck', () => {
   });
 
   describe('setTrace', () => {
+    beforeEach(() => {
+      filterSpansSpy.mockClear();
+    });
+
     const setTraceAction = actions.setTrace(trace);
 
     it('retains all state when setting to the same traceID', () => {
@@ -175,22 +177,37 @@ describe('TraceTimelineViewer/duck', () => {
 
     it('calls calculateHiddenIdsAndDetailStates iff a truthy uiFind is provided', () => {
       store.dispatch(setTraceAction);
-      let state = store.getState();
-      expect(state.childrenHiddenIDs).toEqual(new Set());
+      expect(filterSpansSpy).not.toHaveBeenCalled();
 
-      store.dispatch(actions.setTrace(trace, null));
-      state = store.getState();
-      expect(state.childrenHiddenIDs).toEqual(new Set());
+      store.dispatch(actions.setTrace(Object.assign({}, trace, { traceID: `${trace.traceID}_1` }), null));
+      expect(filterSpansSpy).not.toHaveBeenCalled();
 
-      store.dispatch(actions.setTrace(trace, undefined));
-      state = store.getState();
-      expect(state.childrenHiddenIDs).toEqual(new Set());
+      store.dispatch(actions.setTrace(Object.assign({}, trace, { traceID: `${trace.traceID}_2` }), ''));
+      expect(filterSpansSpy).not.toHaveBeenCalled();
 
-      store.dispatch(actions.setTrace(trace, 'truthy uiFind string'));
-      state = store.getState();
-      expect(state.childrenHiddenIDs).toEqual(new Set());
+      store.dispatch(
+        actions.setTrace(Object.assign({}, trace, { traceID: `${trace.traceID}_3` }), 'truthy uiFind string')
+      );
+      expect(filterSpansSpy).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('clearShouldScrollToFirstUiFindMatch', () => {
+    const clearShouldScrollToFirstUiFindMatchAction = actions.clearShouldScrollToFirstUiFindMatch();
+
+    it('returns existing state if current state does not indicate need to scroll', () => {
+      const state = store.getState();
+      store.dispatch(clearShouldScrollToFirstUiFindMatchAction);
+      expect(store.getState()).toBe(state);
+    });
+
+    it('sets state.shouldScrollToFirstUiFindMatch to false if it is currently true', () => {
+      const state = store.getState();
+      state.shouldScrollToFirstUiFindMatch = true;
+      expect(store.getState().shouldScrollToFirstUiFindMatch).toBe(true);
+      store.dispatch(clearShouldScrollToFirstUiFindMatchAction);
+      expect(store.getState().shouldScrollToFirstUiFindMatch).toBe(false);
     });
-    // describe('calls focus sometmisth', () => {
   });
 
   describe('toggles children and details', () => {
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
index ca09021b07..eeee8faf16 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
@@ -51,6 +51,7 @@ export function newInitialState(): TTraceTimeline {
     childrenHiddenIDs: new Set(),
     detailStates: new Map(),
     hoverIndentGuideIds: new Set(),
+    shouldScrollToFirstUiFindMatch: false,
     spanNameColumnWidth: 0.25,
     traceID: null,
   };
@@ -59,6 +60,7 @@ export function newInitialState(): TTraceTimeline {
 export const actionTypes = generateActionTypes('@jaeger-ui/trace-timeline-viewer', [
   'ADD_HOVER_INDENT_GUIDE_ID',
   'CHILDREN_TOGGLE',
+  'CLEAR_SHOULD_SCROLL_TO_FIRST_UI_FIND_MATCH',
   'COLLAPSE_ALL',
   'COLLAPSE_ONE',
   'DETAIL_TOGGLE',
@@ -77,6 +79,7 @@ export const actionTypes = generateActionTypes('@jaeger-ui/trace-timeline-viewer
 const fullActions = createActions<TActionTypes>({
   [actionTypes.ADD_HOVER_INDENT_GUIDE_ID]: (spanID: string) => ({ spanID }),
   [actionTypes.CHILDREN_TOGGLE]: (spanID: string) => ({ spanID }),
+  [actionTypes.CLEAR_SHOULD_SCROLL_TO_FIRST_UI_FIND_MATCH]: () => ({}),
   [actionTypes.COLLAPSE_ALL]: (spans: Span[]) => ({ spans }),
   [actionTypes.COLLAPSE_ONE]: (spans: Span[]) => ({ spans }),
   [actionTypes.DETAIL_LOG_ITEM_TOGGLE]: (spanID: string, logItem: Log) => ({ logItem, spanID }),
@@ -96,8 +99,9 @@ export const actions = (fullActions as any).jaegerUi.traceTimelineViewer as TTim
 
 function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
   const spansMap = new Map();
-  const childrenHiddenIDs: Set<string> & { needToScroll?: true } = new Set();
-  const detailStates: Map<string, DetailState> & { needToScroll?: true } = new Map();
+  const childrenHiddenIDs: Set<string> = new Set();
+  const detailStates: Map<string, DetailState> = new Map();
+  let shouldScrollToFirstUiFindMatch: boolean = false;
 
   spans.forEach(span => {
     spansMap.set(span.spanID, span);
@@ -110,12 +114,12 @@ function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
       detailStates.set(spanID, new DetailState());
       spanAncestorIds(span).forEach(ancestorID => childrenHiddenIDs.delete(ancestorID));
     });
-    childrenHiddenIDs.needToScroll = true;
-    detailStates.needToScroll = true;
+    shouldScrollToFirstUiFindMatch = true;
   }
   return {
     childrenHiddenIDs,
     detailStates,
+    shouldScrollToFirstUiFindMatch,
   };
 }
 
@@ -127,6 +131,13 @@ function focusUiFindMatches(state: TTraceTimeline, { uiFind, trace }: TTraceUiFi
   };
 }
 
+function clearShouldScrollToFirstUiFindMatch(state: TTraceTimeline) {
+  if (state.shouldScrollToFirstUiFindMatch) {
+    return { ...state, shouldScrollToFirstUiFindMatch: false };
+  }
+  return state;
+}
+
 function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
   const { traceID, spans } = trace;
   if (traceID === state.traceID) {
@@ -280,6 +291,9 @@ export default handleActions(
   {
     [actionTypes.ADD_HOVER_INDENT_GUIDE_ID]: guardReducer(addHoverIndentGuideId),
     [actionTypes.CHILDREN_TOGGLE]: guardReducer(childrenToggle),
+    [actionTypes.CLEAR_SHOULD_SCROLL_TO_FIRST_UI_FIND_MATCH]: guardReducer(
+      clearShouldScrollToFirstUiFindMatch
+    ),
     [actionTypes.COLLAPSE_ALL]: guardReducer(collapseAll),
     [actionTypes.COLLAPSE_ONE]: guardReducer(collapseOne),
     [actionTypes.DETAIL_LOGS_TOGGLE]: guardReducer(detailLogsToggle),
diff --git a/packages/jaeger-ui/src/types/TTraceTimeline.tsx b/packages/jaeger-ui/src/types/TTraceTimeline.tsx
index 55c173c01e..6ed8db47f5 100644
--- a/packages/jaeger-ui/src/types/TTraceTimeline.tsx
+++ b/packages/jaeger-ui/src/types/TTraceTimeline.tsx
@@ -16,9 +16,10 @@ import DetailState from '../components/TracePage/TraceTimelineViewer/SpanDetail/
 import TNil from './TNil';
 
 type TTraceTimeline = {
-  childrenHiddenIDs: Set<string> & { needToScroll?: true };
-  detailStates: Map<string, DetailState> & { needToScroll?: true };
+  childrenHiddenIDs: Set<string>;
+  detailStates: Map<string, DetailState>;
   hoverIndentGuideIds: Set<string>;
+  shouldScrollToFirstUiFindMatch: boolean;
   spanNameColumnWidth: number;
   traceID: string | TNil;
 };

From 43a8ca5ab36f5092961d28a18db7d422b6cc4998 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Wed, 24 Apr 2019 11:48:32 -0400
Subject: [PATCH 09/13] Update CopyIcon to support spanID deep link

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../__snapshots__/drawNode.test.js.snap       | 24 +++++++++++++++++++
 .../SpanDetail/index.test.js                  | 15 ++++++------
 .../TraceTimelineViewer/SpanDetail/index.tsx  | 22 +++++++----------
 .../TraceTimelineViewer/SpanDetailRow.tsx     |  3 ---
 .../VirtualizedTraceView.tsx                  | 13 ----------
 .../src/components/common/CopyIcon.test.js    |  6 ++---
 .../src/components/common/CopyIcon.tsx        | 17 ++++++++++---
 .../__snapshots__/CopyIcon.test.js.snap       |  9 +++++--
 8 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/drawNode.test.js.snap b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/drawNode.test.js.snap
index 7b8d1ccb15..66e599f8da 100644
--- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/drawNode.test.js.snap
+++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/drawNode.test.js.snap
@@ -25,6 +25,8 @@ exports[`drawNode diffNode renders as expected when props.a and props.b are the
             <CopyIcon
               className="DiffNode--copyIcon"
               copyText="serviceName operationName"
+              icon="copy"
+              placement="left"
               tooltipTitle="Copy label"
             />
           </td>
@@ -70,6 +72,8 @@ exports[`drawNode diffNode renders as expected when props.a and props.b are the
           <CopyIcon
             className="DiffNode--copyIcon"
             copyText="serviceName operationName"
+            icon="copy"
+            placement="left"
             tooltipTitle="Copy label"
           />
         </td>
@@ -116,6 +120,8 @@ exports[`drawNode diffNode renders as expected when props.a is 0 1`] = `
             <CopyIcon
               className="DiffNode--copyIcon"
               copyText="serviceName operationName"
+              icon="copy"
+              placement="left"
               tooltipTitle="Copy label"
             />
           </td>
@@ -181,6 +187,8 @@ exports[`drawNode diffNode renders as expected when props.a is 0 1`] = `
           <CopyIcon
             className="DiffNode--copyIcon"
             copyText="serviceName operationName"
+            icon="copy"
+            placement="left"
             tooltipTitle="Copy label"
           />
         </td>
@@ -242,6 +250,8 @@ exports[`drawNode diffNode renders as expected when props.a is less than props.b
             <CopyIcon
               className="DiffNode--copyIcon"
               copyText="serviceName operationName"
+              icon="copy"
+              placement="left"
               tooltipTitle="Copy label"
             />
           </td>
@@ -307,6 +317,8 @@ exports[`drawNode diffNode renders as expected when props.a is less than props.b
           <CopyIcon
             className="DiffNode--copyIcon"
             copyText="serviceName operationName"
+            icon="copy"
+            placement="left"
             tooltipTitle="Copy label"
           />
         </td>
@@ -368,6 +380,8 @@ exports[`drawNode diffNode renders as expected when props.a is more than props.b
             <CopyIcon
               className="DiffNode--copyIcon"
               copyText="serviceName operationName"
+              icon="copy"
+              placement="left"
               tooltipTitle="Copy label"
             />
           </td>
@@ -433,6 +447,8 @@ exports[`drawNode diffNode renders as expected when props.a is more than props.b
           <CopyIcon
             className="DiffNode--copyIcon"
             copyText="serviceName operationName"
+            icon="copy"
+            placement="left"
             tooltipTitle="Copy label"
           />
         </td>
@@ -494,6 +510,8 @@ exports[`drawNode diffNode renders as expected when props.b is 0 1`] = `
             <CopyIcon
               className="DiffNode--copyIcon"
               copyText="serviceName operationName"
+              icon="copy"
+              placement="left"
               tooltipTitle="Copy label"
             />
           </td>
@@ -559,6 +577,8 @@ exports[`drawNode diffNode renders as expected when props.b is 0 1`] = `
           <CopyIcon
             className="DiffNode--copyIcon"
             copyText="serviceName operationName"
+            icon="copy"
+            placement="left"
             tooltipTitle="Copy label"
           />
         </td>
@@ -615,6 +635,8 @@ exports[`drawNode diffNode renders as expected when props.isUiFindMatch is true
             <CopyIcon
               className="DiffNode--copyIcon"
               copyText="serviceName operationName"
+              icon="copy"
+              placement="left"
               tooltipTitle="Copy label"
             />
           </td>
@@ -660,6 +682,8 @@ exports[`drawNode diffNode renders as expected when props.isUiFindMatch is true
           <CopyIcon
             className="DiffNode--copyIcon"
             copyText="serviceName operationName"
+            icon="copy"
+            placement="left"
             tooltipTitle="Copy label"
           />
         </td>
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
index fa5e822fd8..3af5e7907a 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
@@ -16,7 +16,6 @@
 jest.mock('../utils');
 
 import React from 'react';
-import { Button } from 'antd';
 import { shallow } from 'enzyme';
 
 import AccordianKeyValues from './AccordianKeyValues';
@@ -24,6 +23,7 @@ import AccordianLogs from './AccordianLogs';
 import DetailState from './DetailState';
 import SpanDetail from './index';
 import { formatDuration } from '../utils';
+import CopyIcon from '../../../common/CopyIcon';
 import LabeledList from '../../../common/LabeledList';
 import traceGenerator from '../../../../demo/trace-generators';
 import transformTraceData from '../../../../model/transform-trace-data';
@@ -39,7 +39,6 @@ describe('<SpanDetail>', () => {
     .toggleTags();
   const traceStartTime = 5;
   const props = {
-    addToUiFind: jest.fn(),
     detailState,
     span,
     traceStartTime,
@@ -65,7 +64,6 @@ describe('<SpanDetail>', () => {
     props.processToggle.mockReset();
     props.logsToggle.mockReset();
     props.logItemToggle.mockReset();
-    props.addToUiFind.mockReset();
     wrapper = shallow(<SpanDetail {...props} />);
   });
 
@@ -122,9 +120,12 @@ describe('<SpanDetail>', () => {
     expect(props.logItemToggle).toHaveBeenLastCalledWith(span.spanID, somethingUniq);
   });
 
-  it('calls addToUiFind when the spanID is clicked', () => {
-    const spanIDButton = wrapper.find(Button);
-    spanIDButton.simulate('click');
-    expect(props.addToUiFind).toHaveBeenCalledWith(props.span.spanID);
+  it('renders CopyIcon with deep link URL', () => {
+    expect(
+      wrapper
+        .find(CopyIcon)
+        .prop('copyText')
+        .includes(`?uiFind=${props.span.spanID}`)
+    ).toBe(true);
   });
 });
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
index c949fc1d3c..64a1cd637c 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
@@ -13,13 +13,13 @@
 // limitations under the License.
 
 import React from 'react';
-import { Button, Divider, Tooltip } from 'antd';
-import ZoomIn from 'react-icons/lib/ti/zoom-in';
+import { Divider } from 'antd';
 
 import AccordianKeyValues from './AccordianKeyValues';
 import AccordianLogs from './AccordianLogs';
 import DetailState from './DetailState';
 import { formatDuration } from '../utils';
+import CopyIcon from '../../../common/CopyIcon';
 import LabeledList from '../../../common/LabeledList';
 
 import { TNil } from '../../../../types';
@@ -28,7 +28,6 @@ import { Log, Span, KeyValuePair, Link } from '../../../../types/trace';
 import './index.css';
 
 type SpanDetailProps = {
-  addToUiFind: (spanID: string) => void;
   detailState: DetailState;
   linksGetter: ((links: KeyValuePair[], index: number) => Link[]) | TNil;
   logItemToggle: (spanID: string, log: Log) => void;
@@ -41,7 +40,6 @@ type SpanDetailProps = {
 
 export default function SpanDetail(props: SpanDetailProps) {
   const {
-    addToUiFind,
     detailState,
     linksGetter,
     logItemToggle,
@@ -70,6 +68,7 @@ export default function SpanDetail(props: SpanDetailProps) {
       value: formatDuration(relativeStartTime),
     },
   ];
+  const deepLinkCopyText = `${window.location.origin}${window.location.pathname}?uiFind=${spanID}`;
 
   return (
     <div>
@@ -116,15 +115,12 @@ export default function SpanDetail(props: SpanDetailProps) {
           )}
         <small className="SpanDetail--debugInfo">
           <span className="SpanDetail--debugLabel" data-label="SpanID:" /> {spanID}
-          <Tooltip placement="topRight" title="Click to add to filter for deep linking">
-            <Button
-              className="SpanDetail--debugValue ant-btn-icon-only"
-              htmlType="button"
-              onClick={() => addToUiFind(spanID)}
-            >
-              <ZoomIn size="1.4em" />
-            </Button>
-          </Tooltip>
+          <CopyIcon
+            copyText={deepLinkCopyText}
+            icon="link"
+            placement="top"
+            tooltipTitle="Copy deep link to this span"
+          />
         </small>
       </div>
     </div>
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx
index 04ff414df9..875d40a783 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx
@@ -25,7 +25,6 @@ import { Log, Span, KeyValuePair, Link } from '../../../types/trace';
 import './SpanDetailRow.css';
 
 type SpanDetailRowProps = {
-  addToUiFind: (spanID: string) => void;
   color: string;
   columnDivision: number;
   detailState: DetailState;
@@ -51,7 +50,6 @@ export default class SpanDetailRow extends React.PureComponent<SpanDetailRowProp
 
   render() {
     const {
-      addToUiFind,
       color,
       columnDivision,
       detailState,
@@ -79,7 +77,6 @@ export default class SpanDetailRow extends React.PureComponent<SpanDetailRowProp
         <TimelineRow.Cell width={1 - columnDivision}>
           <div className="detail-info-wrapper" style={{ borderTopColor: color }}>
             <SpanDetail
-              addToUiFind={addToUiFind}
               detailState={detailState}
               linksGetter={this._linksGetter}
               logItemToggle={logItemToggle}
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index 7da5319438..db910990a8 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -36,7 +36,6 @@ import { Accessors } from '../ScrollManager';
 import { extractUiFindFromState, TExtractUiFindFromStateReturn } from '../../common/UiFindInput';
 import getLinks from '../../../model/link-patterns';
 import colorGenerator from '../../../utils/color-generator';
-import updateUiFind from '../../../utils/update-ui-find';
 import { TNil, ReduxState } from '../../../types';
 import { Log, Span, Trace, KeyValuePair } from '../../../types/trace';
 import TTraceTimeline from '../../../types/TTraceTimeline';
@@ -224,17 +223,6 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     }
   }
 
-  addToUiFind = (addition: string) => {
-    const { uiFind, history, location } = this.props;
-    if (!uiFind || !uiFind.includes(addition)) {
-      updateUiFind({
-        history,
-        location,
-        uiFind: cx(uiFind, addition),
-      });
-    }
-  };
-
   getAccessors() {
     const lv = this.listView;
     if (!lv) {
@@ -401,7 +389,6 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
     return (
       <div className="VirtualizedTraceView--row" key={key} style={{ ...style, zIndex: 1 }} {...attrs}>
         <SpanDetailRow
-          addToUiFind={this.addToUiFind}
           color={color}
           columnDivision={spanNameColumnWidth}
           onDetailToggled={detailToggle}
diff --git a/packages/jaeger-ui/src/components/common/CopyIcon.test.js b/packages/jaeger-ui/src/components/common/CopyIcon.test.js
index f94adacfac..57a7861c51 100644
--- a/packages/jaeger-ui/src/components/common/CopyIcon.test.js
+++ b/packages/jaeger-ui/src/components/common/CopyIcon.test.js
@@ -14,7 +14,7 @@
 
 import React from 'react';
 import { shallow } from 'enzyme';
-import { Icon, Tooltip } from 'antd';
+import { Button, Tooltip } from 'antd';
 
 import CopyIcon from './CopyIcon';
 
@@ -34,9 +34,9 @@ describe('<CopyIcon />', () => {
     expect(wrapper).toMatchSnapshot();
   });
 
-  it('updates state when icon is clicked', () => {
+  it('updates state when clicked', () => {
     expect(wrapper.state().hasCopied).toBe(false);
-    wrapper.find(Icon).simulate('click');
+    wrapper.find(Button).simulate('click');
     expect(wrapper.state().hasCopied).toBe(true);
   });
 
diff --git a/packages/jaeger-ui/src/components/common/CopyIcon.tsx b/packages/jaeger-ui/src/components/common/CopyIcon.tsx
index 7c6d1dd932..f21d86f7e0 100644
--- a/packages/jaeger-ui/src/components/common/CopyIcon.tsx
+++ b/packages/jaeger-ui/src/components/common/CopyIcon.tsx
@@ -14,12 +14,16 @@
 
 import * as React from 'react';
 
-import { Icon, Tooltip } from 'antd';
+import { Tooltip, Button } from 'antd';
+import { TooltipPlacement } from 'antd/lib/tooltip/index';
+
 import CopyToClipboard from 'react-copy-to-clipboard';
 
 type PropsType = {
   className?: string;
   copyText: string;
+  icon?: string;
+  placement?: TooltipPlacement;
   tooltipTitle: string;
 };
 
@@ -30,6 +34,8 @@ type StateType = {
 export default class CopyIcon extends React.PureComponent<PropsType, StateType> {
   static defaultProps: Partial<PropsType> = {
     className: undefined,
+    icon: 'copy',
+    placement: 'left',
   };
 
   state = {
@@ -56,11 +62,16 @@ export default class CopyIcon extends React.PureComponent<PropsType, StateType>
         arrowPointAtCenter
         mouseLeaveDelay={0.5}
         onVisibleChange={this.handleTooltipVisibilityChange}
-        placement="left"
+        placement={this.props.placement}
         title={this.state.hasCopied ? 'Copied' : this.props.tooltipTitle}
       >
         <CopyToClipboard text={this.props.copyText}>
-          <Icon className={this.props.className} onClick={this.handleClick} type="copy" />
+          <Button
+            className={this.props.className}
+            htmlType="button"
+            onClick={this.handleClick}
+            icon={this.props.icon}
+          />
         </CopyToClipboard>
       </Tooltip>
     );
diff --git a/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap b/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap
index 4f077742e3..96c2af0722 100644
--- a/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap
+++ b/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap
@@ -15,10 +15,15 @@ exports[`<CopyIcon /> renders as expected 1`] = `
   <CopyToClipboard
     text="copyTextValue"
   >
-    <Icon
+    <Button
+      block={false}
       className="classNameValue"
+      ghost={false}
+      htmlType="button"
+      icon="copy"
+      loading={false}
       onClick={[Function]}
-      type="copy"
+      prefixCls="ant-btn"
     />
   </CopyToClipboard>
 </Tooltip>

From 5801d3ec548a93834671827d2c47294d6deb4e5f Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 25 Apr 2019 13:43:02 -0400
Subject: [PATCH 10/13] Fix CopyIcon css, Swap in copy-to-clipboard

Signed-off-by: Everett Ross <reverett@uber.com>
---
 packages/jaeger-ui/package.json               |  2 +-
 .../TraceDiff/TraceDiffGraph/drawNode.css     |  1 +
 .../TracePage/TraceGraph/OpNode.css           |  1 +
 .../TraceTimelineViewer/SpanDetail/index.tsx  |  2 +-
 .../TraceTimelineViewer/SpanDetailRow.tsx     |  5 ++--
 .../src/components/common/CopyIcon.css        | 30 +++++++++++++++++++
 .../src/components/common/CopyIcon.test.js    | 14 ++++++++-
 .../src/components/common/CopyIcon.tsx        | 21 ++++++-------
 .../__snapshots__/CopyIcon.test.js.snap       | 24 +++++++--------
 yarn.lock                                     | 18 ++++-------
 10 files changed, 76 insertions(+), 42 deletions(-)
 create mode 100644 packages/jaeger-ui/src/components/common/CopyIcon.css

diff --git a/packages/jaeger-ui/package.json b/packages/jaeger-ui/package.json
index 9f439350e4..299ccea102 100644
--- a/packages/jaeger-ui/package.json
+++ b/packages/jaeger-ui/package.json
@@ -51,6 +51,7 @@
     "chance": "^1.0.10",
     "classnames": "^2.2.5",
     "combokeys": "^3.0.0",
+    "copy-to-clipboard": "^3.1.0",
     "cytoscape": "^3.2.1",
     "cytoscape-dagre": "^2.0.0",
     "d3-scale": "^1.0.6",
@@ -71,7 +72,6 @@
     "query-string": "^6.3.0",
     "raven-js": "^3.22.1",
     "react": "^16.3.2",
-    "react-copy-to-clipboard": "^5.0.1",
     "react-dimensions": "^1.3.0",
     "react-dom": "^16.3.2",
     "react-ga": "^2.4.1",
diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css
index f6e703c07d..c36635e989 100644
--- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css
+++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css
@@ -88,6 +88,7 @@ limitations under the License.
 .DiffNode--popover .DiffNode--copyIcon,
 .DiffNode:not(:hover) .DiffNode--copyIcon {
   color: transparent;
+  pointer-events: none;
 }
 
 /* Tweak the popover aesthetics - unfortunate but necessary */
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css
index fd8cc52e74..c91021d31a 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css
+++ b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css
@@ -57,6 +57,7 @@ limitations under the License.
 .OpNode--popover .OpNode--copyIcon,
 .OpNode:not(:hover) .OpNode--copyIcon {
   color: transparent;
+  pointer-events: none;
 }
 
 .OpNode--service {
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
index 64a1cd637c..3766b62d52 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
@@ -118,7 +118,7 @@ export default function SpanDetail(props: SpanDetailProps) {
           <CopyIcon
             copyText={deepLinkCopyText}
             icon="link"
-            placement="top"
+            placement="topRight"
             tooltipTitle="Copy deep link to this span"
           />
         </small>
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx
index 875d40a783..e78da95ce2 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx
@@ -19,7 +19,6 @@ import DetailState from './SpanDetail/DetailState';
 import SpanTreeOffset from './SpanTreeOffset';
 import TimelineRow from './TimelineRow';
 
-import { TNil } from '../../../types';
 import { Log, Span, KeyValuePair, Link } from '../../../types/trace';
 
 import './SpanDetailRow.css';
@@ -29,7 +28,7 @@ type SpanDetailRowProps = {
   columnDivision: number;
   detailState: DetailState;
   onDetailToggled: (spanID: string) => void;
-  linksGetter: ((span: Span, links: KeyValuePair[], index: number) => Link[]) | TNil;
+  linksGetter: (span: Span, links: KeyValuePair[], index: number) => Link[];
   logItemToggle: (spanID: string, log: Log) => void;
   logsToggle: (spanID: string) => void;
   processToggle: (spanID: string) => void;
@@ -45,7 +44,7 @@ export default class SpanDetailRow extends React.PureComponent<SpanDetailRowProp
 
   _linksGetter = (items: KeyValuePair[], itemIndex: number) => {
     const { linksGetter, span } = this.props;
-    return linksGetter ? linksGetter(span, items, itemIndex) : [];
+    return linksGetter(span, items, itemIndex);
   };
 
   render() {
diff --git a/packages/jaeger-ui/src/components/common/CopyIcon.css b/packages/jaeger-ui/src/components/common/CopyIcon.css
new file mode 100644
index 0000000000..25738d372e
--- /dev/null
+++ b/packages/jaeger-ui/src/components/common/CopyIcon.css
@@ -0,0 +1,30 @@
+/*
+Copyright (c) 2019 Uber Technologies, Inc.
+
+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
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+.CopyIcon,
+.CopyIcon:hover {
+  background-color: transparent;
+  border: none;
+  color: inherit;
+  height: 100%;
+  overflow: hidden;
+  padding: 0px;
+}
+
+.CopyIcon:focus {
+  background-color: rgba(255, 255, 255, 0.25);
+  color: inherit;
+}
diff --git a/packages/jaeger-ui/src/components/common/CopyIcon.test.js b/packages/jaeger-ui/src/components/common/CopyIcon.test.js
index 57a7861c51..d7b97efed7 100644
--- a/packages/jaeger-ui/src/components/common/CopyIcon.test.js
+++ b/packages/jaeger-ui/src/components/common/CopyIcon.test.js
@@ -15,18 +15,27 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 import { Button, Tooltip } from 'antd';
+import * as copy from 'copy-to-clipboard';
 
 import CopyIcon from './CopyIcon';
 
+jest.mock('copy-to-clipboard');
+
 describe('<CopyIcon />', () => {
   const props = {
     className: 'classNameValue',
     copyText: 'copyTextValue',
     tooltipTitle: 'tooltipTitleValue',
   };
+  let copySpy;
   let wrapper;
 
+  beforeAll(() => {
+    copySpy = jest.spyOn(copy, 'default');
+  });
+
   beforeEach(() => {
+    copySpy.mockReset();
     wrapper = shallow(<CopyIcon {...props} />);
   });
 
@@ -34,10 +43,13 @@ describe('<CopyIcon />', () => {
     expect(wrapper).toMatchSnapshot();
   });
 
-  it('updates state when clicked', () => {
+  it('updates state and copies when clicked', () => {
     expect(wrapper.state().hasCopied).toBe(false);
+    expect(copySpy).not.toHaveBeenCalled();
+
     wrapper.find(Button).simulate('click');
     expect(wrapper.state().hasCopied).toBe(true);
+    expect(copySpy).toHaveBeenCalledWith(props.copyText);
   });
 
   it('updates state when tooltip hides and state.hasCopied is true', () => {
diff --git a/packages/jaeger-ui/src/components/common/CopyIcon.tsx b/packages/jaeger-ui/src/components/common/CopyIcon.tsx
index f21d86f7e0..546da2b04d 100644
--- a/packages/jaeger-ui/src/components/common/CopyIcon.tsx
+++ b/packages/jaeger-ui/src/components/common/CopyIcon.tsx
@@ -14,10 +14,12 @@
 
 import * as React from 'react';
 
-import { Tooltip, Button } from 'antd';
+import { Button, Tooltip } from 'antd';
 import { TooltipPlacement } from 'antd/lib/tooltip/index';
+import cx from 'classnames';
+import copy from 'copy-to-clipboard';
 
-import CopyToClipboard from 'react-copy-to-clipboard';
+import './CopyIcon.css';
 
 type PropsType = {
   className?: string;
@@ -46,6 +48,7 @@ export default class CopyIcon extends React.PureComponent<PropsType, StateType>
     this.setState({
       hasCopied: true,
     });
+    copy(this.props.copyText);
   };
 
   handleTooltipVisibilityChange = (visible: boolean) => {
@@ -65,14 +68,12 @@ export default class CopyIcon extends React.PureComponent<PropsType, StateType>
         placement={this.props.placement}
         title={this.state.hasCopied ? 'Copied' : this.props.tooltipTitle}
       >
-        <CopyToClipboard text={this.props.copyText}>
-          <Button
-            className={this.props.className}
-            htmlType="button"
-            onClick={this.handleClick}
-            icon={this.props.icon}
-          />
-        </CopyToClipboard>
+        <Button
+          className={cx(this.props.className, 'CopyIcon')}
+          htmlType="button"
+          icon={this.props.icon}
+          onClick={this.handleClick}
+        />
       </Tooltip>
     );
   }
diff --git a/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap b/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap
index 96c2af0722..7f3c06cfd7 100644
--- a/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap
+++ b/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap
@@ -12,19 +12,15 @@ exports[`<CopyIcon /> renders as expected 1`] = `
   title="tooltipTitleValue"
   transitionName="zoom-big-fast"
 >
-  <CopyToClipboard
-    text="copyTextValue"
-  >
-    <Button
-      block={false}
-      className="classNameValue"
-      ghost={false}
-      htmlType="button"
-      icon="copy"
-      loading={false}
-      onClick={[Function]}
-      prefixCls="ant-btn"
-    />
-  </CopyToClipboard>
+  <Button
+    block={false}
+    className="classNameValue CopyIcon"
+    ghost={false}
+    htmlType="button"
+    icon="copy"
+    loading={false}
+    onClick={[Function]}
+    prefixCls="ant-btn"
+  />
 </Tooltip>
 `;
diff --git a/yarn.lock b/yarn.lock
index 80425d4eaf..e34826087b 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3510,11 +3510,12 @@ copy-descriptor@^0.1.0:
   version "0.1.1"
   resolved "https://registry.yarnpkg.com/copy-descriptor/-/copy-descriptor-0.1.1.tgz#676f6eb3c39997c2ee1ac3a924fd6124748f578d"
 
-copy-to-clipboard@^3:
-  version "3.0.8"
-  resolved "https://registry.yarnpkg.com/copy-to-clipboard/-/copy-to-clipboard-3.0.8.tgz#f4e82f4a8830dce4666b7eb8ded0c9bcc313aba9"
+copy-to-clipboard@^3.1.0:
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/copy-to-clipboard/-/copy-to-clipboard-3.1.0.tgz#0a28141899e6bd217b9dc13fd1689b3b38820b44"
+  integrity sha512-+RNyDq266tv5aGhfRsL6lxgj8Y6sCvTrVJnFUVvuxuqkcSMaLISt1wd4JkdQSphbcLTIQ9kEpTULNnoCXAFdng==
   dependencies:
-    toggle-selection "^1.0.3"
+    toggle-selection "^1.0.6"
 
 core-js@2.6.4:
   version "2.6.4"
@@ -10469,13 +10470,6 @@ react-app-rewired@2.0.1:
     dotenv "^6.2.0"
     semver "^5.6.0"
 
-react-copy-to-clipboard@^5.0.1:
-  version "5.0.1"
-  resolved "https://registry.yarnpkg.com/react-copy-to-clipboard/-/react-copy-to-clipboard-5.0.1.tgz#8eae107bb400be73132ed3b6a7b4fb156090208e"
-  dependencies:
-    copy-to-clipboard "^3"
-    prop-types "^15.5.8"
-
 react-dev-utils@^7.0.0:
   version "7.0.5"
   resolved "https://registry.yarnpkg.com/react-dev-utils/-/react-dev-utils-7.0.5.tgz#cb95375d01ae71ca27b3c7616006ef7a77d14e8e"
@@ -12365,7 +12359,7 @@ to-space-case@^1.0.0:
   dependencies:
     to-no-case "^1.0.0"
 
-toggle-selection@^1.0.3:
+toggle-selection@^1.0.6:
   version "1.0.6"
   resolved "https://registry.yarnpkg.com/toggle-selection/-/toggle-selection-1.0.6.tgz#6e45b1263f2017fa0acc7d89d78b15b8bf77da32"
 

From 73e86915e8c4fa72c1f8763e2283516feb8ec8a4 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 25 Apr 2019 14:45:06 -0400
Subject: [PATCH 11/13] Improve VirtualizedTraceView shouldComponentUpdate

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../VirtualizedTraceView.test.js              | 18 +++++++-------
 .../VirtualizedTraceView.tsx                  | 24 +++++++------------
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
index e70371e6a1..d2f84783d2 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
@@ -348,7 +348,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
   });
 
   describe('shouldScrollToFirstUiFindMatch', () => {
-    const propsWithTruthShouldScrollToFirstUiFindMatch = { ...props, shouldScrollToFirstUiFindMatch: true };
+    const propsWithTrueShouldScrollToFirstUiFindMatch = { ...props, shouldScrollToFirstUiFindMatch: true };
 
     beforeEach(() => {
       props.scrollToFirstVisibleSpan.mockReset();
@@ -359,25 +359,21 @@ describe('<VirtualizedTraceViewImpl>', () => {
       expect(props.scrollToFirstVisibleSpan).not.toHaveBeenCalled();
       expect(props.clearShouldScrollToFirstUiFindMatch).not.toHaveBeenCalled();
 
-      wrapper.setProps(propsWithTruthShouldScrollToFirstUiFindMatch);
+      wrapper.setProps(propsWithTrueShouldScrollToFirstUiFindMatch);
       expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1);
       expect(props.clearShouldScrollToFirstUiFindMatch).toHaveBeenCalledTimes(1);
     });
 
     describe('shouldComponentUpdate', () => {
-      it('returns true if props.shouldScrollToFirstUiFindMatch is unchanged', () => {
-        expect(wrapper.instance().shouldComponentUpdate(props)).toBe(true);
-      });
-
       it('returns true if props.shouldScrollToFirstUiFindMatch changes to true', () => {
-        expect(wrapper.instance().shouldComponentUpdate(propsWithTruthShouldScrollToFirstUiFindMatch)).toBe(
+        expect(wrapper.instance().shouldComponentUpdate(propsWithTrueShouldScrollToFirstUiFindMatch)).toBe(
           true
         );
       });
 
       it('returns true if props.shouldScrollToFirstUiFindMatch changes to false and another props change', () => {
         const propsWithOtherDifferenceAndTrueshouldScrollToFirstUiFindMatch = {
-          ...propsWithTruthShouldScrollToFirstUiFindMatch,
+          ...propsWithTrueShouldScrollToFirstUiFindMatch,
           clearShouldScrollToFirstUiFindMatch: () => {},
         };
         wrapper.setProps(propsWithOtherDifferenceAndTrueshouldScrollToFirstUiFindMatch);
@@ -385,7 +381,11 @@ describe('<VirtualizedTraceViewImpl>', () => {
       });
 
       it('returns false if props.shouldScrollToFirstUiFindMatch changes to false and no other props change', () => {
-        wrapper.setProps(propsWithTruthShouldScrollToFirstUiFindMatch);
+        wrapper.setProps(propsWithTrueShouldScrollToFirstUiFindMatch);
+        expect(wrapper.instance().shouldComponentUpdate(props)).toBe(false);
+      });
+
+      it('returns false if all props are unchanged', () => {
         expect(wrapper.instance().shouldComponentUpdate(props)).toBe(false);
       });
     });
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index db910990a8..4763c86f53 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -135,8 +135,7 @@ function getCssClasses(currentViewRange: [number, number]) {
 }
 
 // export from tests
-// eslint-disable-next-line react/no-redundant-should-component-update
-export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTraceViewProps> {
+export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceViewProps> {
   clippingCssClasses: string;
   listView: ListView | TNil;
   rowStates: RowState[];
@@ -161,22 +160,17 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
   }
 
   shouldComponentUpdate(nextProps: VirtualizedTraceViewProps) {
-    if (
-      !nextProps.shouldScrollToFirstUiFindMatch &&
-      nextProps.shouldScrollToFirstUiFindMatch !== this.props.shouldScrollToFirstUiFindMatch
-    ) {
-      const nextPropKeys = Object.keys(nextProps);
-      for (let i = 0; i < nextPropKeys.length; i += 1) {
-        if (
-          nextProps[nextPropKeys[i] as keyof VirtualizedTraceViewProps] !==
-            this.props[nextPropKeys[i] as keyof VirtualizedTraceViewProps] &&
-          nextPropKeys[i] !== 'shouldScrollToFirstUiFindMatch'
-        )
+    const nextPropKeys = Object.keys(nextProps) as (keyof VirtualizedTraceViewProps)[];
+    for (let i = 0; i < nextPropKeys.length; i += 1) {
+      if (nextProps[nextPropKeys[i]] !== this.props[nextPropKeys[i]]) {
+        if (nextPropKeys[i] === 'shouldScrollToFirstUiFindMatch') {
+          if (nextProps[nextPropKeys[i]]) return true;
+        } else {
           return true;
+        }
       }
-      return false;
     }
-    return true;
+    return false;
   }
 
   componentWillUpdate(nextProps: VirtualizedTraceViewProps) {

From 9b9f5c0809aff2a62e100e895f7cf613113ad560 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Thu, 2 May 2019 11:33:16 -0400
Subject: [PATCH 12/13] Remove unnecessary function calls, improve names

Signed-off-by: Everett Ross <reverett@uber.com>
---
 .../TracePage/TracePageHeader/TracePageSearchBar.css      | 6 +-----
 .../TracePage/TracePageHeader/TracePageSearchBar.tsx      | 4 ++--
 .../TraceTimelineViewer/VirtualizedTraceView.tsx          | 2 ++
 .../src/components/TracePage/TraceTimelineViewer/duck.tsx | 6 +++---
 packages/jaeger-ui/src/components/TracePage/index.test.js | 8 +-------
 packages/jaeger-ui/src/components/TracePage/index.tsx     | 4 +++-
 6 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
index 703e81f7a2..bdac0b9cb0 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
@@ -18,10 +18,6 @@ limitations under the License.
   width: 40%;
 }
 
-.TracePageSearchBar--InputGroup {
-  justify-content: flex-end;
-}
-
 .TracePageSearchBar--bar {
   max-width: 20rem;
   transition: max-width 0.5s;
@@ -44,6 +40,6 @@ limitations under the License.
   opacity: 0.5;
 }
 
-.TracePageSearchBar--btn.locate-btn {
+.TracePageSearchBar--locateBtn {
   padding: 1px 8px 4px;
 }
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
index bf88749497..0c09f5f4f9 100644
--- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
@@ -59,7 +59,7 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
   return (
     <div className="TracePageSearchBar">
       {/* style inline because compact overwrites the display */}
-      <Input.Group className="TracePageSearchBar--InputGroup" compact style={{ display: 'flex' }}>
+      <Input.Group className="ub-justify-end" compact style={{ display: 'flex' }}>
         <UiFindInput
           inputProps={uiFindInputInputProps}
           forwardedRef={forwardedRef}
@@ -68,7 +68,7 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
         {navigable && (
           <>
             <Button
-              className={cx(btnClass, 'locate-btn')}
+              className={cx(btnClass, 'TracePageSearchBar--locateBtn')}
               disabled={!textFilter}
               htmlType="button"
               onClick={focusUiFindMatches}
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index 4763c86f53..55e1d66ecf 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -160,9 +160,11 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
   }
 
   shouldComponentUpdate(nextProps: VirtualizedTraceViewProps) {
+    // If any prop updates, VirtualizedTraceViewImpl should update.
     const nextPropKeys = Object.keys(nextProps) as (keyof VirtualizedTraceViewProps)[];
     for (let i = 0; i < nextPropKeys.length; i += 1) {
       if (nextProps[nextPropKeys[i]] !== this.props[nextPropKeys[i]]) {
+        // Unless the only change was props.shouldScrollToFirstUiFindMatch changing to false.
         if (nextPropKeys[i] === 'shouldScrollToFirstUiFindMatch') {
           if (nextProps[nextPropKeys[i]]) return true;
         } else {
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
index eeee8faf16..b5741442f2 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
@@ -97,7 +97,7 @@ const fullActions = createActions<TActionTypes>({
 
 export const actions = (fullActions as any).jaegerUi.traceTimelineViewer as TTimelineViewerActions;
 
-function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
+function calculateFocusedFindRowStates(uiFind: string, spans: Span[]) {
   const spansMap = new Map();
   const childrenHiddenIDs: Set<string> = new Set();
   const detailStates: Map<string, DetailState> = new Map();
@@ -127,7 +127,7 @@ function focusUiFindMatches(state: TTraceTimeline, { uiFind, trace }: TTraceUiFi
   if (!uiFind) return state;
   return {
     ...state,
-    ...calculateHiddenIdsAndDetailStates(uiFind, trace.spans),
+    ...calculateFocusedFindRowStates(uiFind, trace.spans),
   };
 }
 
@@ -147,7 +147,7 @@ function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {
 
   return Object.assign(
     { ...newInitialState(), spanNameColumnWidth, traceID },
-    uiFind ? calculateHiddenIdsAndDetailStates(uiFind, spans) : null
+    uiFind ? calculateFocusedFindRowStates(uiFind, spans) : null
   );
 }
 
diff --git a/packages/jaeger-ui/src/components/TracePage/index.test.js b/packages/jaeger-ui/src/components/TracePage/index.test.js
index 60a71f4e7e..b1ab787009 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/index.test.js
@@ -136,20 +136,15 @@ describe('<TracePage>', () => {
   });
 
   describe('focusUiFindMatches', () => {
-    let scrollToFirstVisibleSpanSpy;
-
     beforeEach(() => {
-      scrollToFirstVisibleSpanSpy = jest.spyOn(wrapper.instance()._scrollManager, 'scrollToFirstVisibleSpan');
-      scrollToFirstVisibleSpanSpy.mockReset();
       defaultProps.focusUiFindMatches.mockReset();
     });
 
-    it('calls _scrollManager.scrollToFirstVisibleSpan and props.focusUiFindMatches with props.trace.data and uiFind when props.trace.data is present', () => {
+    it('calls props.focusUiFindMatches with props.trace.data and uiFind when props.trace.data is present', () => {
       const uiFind = 'test ui find';
       wrapper.setProps({ uiFind });
       wrapper.find(TracePageHeader).prop('focusUiFindMatches')();
       expect(defaultProps.focusUiFindMatches).toHaveBeenCalledWith(defaultProps.trace.data, uiFind);
-      expect(scrollToFirstVisibleSpanSpy).toHaveBeenCalled();
     });
 
     it('handles when props.trace.data is absent', () => {
@@ -157,7 +152,6 @@ describe('<TracePage>', () => {
       wrapper.setProps({ trace: {} });
       propFn();
       expect(defaultProps.focusUiFindMatches).not.toHaveBeenCalled();
-      expect(scrollToFirstVisibleSpanSpy).not.toHaveBeenCalled();
     });
   });
 
diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 22a50fc124..73332cad53 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -280,6 +280,9 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
     if (this.props.trace && this.props.trace.data) {
       this.traceDagEV = calculateTraceDagEV(this.props.trace.data);
     }
+    if (traceGraphView) {
+      this.focusUiFindMatches();
+    }
     this.setState({ traceGraphView: !traceGraphView });
   };
 
@@ -308,7 +311,6 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
   focusUiFindMatches = () => {
     const { trace, focusUiFindMatches, uiFind } = this.props;
     if (trace && trace.data) {
-      this._scrollManager.scrollToFirstVisibleSpan();
       focusUiFindMatches(trace.data, uiFind);
     }
   };

From cdbdd9dea3802b418fb87033c3e848812ffc5db2 Mon Sep 17 00:00:00 2001
From: Everett Ross <reverett@uber.com>
Date: Fri, 3 May 2019 11:06:59 -0400
Subject: [PATCH 13/13] Remove nonfunctional focusUiFindMatches call

Signed-off-by: Everett Ross <reverett@uber.com>
---
 packages/jaeger-ui/src/components/TracePage/index.tsx | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx
index 73332cad53..9331dddca6 100644
--- a/packages/jaeger-ui/src/components/TracePage/index.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/index.tsx
@@ -280,9 +280,6 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
     if (this.props.trace && this.props.trace.data) {
       this.traceDagEV = calculateTraceDagEV(this.props.trace.data);
     }
-    if (traceGraphView) {
-      this.focusUiFindMatches();
-    }
     this.setState({ traceGraphView: !traceGraphView });
   };