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/__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 @@ -70,6 +72,8 @@ exports[`drawNode diffNode renders as expected when props.a and props.b are the @@ -116,6 +120,8 @@ exports[`drawNode diffNode renders as expected when props.a is 0 1`] = ` @@ -181,6 +187,8 @@ exports[`drawNode diffNode renders as expected when props.a is 0 1`] = ` @@ -242,6 +250,8 @@ exports[`drawNode diffNode renders as expected when props.a is less than props.b @@ -307,6 +317,8 @@ exports[`drawNode diffNode renders as expected when props.a is less than props.b @@ -368,6 +380,8 @@ exports[`drawNode diffNode renders as expected when props.a is more than props.b @@ -433,6 +447,8 @@ exports[`drawNode diffNode renders as expected when props.a is more than props.b @@ -494,6 +510,8 @@ exports[`drawNode diffNode renders as expected when props.b is 0 1`] = ` @@ -559,6 +577,8 @@ exports[`drawNode diffNode renders as expected when props.b is 0 1`] = ` @@ -615,6 +635,8 @@ exports[`drawNode diffNode renders as expected when props.isUiFindMatch is true @@ -660,6 +682,8 @@ exports[`drawNode diffNode renders as expected when props.isUiFindMatch is true 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/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/ScrollManager.tsx b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx index 96c4afee03..807a0f25a5 100644 --- a/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx +++ b/packages/jaeger-ui/src/components/TracePage/ScrollManager.tsx @@ -120,7 +120,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 +131,14 @@ export default class ScrollManager { } const { duration, spans, startTime: traceStartTime } = this._trace; const isUp = direction < 0; - const boundaryRow = isUp ? xrs.getTopRowIndexVisible() : xrs.getBottomRowIndexVisible(); + 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; @@ -184,7 +191,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 +262,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/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/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('', () => { 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/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx index 3ed9cb6386..59e185972c 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; + focusUiFindMatches: () => void; hideMap: boolean; hideSummary: boolean; linkToStandalone: string; @@ -95,6 +96,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded const { canCollapse, clearSearch, + focusUiFindMatches, forwardedRef, hideMap, hideSummary, @@ -161,9 +163,9 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded ) : ( title )} - {showShortcutsHelp && } - + {showShortcutsHelp && } {showViewOptions && ( ', () => { - 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('', () => { let wrapper; beforeEach(() => { @@ -55,7 +55,7 @@ describe('', () => { 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('', () => { 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('', () => { 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); @@ -98,3 +94,12 @@ describe('', () => { }); }); }); + +describe('', () => { + const { forwardedRef: ref, ...propsWithoutRef } = defaultProps; + + it('forwardsRef correctly', () => { + const wrapper = shallow(); + expect(wrapper.find(TracePageSearchBar).props()).toEqual(defaultProps); + }); +}); diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx index 56e00bf326..0c09f5f4f9 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'; @@ -28,17 +29,25 @@ type TracePageSearchBarProps = { prevResult: () => void; nextResult: () => void; clearSearch: () => void; + focusUiFindMatches: () => void; resultCount: number; navigable: boolean; }; export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwardedRef: React.Ref }) { - const { clearSearch, forwardedRef, navigable, nextResult, prevResult, resultCount, textFilter } = props; + const { + clearSearch, + focusUiFindMatches, + forwardedRef, + navigable, + nextResult, + prevResult, + resultCount, + textFilter, + } = props; const count = textFilter ? {resultCount} : null; - const navigationBtnDisabled = !navigable || !textFilter; - const navigationBtnClass = cx('TracePageSearchBar--btn', { 'is-disabled': navigationBtnDisabled }); const btnClass = cx('TracePageSearchBar--btn', { 'is-disabled': !textFilter }); const uiFindInputInputProps = { 'data-test': markers.IN_TRACE_SEARCH, @@ -48,28 +57,40 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde }; return ( -
+
{/* style inline because compact overwrites the display */} - + - + - + {spanID} +
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx index 04ff414df9..e78da95ce2 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.tsx @@ -19,18 +19,16 @@ 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'; type SpanDetailRowProps = { - addToUiFind: (spanID: string) => void; color: string; 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; @@ -46,12 +44,11 @@ export default class SpanDetailRow extends React.PureComponent { const { linksGetter, span } = this.props; - return linksGetter ? linksGetter(span, items, itemIndex) : []; + return linksGetter(span, items, itemIndex); }; render() { const { - addToUiFind, color, columnDivision, detailState, @@ -79,7 +76,6 @@ export default class SpanDetailRow extends React.PureComponent
', () => { const props = { childrenHiddenIDs: new Set(), childrenToggle: jest.fn(), + clearShouldScrollToFirstUiFindMatch: jest.fn(), currentViewRangeTime: [0.25, 0.75], detailLogItemToggle: jest.fn(), detailLogsToggle: jest.fn(), @@ -41,8 +42,10 @@ describe('', () => { detailToggle: jest.fn(), findMatchesIDs: null, registerAccessors: jest.fn(), + scrollToFirstVisibleSpan: jest.fn(), setSpanNameColumnWidth: jest.fn(), setTrace: jest.fn(), + shouldScrollToFirstUiFindMatch: false, spanNameColumnWidth: 0.5, trace, uiFind: 'uiFind', @@ -343,4 +346,48 @@ describe('', () => { ).toBe(true); }); }); + + describe('shouldScrollToFirstUiFindMatch', () => { + const propsWithTrueShouldScrollToFirstUiFindMatch = { ...props, shouldScrollToFirstUiFindMatch: true }; + + beforeEach(() => { + props.scrollToFirstVisibleSpan.mockReset(); + props.clearShouldScrollToFirstUiFindMatch.mockReset(); + }); + + it('calls props.scrollToFirstVisibleSpan if shouldScrollToFirstUiFindMatch is true', () => { + expect(props.scrollToFirstVisibleSpan).not.toHaveBeenCalled(); + expect(props.clearShouldScrollToFirstUiFindMatch).not.toHaveBeenCalled(); + + wrapper.setProps(propsWithTrueShouldScrollToFirstUiFindMatch); + expect(props.scrollToFirstVisibleSpan).toHaveBeenCalledTimes(1); + expect(props.clearShouldScrollToFirstUiFindMatch).toHaveBeenCalledTimes(1); + }); + + describe('shouldComponentUpdate', () => { + it('returns true if props.shouldScrollToFirstUiFindMatch changes to true', () => { + expect(wrapper.instance().shouldComponentUpdate(propsWithTrueShouldScrollToFirstUiFindMatch)).toBe( + true + ); + }); + + it('returns true if props.shouldScrollToFirstUiFindMatch changes to false and another props change', () => { + const propsWithOtherDifferenceAndTrueshouldScrollToFirstUiFindMatch = { + ...propsWithTrueShouldScrollToFirstUiFindMatch, + clearShouldScrollToFirstUiFindMatch: () => {}, + }; + wrapper.setProps(propsWithOtherDifferenceAndTrueshouldScrollToFirstUiFindMatch); + expect(wrapper.instance().shouldComponentUpdate(props)).toBe(true); + }); + + it('returns false if props.shouldScrollToFirstUiFindMatch changes to false and no other props change', () => { + 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 4a2f750805..55e1d66ecf 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'; @@ -52,12 +51,14 @@ type RowState = { type TVirtualizedTraceViewOwnProps = { currentViewRangeTime: [number, number]; findMatchesIDs: Set | TNil; + scrollToFirstVisibleSpan: () => void; registerAccessors: (accesors: Accessors) => void; trace: Trace; }; type TDispatchProps = { childrenToggle: (spanID: string) => void; + clearShouldScrollToFirstUiFindMatch: () => void; detailLogItemToggle: (spanID: string, log: Log) => void; detailLogsToggle: (spanID: string) => void; detailProcessToggle: (spanID: string) => void; @@ -134,7 +135,7 @@ function getCssClasses(currentViewRange: [number, number]) { } // export from tests -export class VirtualizedTraceViewImpl extends React.PureComponent { +export class VirtualizedTraceViewImpl extends React.Component { clippingCssClasses: string; listView: ListView | TNil; rowStates: RowState[]; @@ -158,6 +159,22 @@ export class VirtualizedTraceViewImpl extends React.PureComponent { - const { uiFind, history, location } = this.props; - if (!uiFind || !uiFind.includes(addition)) { - updateUiFind({ - history, - location, - uiFind: cx(uiFind, addition), - }); + componentDidUpdate() { + const { + shouldScrollToFirstUiFindMatch, + clearShouldScrollToFirstUiFindMatch, + scrollToFirstVisibleSpan, + } = this.props; + if (shouldScrollToFirstUiFindMatch) { + scrollToFirstVisibleSpan(); + clearShouldScrollToFirstUiFindMatch(); } - }; + } getAccessors() { const lv = this.listView; @@ -367,7 +385,6 @@ export class VirtualizedTraceViewImpl extends React.PureComponent ): 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 ec0405c612..f25066d929 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.test.js @@ -51,101 +51,162 @@ 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; + it('indicates the need to scroll iff there are uiFindMatches', () => { + expect(state.shouldScrollToFirstUiFindMatch).toBe(true); - 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(); - }); + filterSpansSpy.mockReturnValue(new Set()); + const singleSpecStore = createStore(reducer, newInitialState()); + singleSpecStore.dispatch(action); + const singleSpecState = singleSpecStore.getState(); + expect(singleSpecState.shouldScrollToFirstUiFindMatch).toBe(false); + }); - it('adds a detailState for each span matching the uiFind filter', () => { - // Sanity check - expect(trace.spans).toHaveLength(30); - expect(uiFindMatches.size).toBe(3); + it('returns existing state if uiFind is falsy', () => { + const emptyStringAction = actions.focusUiFindMatches(trace, ''); + focusUiFindMatchesStore.dispatch(emptyStringAction); + expect(focusUiFindMatchesStore.getState()).toBe(state); - expect(filterSpansSpy).toHaveBeenCalledWith(uiFind, trace.spans); - trace.spans.forEach(({ spanID }) => { - expect(state.detailStates.has(spanID)).toBe(uiFindMatches.has(spanID)); - }); - }); + const nullAction = actions.focusUiFindMatches(trace, null); + focusUiFindMatchesStore.dispatch(nullAction); + 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 undefinedAction = actions.focusUiFindMatches(trace, undefined); + focusUiFindMatchesStore.dispatch(undefinedAction); + expect(focusUiFindMatchesStore.getState()).toBe(state); + }); + }); - trace.spans.forEach(({ spanID }) => { - expect(state.childrenHiddenIDs.has(spanID)).toBe(!uiFindAncestorIdsSet.has(spanID)); - }); - }); + describe('setTrace', () => { + beforeEach(() => { + filterSpansSpy.mockClear(); + }); + + 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); + expect(filterSpansSpy).not.toHaveBeenCalled(); + + store.dispatch(actions.setTrace(Object.assign({}, trace, { traceID: `${trace.traceID}_1` }), null)); + expect(filterSpansSpy).not.toHaveBeenCalled(); + + store.dispatch(actions.setTrace(Object.assign({}, trace, { traceID: `${trace.traceID}_2` }), '')); + expect(filterSpansSpy).not.toHaveBeenCalled(); + + 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); }); }); @@ -348,13 +409,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/TraceTimelineViewer/duck.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx index a5dc27668b..b5741442f2 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', @@ -68,67 +70,87 @@ export const actionTypes = generateActionTypes('@jaeger-ui/trace-timeline-viewer 'DETAIL_LOG_ITEM_TOGGLE', 'EXPAND_ALL', 'EXPAND_ONE', + 'FOCUS_UI_FIND_MATCHES', 'REMOVE_HOVER_INDENT_GUIDE_ID', 'SET_SPAN_NAME_COLUMN_WIDTH', 'SET_TRACE', ]); const fullActions = createActions({ - [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.CLEAR_SHOULD_SCROLL_TO_FIRST_UI_FIND_MATCH]: () => ({}), [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_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 }), }); 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 calculateFocusedFindRowStates(uiFind: string, spans: Span[]) { const spansMap = new Map(); - const childrenHiddenIDs = new Set(); - const detailStates = new Map(); + const childrenHiddenIDs: Set = new Set(); + const detailStates: Map = new Map(); + let shouldScrollToFirstUiFindMatch: boolean = false; - spans.forEach((span: Span) => { + spans.forEach(span => { spansMap.set(span.spanID, 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()); spanAncestorIds(span).forEach(ancestorID => childrenHiddenIDs.delete(ancestorID)); }); + shouldScrollToFirstUiFindMatch = true; } return { - ...newInitialState(), - spanNameColumnWidth, childrenHiddenIDs, detailStates, - traceID, + shouldScrollToFirstUiFindMatch, }; } +function focusUiFindMatches(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) { + if (!uiFind) return state; + return { + ...state, + ...calculateFocusedFindRowStates(uiFind, trace.spans), + }; +} + +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) { + return state; + } + const { spanNameColumnWidth } = state; + + return Object.assign( + { ...newInitialState(), spanNameColumnWidth, traceID }, + uiFind ? calculateFocusedFindRowStates(uiFind, spans) : null + ); +} + function setColumnWidth(state: TTraceTimeline, { width }: TWidthValue): TTraceTimeline { return { ...state, spanNameColumnWidth: width }; } @@ -269,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), @@ -278,6 +303,7 @@ export default handleActions( [actionTypes.DETAIL_TOGGLE]: guardReducer(detailToggle), [actionTypes.EXPAND_ALL]: guardReducer(expandAll), [actionTypes.EXPAND_ONE]: guardReducer(expandOne), + [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/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 | TNil; + scrollToFirstVisibleSpan: () => void; spanNameColumnWidth: number; trace: Trace; updateNextViewRangeTime: (update: ViewRangeTimeUpdate) => void; diff --git a/packages/jaeger-ui/src/components/TracePage/index.test.js b/packages/jaeger-ui/src/components/TracePage/index.test.js index 42a36d132d..b1ab787009 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('', () => { const defaultProps = { acknowledgeArchive: () => {}, fetchTrace() {}, + focusUiFindMatches: jest.fn(), id: trace.traceID, history: { replace: () => {}, @@ -132,6 +135,26 @@ describe('', () => { }); }); + describe('focusUiFindMatches', () => { + beforeEach(() => { + defaultProps.focusUiFindMatches.mockReset(); + }); + + 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); + }); + + it('handles when props.trace.data is absent', () => { + const propFn = wrapper.find(TracePageHeader).prop('focusUiFindMatches'); + wrapper.setProps({ trace: {} }); + propFn(); + expect(defaultProps.focusUiFindMatches).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 +354,17 @@ describe('', () => { }); 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 +373,26 @@ describe('', () => { 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 +508,7 @@ describe('', () => { let header; let spanGraph; let timeline; + let calculateTraceDagEVSpy; function refreshWrappers() { header = wrapper.find(TracePageHeader); @@ -465,6 +516,10 @@ describe('', () => { timeline = wrapper.find(TraceTimelineViewer); } + beforeAll(() => { + calculateTraceDagEVSpy = jest.spyOn(calculateTraceDagEV, 'default'); + }); + beforeEach(() => { wrapper = mount(); // use the method directly because it is a `ref` prop @@ -490,6 +545,15 @@ describe('', () => { 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( + + ); + expect(wrapper.state('slimView')).toBe(true); + }); + it('propagates slimView changes', () => { const { onSlimViewClicked } = header.props(); expect(header.prop('slimView')).toBe(false); @@ -516,6 +580,11 @@ describe('', () => { 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', () => { @@ -581,9 +650,10 @@ describe('', () => { 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 3a2ff5874e..9331dddca6 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; + focusUiFindMatches: (trace: Trace, uiFind: string | TNil) => void; }; type TOwnProps = { @@ -179,10 +182,8 @@ export class TracePageImpl extends React.PureComponent { } 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) { @@ -201,13 +202,11 @@ export class TracePageImpl extends React.PureComponent { 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) { @@ -306,6 +305,13 @@ export class TracePageImpl extends React.PureComponent { } } + focusUiFindMatches = () => { + const { trace, focusUiFindMatches, uiFind } = this.props; + if (trace && trace.data) { + focusUiFindMatches(trace.data, uiFind); + } + }; + render() { const { archiveEnabled, archiveTraceState, embedded, id, searchUrl, uiFind, trace } = this.props; const { slimView, traceGraphView, headerHeight, viewRange } = this.state; @@ -317,7 +323,6 @@ export class TracePageImpl extends React.PureComponent { return ; } - // $FlowIgnore because flow believes Set cannot be assigned to Set let findCount = 0; let graphFindMatches; let spanFindMatches; @@ -330,10 +335,10 @@ export class TracePageImpl extends React.PureComponent { findCount = spanFindMatches ? spanFindMatches.size : 0; } } - // = ? : null; - // const spanFindMatches = !traceGraphView ? this._filterSpans(uiFind || '', _get(trace, 'data.spans')) : null; + const isEmbedded = Boolean(embedded); const headerProps = { + focusUiFindMatches: this.focusUiFindMatches, slimView, textFilter: uiFind, traceGraphView, @@ -382,6 +387,7 @@ export class TracePageImpl extends React.PureComponent {
): TDispatchProps { const { fetchTrace } = bindActionCreators(jaegerApiActions, dispatch); const { archiveTrace, acknowledge: acknowledgeArchive } = bindActionCreators(archiveActions, dispatch); - return { acknowledgeArchive, archiveTrace, fetchTrace }; + const { focusUiFindMatches } = bindActionCreators(timelineActions, dispatch); + return { acknowledgeArchive, archiveTrace, fetchTrace, focusUiFindMatches }; } export default connect(mapStateToProps, mapDispatchToProps)(TracePageImpl); 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 f94adacfac..d7b97efed7 100644 --- a/packages/jaeger-ui/src/components/common/CopyIcon.test.js +++ b/packages/jaeger-ui/src/components/common/CopyIcon.test.js @@ -14,19 +14,28 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { Icon, Tooltip } from 'antd'; +import { Button, Tooltip } from 'antd'; +import * as copy from 'copy-to-clipboard'; import CopyIcon from './CopyIcon'; +jest.mock('copy-to-clipboard'); + describe('', () => { const props = { className: 'classNameValue', copyText: 'copyTextValue', tooltipTitle: 'tooltipTitleValue', }; + let copySpy; let wrapper; + beforeAll(() => { + copySpy = jest.spyOn(copy, 'default'); + }); + beforeEach(() => { + copySpy.mockReset(); wrapper = shallow(); }); @@ -34,10 +43,13 @@ describe('', () => { expect(wrapper).toMatchSnapshot(); }); - it('updates state when icon is clicked', () => { + it('updates state and copies when clicked', () => { expect(wrapper.state().hasCopied).toBe(false); - wrapper.find(Icon).simulate('click'); + 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 7c6d1dd932..546da2b04d 100644 --- a/packages/jaeger-ui/src/components/common/CopyIcon.tsx +++ b/packages/jaeger-ui/src/components/common/CopyIcon.tsx @@ -14,12 +14,18 @@ import * as React from 'react'; -import { Icon, Tooltip } from 'antd'; -import CopyToClipboard from 'react-copy-to-clipboard'; +import { Button, Tooltip } from 'antd'; +import { TooltipPlacement } from 'antd/lib/tooltip/index'; +import cx from 'classnames'; +import copy from 'copy-to-clipboard'; + +import './CopyIcon.css'; type PropsType = { className?: string; copyText: string; + icon?: string; + placement?: TooltipPlacement; tooltipTitle: string; }; @@ -30,6 +36,8 @@ type StateType = { export default class CopyIcon extends React.PureComponent { static defaultProps: Partial = { className: undefined, + icon: 'copy', + placement: 'left', }; state = { @@ -40,6 +48,7 @@ export default class CopyIcon extends React.PureComponent this.setState({ hasCopied: true, }); + copy(this.props.copyText); }; handleTooltipVisibilityChange = (visible: boolean) => { @@ -56,12 +65,15 @@ export default class CopyIcon extends React.PureComponent arrowPointAtCenter mouseLeaveDelay={0.5} onVisibleChange={this.handleTooltipVisibilityChange} - placement="left" + placement={this.props.placement} title={this.state.hasCopied ? 'Copied' : this.props.tooltipTitle} > - - - +