From 939104c3f09d1fdb97ca151ef1c01501e8894c3f 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 | 25 ++- .../src/components/SearchTracePage/index.js | 21 +-- .../components/TracePage/TracePageHeader.css | 11 +- .../components/TracePage/TracePageHeader.js | 106 ++++------- .../TracePage/TracePageHeaderEmbed.js | 174 ++++++++++++++++++ .../src/components/TracePage/index.js | 52 +++--- 8 files changed, 255 insertions(+), 143 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..55ef85436e 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,7 +153,7 @@ export default class SearchResults extends React.PureComponent
- {!disableComparision && diffSelection} + {diffSelection}
    {traces.map(trace => (
  • @@ -166,7 +163,7 @@ export default class SearchResults extends React.PureComponent
    • ))} diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.js b/packages/jaeger-ui/src/components/SearchTracePage/index.js index 90738b4268..0c2c509089 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 !== undefined, + hideGraph: hideGraph !== undefined, isHomepage, loadingServices, loadingTraces, 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..a98a5483c0 --- /dev/null +++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeaderEmbed.js @@ -0,0 +1,174 @@ +// @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 { 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, + onGoBackClicked: () => void, + 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, + onGoBackClicked, + onGoFullViewClicked, + updateTextFilter, + textFilter, + prevResult, + nextResult, + clearSearch, + resultCount, + forwardedRef, + details, + } = 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 ( +
+
+ +

+ {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..b62cee5738 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'; @@ -162,9 +163,6 @@ 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 && ( )}
- + {isEmbed ? : } {((!slimView && !isEmbed) || (isEmbed && minimap)) && (