From 83bc7d0472539782452f9ebbcf0fbc2c499e1e23 Mon Sep 17 00:00:00 2001 From: Alberto Gutierrez Date: Sun, 11 Nov 2018 21:15:48 +0100 Subject: [PATCH] Commnets addressed Signed-off-by: Alberto Gutierrez --- packages/jaeger-ui/src/components/App/Page.js | 7 +- .../SearchResults/ResultItem.js | 2 +- .../SearchTracePage/SearchResults/index.js | 29 ++- .../SearchResults/index.test.js | 6 - .../src/components/SearchTracePage/index.js | 21 +- .../components/SearchTracePage/index.test.js | 11 -- .../components/TracePage/TracePageHeader.css | 11 +- .../components/TracePage/TracePageHeader.js | 106 +++-------- .../TracePage/TracePageHeaderEmbed.js | 179 ++++++++++++++++++ .../src/components/TracePage/index.js | 93 +++++---- .../src/components/TracePage/index.test.js | 4 +- 11 files changed, 297 insertions(+), 172 deletions(-) create mode 100644 packages/jaeger-ui/src/components/TracePage/TracePageHeaderEmbed.js diff --git a/packages/jaeger-ui/src/components/App/Page.js b/packages/jaeger-ui/src/components/App/Page.js index c4ea04bfe4..e98b47532c 100644 --- a/packages/jaeger-ui/src/components/App/Page.js +++ b/packages/jaeger-ui/src/components/App/Page.js @@ -15,6 +15,7 @@ // limitations under the License. import * as React from 'react'; +import queryString from 'query-string'; import { Layout } from 'antd'; import Helmet from 'react-helmet'; import { connect } from 'react-redux'; @@ -52,17 +53,17 @@ export class PageImpl extends React.Component { } render() { - const isEmbed = this.props.search.includes('embed'); + const { embed } = queryString.parse(this.props.search); return (
- {!isEmbed && ( + {embed === undefined && (
)} - {this.props.children} + {this.props.children}
); diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.js index fd6c7fddd1..b72d93d7da 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.js @@ -46,10 +46,10 @@ export default class ResultItem extends React.PureComponent { render() { const { + disableComparision, durationPercent, isInDiffCohort, linkTo, - disableComparision, toggleComparison, trace, } = this.props; diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js index 82f8923bcd..657f3805bc 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js @@ -15,7 +15,7 @@ // limitations under the License. import * as React from 'react'; -import { Button, Select } from 'antd'; +import { Select, Button } from 'antd'; import { Field, reduxForm, formValueSelector } from 'redux-form'; import DiffSelection from './DiffSelection'; @@ -39,9 +39,8 @@ type SearchResultsProps = { goToTrace: string => void, isEmbed?: boolean, hideGraph?: boolean, - disableComparision?: boolean, loading: boolean, - onGoFullClicked: () => void, + getSearchURL: () => string, maxTraceDuration: number, skipMessage?: boolean, traces: TraceSummary[], @@ -111,14 +110,7 @@ export default class SearchResults extends React.PureComponent ); } - const { - goToTrace, - isEmbed, - hideGraph, - disableComparision, - onGoFullClicked, - maxTraceDuration, - } = this.props; + const { goToTrace, isEmbed, hideGraph, getSearchURL, maxTraceDuration } = this.props; const cohortIds = new Set(diffCohort.map(datum => datum.id)); return (
@@ -144,7 +136,12 @@ export default class SearchResults extends React.PureComponent {isEmbed && ( @@ -156,17 +153,19 @@ export default class SearchResults extends React.PureComponent
- {!disableComparision && diffSelection} + {diffSelection}
    {traces.map(trace => (
    • ))} diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index b70d365df7..0242f0fe15 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -20,7 +20,6 @@ import * as markers from './index.markers'; import ResultItem from './ResultItem'; import ScatterPlot from './ScatterPlot'; import LoadingIndicator from '../../common/LoadingIndicator'; -import DiffSelection from './DiffSelection'; describe('', () => { let wrapper; @@ -54,11 +53,6 @@ describe('', () => { expect(wrapper.find(ScatterPlot).length).toBe(0); }); - it('hide DiffSelection if queryparam disableComparision', () => { - wrapper.setProps({ disableComparision: true }); - expect(wrapper.find(DiffSelection).length).toBe(0); - }); - describe('search finished with results', () => { it('shows a scatter plot', () => { expect(wrapper.find(ScatterPlot).length).toBe(1); diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.js b/packages/jaeger-ui/src/components/SearchTracePage/index.js index 90738b4268..b951e35981 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/index.js @@ -64,11 +64,10 @@ export class SearchTracePageImpl extends Component { this.props.history.push(prefixUrl(url)); }; - goFullView = () => { + getSearchURL = () => { const urlQuery = this.props.query; delete urlQuery.embed; - const url = `/search?${queryString.stringify(urlQuery)}`; - window.open(prefixUrl(url), '_blank'); + return `/search?${queryString.stringify(urlQuery)}`; }; render() { @@ -85,7 +84,6 @@ export class SearchTracePageImpl extends Component { traceResults, isEmbed, hideGraph, - disableComparision, } = this.props; const hasTraceResults = traceResults && traceResults.length > 0; const showErrors = errors && !loadingTraces; @@ -101,7 +99,7 @@ export class SearchTracePageImpl extends Component {
)} - + {showErrors && (

There was an error querying for traces:

@@ -117,11 +115,10 @@ export class SearchTracePageImpl extends Component { cohortRemoveTrace={cohortRemoveTrace} diffCohort={diffCohort} skipMessage={isHomepage} - onGoFullClicked={this.goFullView} + getSearchURL={this.getSearchURL} traces={traceResults} isEmbed={isEmbed} hideGraph={hideGraph} - disableComparision={disableComparision} /> )} {showLogo && @@ -145,7 +142,6 @@ SearchTracePageImpl.propTypes = { isHomepage: PropTypes.bool, isEmbed: PropTypes.bool, hideGraph: PropTypes.bool, - disableComparision: PropTypes.bool, // eslint-disable-next-line react/forbid-prop-types traceResults: PropTypes.array, diffCohort: PropTypes.array, @@ -218,9 +214,7 @@ const stateServicesXformer = getLastXformCacher(stateServices => { // export to test export function mapStateToProps(state) { const query = queryString.parse(state.router.location.search); - const isEmbed = 'embed' in query; - const hideGraph = 'hideGraph' in query; - const disableComparision = 'disableComparision' in query; + const { embed, hideGraph } = queryString.parse(state.router.location.search); const isHomepage = !Object.keys(query).length; const { traces, maxDuration, traceError, loadingTraces } = stateTraceXformer(state.trace); const diffCohort = stateTraceDiffXformer(state.trace, state.traceDiff); @@ -237,9 +231,8 @@ export function mapStateToProps(state) { return { query, diffCohort, - isEmbed, - hideGraph, - disableComparision, + isEmbed: embed === 'v0', + hideGraph: hideGraph !== undefined, isHomepage, loadingServices, loadingTraces, diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/index.test.js index 904151dfa2..faee55db1f 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/index.test.js @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import prefixUrl from '../../utils/prefix-url'; - jest.mock('redux-form', () => { function reduxForm() { return component => component; @@ -51,7 +49,6 @@ describe('', () => { props = { traceResults, isEmbed: false, - disableComparision: false, isHomepage: false, loadingServices: false, loadingTraces: false, @@ -114,13 +111,6 @@ describe('', () => { wrapper.setProps({ isEmbed: true }); expect(wrapper.find('.js-test-logo').length).toBe(0); }); - - it('open a window when goFullView is called', () => { - wrapper.setProps({ query: { embed: 'embed', service: 'jaeger-query' } }); - global.open = jest.fn(); - wrapper.instance().goFullView(); - expect(global.open).toBeCalledWith(prefixUrl('/search?service=jaeger-query'), '_blank'); - }); }); describe('mapStateToProps()', () => { @@ -163,7 +153,6 @@ describe('mapStateToProps()', () => { expect(rest).toEqual({ isEmbed: false, hideGraph: false, - disableComparision: false, query: {}, isHomepage: true, // the redux-form `formValueSelector` mock returns `null` for "sortBy" diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader.css b/packages/jaeger-ui/src/components/TracePage/TracePageHeader.css index fb663ad358..1fb0c04bd4 100644 --- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader.css +++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader.css @@ -19,10 +19,6 @@ limitations under the License. display: flex; } -Divider { - color: red; -} - .TracePageHeader--titleRowEmbed { align-items: center; background-color: #ececec; @@ -30,15 +26,12 @@ Divider { display: flex; } -.TracePageHeader--title { +.TracePageHeader--title, +TracePageHeader--titleEmbed { margin: 0; padding: 0.25rem 0.5rem; } -.TracePageHeader--titleEmbed { - margin: 0; - padding: 0.25rem 0.5rem; -} .TracePageHeader--title:hover { background: #f5f5f5; } diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeader.js index a657ffa704..8689264b7c 100644 --- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader.js +++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader.js @@ -15,10 +15,9 @@ // limitations under the License. import * as React from 'react'; -import { Button, Dropdown, Icon, Input, Menu, Divider } from 'antd'; +import { Button, Dropdown, Icon, Input, Menu } from 'antd'; import IoChevronDown from 'react-icons/lib/io/chevron-down'; import IoChevronRight from 'react-icons/lib/io/chevron-right'; -import IoChevronLeft from 'react-icons/lib/io/chevron-left'; import IoIosFilingOutline from 'react-icons/lib/io/ios-filing-outline'; import { Link } from 'react-router-dom'; @@ -46,10 +45,6 @@ type TracePageHeaderProps = { resultCount: number, archiveButtonVisible: boolean, onArchiveClicked: () => void, - onGoBackClicked: () => void, - onGoFullViewClicked: () => void, - embed: boolean, - details: boolean, // these props are used by the `HEADER_ITEMS` // eslint-disable-next-line react/no-unused-prop-types timestamp: number, @@ -109,8 +104,6 @@ export function TracePageHeaderFn(props: TracePageHeaderProps) { name, slimView, onSlimViewClicked, - onGoBackClicked, - onGoFullViewClicked, updateTextFilter, textFilter, prevResult, @@ -118,8 +111,6 @@ export function TracePageHeaderFn(props: TracePageHeaderProps) { clearSearch, resultCount, forwardedRef, - embed, - details, } = props; if (!traceID) { @@ -179,74 +170,39 @@ export function TracePageHeaderFn(props: TracePageHeaderProps) { }, ]; - const embedComponent = ( -
- - Go back - - -

- {name || FALLBACK_TRACE_NAME} -

- - View Trace - - -
- ); - return (
- {embed ? ( - embedComponent - ) : ( -
- -

- {slimView ? : } - {name || FALLBACK_TRACE_NAME} -

- - - - - - +
+ +

+ {slimView ? : } + {name || FALLBACK_TRACE_NAME} +

+ + + + + + - {archiveButtonVisible && ( - - )} -
- )} - {((!slimView && !embed) || (embed && details)) && ( - - )} + {archiveButtonVisible && ( + + )} +
+ {!slimView && }
); } diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeaderEmbed.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeaderEmbed.js new file mode 100644 index 0000000000..8f3fdd1cdd --- /dev/null +++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeaderEmbed.js @@ -0,0 +1,179 @@ +// @flow + +// Copyright (c) 2017 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. + +import * as React from 'react'; +import type { RouterHistory } from 'react-router-dom'; +import { Input, Button } from 'antd'; +import IoChevronLeft from 'react-icons/lib/io/chevron-left'; + +import TracePageSearchBar from './TracePageSearchBar'; +import LabeledList from '../common/LabeledList'; +import { FALLBACK_TRACE_NAME } from '../../constants'; +import { formatDatetime, formatDuration } from '../../utils/date'; + +import './TracePageHeader.css'; + +type TracePageHeaderEmbedProps = { + traceID: string, + name: String, + slimView: boolean, + updateTextFilter: string => void, + textFilter: string, + prevResult: () => void, + nextResult: () => void, + clearSearch: () => void, + forwardedRef: { current: Input | null }, + resultCount: number, + history: RouterHistory, + fromSearch: boolean, + onGoFullViewClicked: () => void, + details: boolean, + // these props are used by the `HEADER_ITEMS` + // eslint-disable-next-line react/no-unused-prop-types + timestamp: number, + // eslint-disable-next-line react/no-unused-prop-types + duration: number, + // eslint-disable-next-line react/no-unused-prop-types + numServices: number, + // eslint-disable-next-line react/no-unused-prop-types + maxDepth: number, + // eslint-disable-next-line react/no-unused-prop-types + numSpans: number, +}; + +export const HEADER_ITEMS = [ + { + key: 'timestamp', + title: 'Trace Start', + propName: null, + renderer: (props: TracePageHeaderEmbedProps) => formatDatetime(props.timestamp), + }, + { + key: 'duration', + title: 'Duration', + propName: null, + renderer: (props: TracePageHeaderEmbedProps) => formatDuration(props.duration), + }, + { + key: 'service-count', + title: 'Services', + propName: 'numServices', + renderer: null, + }, + { + key: 'depth', + title: 'Depth', + propName: 'maxDepth', + renderer: null, + }, + { + key: 'span-count', + title: 'Total Spans', + propName: 'numSpans', + renderer: null, + }, +]; + +export function TracePageHeaderEmbedFn(props: TracePageHeaderEmbedProps) { + const { + duration, + maxDepth, + numSpans, + timestamp, + numServices, + traceID, + name, + slimView, + history, + onGoFullViewClicked, + updateTextFilter, + textFilter, + prevResult, + nextResult, + clearSearch, + resultCount, + forwardedRef, + details, + fromSearch, + } = props; + + if (!traceID) { + return null; + } + + const overviewItems = [ + { + key: 'start', + label: 'Trace Start:', + value: formatDatetime(timestamp), + }, + { + key: 'duration', + label: 'Duration:', + value: formatDuration(duration), + }, + { + key: 'svc-count', + label: 'Services:', + value: numServices, + }, + { + key: 'depth', + label: 'Depth:', + value: maxDepth, + }, + { + key: 'span-count', + label: 'Total Spans:', + value: numSpans, + }, + ]; + + return ( +
+
+ {fromSearch && ( + + )} +

+ {name || FALLBACK_TRACE_NAME} +

+ + +
+ {details && + !slimView && } +
+ ); +} + +// ghetto fabulous cast because the 16.3 API is not in flow yet +// https://github.com/facebook/flow/issues/6103 +export default (React: any).forwardRef((props, ref) => ( + +)); diff --git a/packages/jaeger-ui/src/components/TracePage/index.js b/packages/jaeger-ui/src/components/TracePage/index.js index 931c8b9355..f9ee6d3ada 100644 --- a/packages/jaeger-ui/src/components/TracePage/index.js +++ b/packages/jaeger-ui/src/components/TracePage/index.js @@ -33,6 +33,7 @@ import { cancel as cancelScroll, scrollBy, scrollTo } from './scroll-page'; import ScrollManager from './ScrollManager'; import SpanGraph from './SpanGraph'; import TracePageHeader from './TracePageHeader'; +import TracePageHeaderEmbed from './TracePageHeaderEmbed'; import { trackSlimHeaderToggle } from './TracePageHeader.track'; import TraceTimelineViewer from './TraceTimelineViewer'; import ErrorMessage from '../common/ErrorMessage'; @@ -60,9 +61,9 @@ type TracePageProps = { id: string, trace: ?FetchedTrace, isEmbed?: boolean, - minimap?: boolean, details?: boolean, mapCollapsed?: boolean, + fromSearch?: boolean, }; type TracePageState = { @@ -162,9 +163,6 @@ export class TracePageImpl extends React.PureComponent { + console.log(this.props.history); + return true; + }; + goFullView = () => { window.open(prefixUrl(`/trace/${this.props.id.toLowerCase()}`), '_blank'); }; render() { - const { archiveEnabled, archiveTraceState, trace, isEmbed, minimap, details } = this.props; + const { + archiveEnabled, + archiveTraceState, + trace, + isEmbed, + mapCollapsed, + details, + history, + fromSearch, + } = this.props; const { slimView, headerHeight, textFilter, viewRange, findMatchesIDs } = this.state; if (!trace || trace.state === fetchedState.LOADING) { return ; @@ -353,37 +365,41 @@ export class TracePageImpl extends React.PureComponent p.serviceName)).size; + const tracePageProps = { + duration, + maxDepth: maxSpanDepth, + name: getTraceName(spans), + numServices: numberOfServices, + numSpans: spans.length, + slimView, + timestamp: startTime, + traceID, + onSlimViewClicked: this.toggleSlimView, + textFilter, + prevResult: this._scrollManager.scrollToPrevVisibleSpan, + nextResult: this._scrollManager.scrollToNextVisibleSpan, + clearSearch: this.clearSearch, + resultCount: findMatchesIDs ? findMatchesIDs.size : 0, + updateTextFilter: this.updateTextFilter, + archiveButtonVisible: archiveEnabled, + onArchiveClicked: this.archiveTrace, + onGoBackClicked: this.goBackSearch, + onGoFullViewClicked: this.goFullView, + details, + ref: this._searchBar, + }; return (
{archiveEnabled && ( )}
- - {((!slimView && !isEmbed) || (isEmbed && minimap)) && ( + {isEmbed ? ( + + ) : ( + + )} + {((!slimView && !isEmbed) || (isEmbed && !mapCollapsed)) && ( ', () => { }); it('collapse map if queryparam mapCollapsed', () => { - wrapper.setProps({ mapCollapsed: true }); + wrapper.setProps({ mapCollapsed: true, isEmbed: true }); expect(wrapper.find(SpanGraph).length).toBe(0); }); @@ -394,7 +394,7 @@ describe('mapStateToProps()', () => { archiveEnabled: false, isEmbed: false, details: false, - minimap: false, + fromSearch: false, mapCollapsed: false, archiveTraceState: undefined, trace: { data: {}, state: fetchedState.DONE },