Traceview find: background color and prev/next (#46527)

* Next/prev buttons * expand * Sticky search bar * Removed anys * testing * testing * Tests for next/prev/suffix * More tests * Span bar row color * Add clear to input and update search bar styles * Update test * PR changes Co-authored-by: Connor Lindsey <cblindsey3@gmail.com>
2022-03-31 09:43:59 +01:00
parent 4d0204d012
commit 58922d78df
13 changed files with 395 additions and 246 deletions
@@ -21,7 +21,6 @@ import { dateTimeFormat, GrafanaTheme2, TimeZone } from '@grafana/data';
 import { useStyles2 } from '@grafana/ui';

 import SpanGraph from './SpanGraph';
-import TracePageSearchBar from './TracePageSearchBar';
 import { autoColor, TUpdateViewRangeTimeFunction, ViewRange, ViewRangeTimeUpdate } from '..';
 import LabeledList from '../common/LabeledList';
 import TraceName from '../common/TraceName';
@@ -138,22 +137,15 @@ const getStyles = (theme: GrafanaTheme2) => {

 type TracePageHeaderEmbedProps = {
  canCollapse: boolean;
-  clearSearch: () => void;
-  focusUiFindMatches: () => void;
  hideMap: boolean;
  hideSummary: boolean;
-  nextResult: () => void;
  onSlimViewClicked: () => void;
  onTraceGraphViewClicked: () => void;
-  prevResult: () => void;
-  resultCount: number;
  slimView: boolean;
  trace: Trace;
  updateNextViewRangeTime: (update: ViewRangeTimeUpdate) => void;
  updateViewRangeTime: TUpdateViewRangeTimeFunction;
  viewRange: ViewRange;
-  searchValue: string;
-  onSearchValueChange: (value: string) => void;
  timeZone: TimeZone;
 };

@@ -200,21 +192,14 @@ export const HEADER_ITEMS = [
 export default function TracePageHeader(props: TracePageHeaderEmbedProps) {
  const {
    canCollapse,
-    clearSearch,
-    focusUiFindMatches,
    hideMap,
    hideSummary,
-    nextResult,
    onSlimViewClicked,
-    prevResult,
-    resultCount,
    slimView,
    trace,
    updateNextViewRangeTime,
    updateViewRangeTime,
    viewRange,
-    searchValue,
-    onSearchValueChange,
    timeZone,
  } = props;

@@ -267,17 +252,6 @@ export default function TracePageHeader(props: TracePageHeaderEmbedProps) {
        ) : (
          title
        )}
-        <TracePageSearchBar
-          clearSearch={clearSearch}
-          focusUiFindMatches={focusUiFindMatches}
-          nextResult={nextResult}
-          prevResult={prevResult}
-          resultCount={resultCount}
-          // TODO: we can change this when we have scroll to span functionality
-          navigable={false}
-          searchValue={searchValue}
-          onSearchValueChange={onSearchValueChange}
-        />
      </div>
      {summaryItems && <LabeledList className={styles.TracePageHeaderOverviewItems} items={summaryItems} />}
      {!hideMap && !slimView && (
@@ -14,6 +14,7 @@

 import React from 'react';
 import { shallow } from 'enzyme';
+import { createTheme } from '@grafana/data';

 import * as markers from './TracePageSearchBar.markers';
 import TracePageSearchBar, { getStyles } from './TracePageSearchBar';
@@ -24,7 +25,7 @@ const defaultProps = {
  navigable: true,
  nextResult: () => {},
  prevResult: () => {},
-  resultCount: 0,
+  suffix: '',
  searchValue: 'something',
 };

@@ -45,26 +46,28 @@ describe('<TracePageSearchBar>', () => {
          name: 'search',
        })
      );
-      expect(suffix.hasClass(getStyles().TracePageSearchBarCount)).toBe(true);
-      expect(suffix.text()).toBe(String(defaultProps.resultCount));
+      const theme = createTheme();
+      expect(suffix.hasClass(getStyles(theme).TracePageSearchBarSuffix)).toBe(true);
+      expect(suffix.text()).toBe(String(defaultProps.suffix));
    });

    it('renders buttons', () => {
      const buttons = wrapper.find('Button');
-      expect(buttons.length).toBe(4);
+      expect(buttons.length).toBe(2);
      buttons.forEach((button) => {
        expect(button.prop('disabled')).toBe(false);
      });
      expect(wrapper.find('Button[icon="arrow-up"]').prop('onClick')).toBe(defaultProps.prevResult);
      expect(wrapper.find('Button[icon="arrow-down"]').prop('onClick')).toBe(defaultProps.nextResult);
-      expect(wrapper.find('Button[icon="times"]').prop('onClick')).toBe(defaultProps.clearSearch);
    });

-    it('hides navigation buttons when not navigable', () => {
+    it('only shows navigable buttons when navigable is true', () => {
      wrapper.setProps({ navigable: false });
-      const button = wrapper.find('Button');
-      expect(button.length).toBe(1);
-      expect(button.prop('icon')).toBe('times');
+      var buttons = wrapper.find('Button');
+      expect(buttons.length).toBe(0);
+      wrapper.setProps({ navigable: true });
+      buttons = wrapper.find('Button');
+      expect(buttons.length).toBe(2);
    });
  });

@@ -79,7 +82,7 @@ describe('<TracePageSearchBar>', () => {

    it('renders buttons', () => {
      const buttons = wrapper.find('Button');
-      expect(buttons.length).toBe(4);
+      expect(buttons.length).toBe(2);
      buttons.forEach((button) => {
        expect(button.prop('disabled')).toBe(true);
      });
@@ -14,7 +14,6 @@

 import * as React from 'react';
 import cx from 'classnames';
-import IoAndroidLocate from 'react-icons/lib/io/android-locate';
 import { css } from '@emotion/css';
 import { Button, useStyles2 } from '@grafana/ui';

@@ -24,11 +23,24 @@ import UiFindInput from '../common/UiFindInput';
 import { ubFlexAuto, ubJustifyEnd } from '../uberUtilityStyles';
 // eslint-disable-next-line no-duplicate-imports
 import { memo } from 'react';
+import { GrafanaTheme2 } from '@grafana/data';

-export const getStyles = () => {
+export const getStyles = (theme: GrafanaTheme2) => {
  return {
    TracePageSearchBar: css`
      label: TracePageSearchBar;
+      float: right;
+      position: sticky;
+      top: 8px;
+      right: 0;
+      z-index: ${theme.zIndex.navbarFixed};
+      background: ${theme.colors.background.primary};
+      margin-top: 8px;
+      margin-bottom: -48px;
+      padding: 8px;
+      margin-right: 2px;
+      border-radius: 4px;
+      box-shadow: ${theme.shadows.z2};
    `,
    TracePageSearchBarBar: css`
      label: TracePageSearchBarBar;
@@ -38,14 +50,14 @@ export const getStyles = () => {
        max-width: 100%;
      }
    `,
-    TracePageSearchBarCount: css`
-      label: TracePageSearchBarCount;
+    TracePageSearchBarSuffix: css`
+      label: TracePageSearchBarSuffix;
      opacity: 0.6;
    `,
    TracePageSearchBarBtn: css`
      label: TracePageSearchBarBtn;
-      border-left: none;
      transition: 0.2s;
+      margin-left: 8px;
    `,
    TracePageSearchBarBtnDisabled: css`
      label: TracePageSearchBarBtnDisabled;
@@ -61,72 +73,62 @@ export const getStyles = () => {
 type TracePageSearchBarProps = {
  prevResult: () => void;
  nextResult: () => void;
-  clearSearch: () => void;
-  focusUiFindMatches: () => void;
-  resultCount: number;
  navigable: boolean;
  searchValue: string;
  onSearchValueChange: (value: string) => void;
+  searchBarSuffix: string;
 };

 export default memo(function TracePageSearchBar(props: TracePageSearchBarProps) {
-  const {
-    clearSearch,
-    focusUiFindMatches,
-    navigable,
-    nextResult,
-    prevResult,
-    resultCount,
-    onSearchValueChange,
-    searchValue,
-  } = props;
+  const { navigable, nextResult, prevResult, onSearchValueChange, searchValue, searchBarSuffix } = props;
  const styles = useStyles2(getStyles);

-  const count = searchValue ? <span className={styles.TracePageSearchBarCount}>{resultCount}</span> : null;
+  const suffix = searchValue ? (
+    <span className={styles.TracePageSearchBarSuffix} data-testid="trace-page-search-bar-suffix">
+      {searchBarSuffix}
+    </span>
+  ) : null;

  const btnClass = cx(styles.TracePageSearchBarBtn, { [styles.TracePageSearchBarBtnDisabled]: !searchValue });
  const uiFindInputInputProps = {
    'data-test': markers.IN_TRACE_SEARCH,
    className: cx(styles.TracePageSearchBarBar, ubFlexAuto),
    name: 'search',
-    suffix: count,
+    suffix,
  };

  return (
    <div className={styles.TracePageSearchBar}>
      <span className={ubJustifyEnd} style={{ display: 'flex' }}>
-        <UiFindInput onChange={onSearchValueChange} value={searchValue} inputProps={uiFindInputInputProps} />
+        <UiFindInput
+          onChange={onSearchValueChange}
+          value={searchValue}
+          inputProps={uiFindInputInputProps}
+          allowClear={true}
+        />
        <>
          {navigable && (
            <>
-              <Button
-                className={cx(btnClass, styles.TracePageSearchBarLocateBtn)}
-                disabled={!searchValue}
-                type="button"
-                onClick={focusUiFindMatches}
-              >
-                <IoAndroidLocate />
-              </Button>
-              <Button className={btnClass} disabled={!searchValue} type="button" icon="arrow-up" onClick={prevResult} />
              <Button
                className={btnClass}
+                variant="secondary"
                disabled={!searchValue}
                type="button"
                icon="arrow-down"
+                data-testid="trace-page-search-bar-next-result-button"
                onClick={nextResult}
              />
+              <Button
+                className={btnClass}
+                variant="secondary"
+                disabled={!searchValue}
+                type="button"
+                icon="arrow-up"
+                data-testid="trace-page-search-bar-prev-result-button"
+                onClick={prevResult}
+              />
            </>
          )}
-          <Button
-            variant={'secondary'}
-            fill={'text'}
-            // className={btnClass}
-            disabled={!searchValue}
-            type="button"
-            icon="times"
-            onClick={clearSearch}
-            title={'Clear search'}
-          />
        </>
      </span>
    </div>
@@ -166,17 +166,17 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
    `,
    rowMatchingFilter: css`
      label: rowMatchingFilter;
-      background-color: ${autoColor(theme, '#fffce4')};
+      background-color: ${autoColor(theme, '#fffbde')};
      &:hover .${nameWrapperClassName} {
        background: linear-gradient(
          90deg,
-          ${autoColor(theme, '#fff5e1')},
-          ${autoColor(theme, '#fff5e1')} 75%,
-          ${autoColor(theme, '#ffe6c9')}
+          ${autoColor(theme, '#fffbde')},
+          ${autoColor(theme, '#fffbde')} 75%,
+          ${autoColor(theme, '#f7f1c6')}
        );
      }
      &:hover .${viewClassName} {
-        background-color: ${autoColor(theme, '#fff3d7')};
+        background-color: ${autoColor(theme, '#f7f1c6')};
        outline: 1px solid ${autoColor(theme, '#ddd')};
      }
    `,
@@ -101,6 +101,7 @@ type TVirtualizedTraceViewOwnProps = {
  createSpanLink?: SpanLinkFunc;
  scrollElement?: Element;
  focusedSpanId?: string;
+  focusedSpanIdForSearch: string;
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
  topOfExploreViewRef?: RefObject<HTMLDivElement>;
 };
@@ -223,6 +224,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
      trace: nextTrace,
      uiFind,
      focusedSpanId,
+      focusedSpanIdForSearch,
    } = this.props;

    if (trace !== nextTrace) {
@@ -241,6 +243,10 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
    if (focusedSpanId !== prevProps.focusedSpanId) {
      this.scrollToSpan(focusedSpanId);
    }
+
+    if (focusedSpanIdForSearch !== prevProps.focusedSpanIdForSearch) {
+      this.scrollToSpan(focusedSpanIdForSearch);
+    }
  }

  getRowStates(): RowState[] {
@@ -378,6 +384,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
      theme,
      createSpanLink,
      focusedSpanId,
+      focusedSpanIdForSearch,
    } = this.props;
    // to avert flow error
    if (!trace) {
@@ -387,7 +394,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
    const isCollapsed = childrenHiddenIDs.has(spanID);
    const isDetailExpanded = detailStates.has(spanID);
    const isMatchingFilter = findMatchesIDs ? findMatchesIDs.has(spanID) : false;
-    const isFocused = spanID === focusedSpanId;
+    const isFocused = spanID === focusedSpanId || spanID === focusedSpanIdForSearch;
    const showErrorIcon = isErrorSpan(span) || (isCollapsed && spanContainsErredSpan(trace.spans, spanIndex));

    // Check for direct child "server" span if the span is a "client" span.
@@ -105,6 +105,7 @@ type TProps = TExtractUiFindFromStateReturn & {
  createSpanLink?: SpanLinkFunc;
  scrollElement?: Element;
  focusedSpanId?: string;
+  focusedSpanIdForSearch: string;
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
  topOfExploreViewRef?: RefObject<HTMLDivElement>;
 };
@@ -163,6 +164,7 @@ export class UnthemedTraceTimelineViewer extends React.PureComponent<TProps, Sta
      traceTimeline,
      theme,
      topOfExploreViewRef,
+      focusedSpanIdForSearch,
      ...rest
    } = this.props;
    const { trace } = rest;
@@ -194,6 +196,7 @@ export class UnthemedTraceTimelineViewer extends React.PureComponent<TProps, Sta
            setSpanNameColumnWidth={setSpanNameColumnWidth}
            currentViewRangeTime={viewRange.time.current}
            topOfExploreViewRef={topOfExploreViewRef}
+            focusedSpanIdForSearch={focusedSpanIdForSearch}
          />
        </div>
      </ExternalLinkContext.Provider>