Skip to content

Commit

Permalink
Revisions for search and trace detail embed mode (#286)
Browse files Browse the repository at this point in the history
* Misc tweaks for search and trace detail embed mode

Mostly from prior comments.

- Rename query parameter for embedding to start with "ui" and use the
  page as the first word, e.g. "uiSearchHideGraph"

- Change query parameters for the minimap and trace details from hiding
  to showing, e.g. "hidemap" -> "uiTimelineShowMap"

- Save the embed query params in Redux state instead of passing them
  around

- Use a Link with an icon instead of text buttons for opening the
  standalone view of the page

- Propagate whether the trace detail page is from the search page or
  not via the Location#state member on the React Router Location

- When returning to the search page use the previous results instead
  of executing a new search. This is done by storing the query with
  the search results.

- Adjusted aesthetic of "Back to Search" button on trace detail page

- Sequester parsing and stripping query parameters for the embed mode
  to a util

- In various places switch to using the component/*/url.js#getUrl
  functions instead of prefixUrl(...)

Signed-off-by: Joe Farro <joef@uber.com>

* Fix test break from merging master

Signed-off-by: Joe Farro <joef@uber.com>

* Keep redux search query synced with results

- Keep redux search query synced with redux search result (and their
  processing). Also fixes #288.

- Bolster unit tests

Signed-off-by: Joe Farro <joef@uber.com>

* Fix typo

Signed-off-by: Joe Farro <joef@uber.com>

* Make TracePageHeader collapsible when embedded

- Reconfigured embed query parameters for timeline:

  - uiTimelineCollapseTitle=1 - TracePageHeader starts out collapsed

  - uiTimelineHideMinimap=1 - TracePageHeader does not show the minimap

  - uiTimelineHideSummary=1 - TracePageHeader does not show the trace
    summary

- Consolidate TracePageHeader and TracePageHeaderEmbed

- Style changes to TracePageHeader

- Embedded TracePageHeader can now be expanded and collapsed

- Misc cleanup in TracePageHeader

- Better comparisons for search page query to prevent re-fetching
  when returning to the search page

Signed-off-by: Joe Farro <joef@uber.com>

* Fix typo disableComparisions

Signed-off-by: Joe Farro <joef@uber.com>

* Use public registry to newly installed packages

Signed-off-by: Joe Farro <joef@uber.com>

* Test improvements

Signed-off-by: Joe Farro <joef@uber.com>
  • Loading branch information
tiffon authored Dec 20, 2018
1 parent 0a36984 commit 53937eb
Show file tree
Hide file tree
Showing 77 changed files with 1,421 additions and 1,105 deletions.
6 changes: 4 additions & 2 deletions packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
"react-app-rewired": "^1.4.0",
"react-scripts": "^1.0.11",
"react-test-renderer": "^15.6.1",
"sinon": "^3.2.1"
"sinon": "^3.2.1",
"source-map-explorer": "^1.6.0"
},
"dependencies": {
"@jaegertracing/plexus": "0.0.1-dev.3",
Expand Down Expand Up @@ -59,7 +60,7 @@
"react-dom": "^16.3.2",
"react-ga": "^2.4.1",
"react-helmet": "^5.1.3",
"react-icons": "^2.2.7",
"react-icons": "2.2.7",
"react-metrics": "^2.3.2",
"react-redux": "^5.0.6",
"react-router-dom": "^4.1.2",
Expand All @@ -79,6 +80,7 @@
"u-basscss": "2.0.0"
},
"scripts": {
"analyze": "source-map-explorer build/static/js/main.*",
"build": "REACT_APP_VSN_STATE=$(../../scripts/get-tracking-version.js) react-app-rewired build",
"coverage": "yarn run test -- --coverage",
"start:ga-debug":
Expand Down
21 changes: 14 additions & 7 deletions packages/jaeger-ui/src/components/App/Page.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,24 @@

import * as React from 'react';
import { Layout } from 'antd';
import cx from 'classnames';
import Helmet from 'react-helmet';
import { connect } from 'react-redux';
import type { Location } from 'react-router-dom';
import { withRouter } from 'react-router-dom';
import { isEmbed } from '../../utils/embedded';

import TopNav from './TopNav';
import { trackPageView } from '../../utils/tracking';

import type { ReduxState } from '../../types';
import type { EmbeddedState } from '../../types/embedded';

import './Page.css';

type Props = {
children: React.Node,
embedded: EmbeddedState,
pathname: string,
search: string,
children: React.Node,
};

const { Header, Content } = Layout;
Expand All @@ -52,26 +56,29 @@ export class PageImpl extends React.Component<Props> {
}

render() {
const { embedded } = this.props;
const contentCls = cx({ 'Page--content': !embedded });
return (
<div>
<Helmet title="Jaeger UI" />
<Layout>
{!isEmbed(this.props.search) && (
{!embedded && (
<Header className="Page--topNav">
<TopNav />
</Header>
)}
<Content className={!isEmbed(this.props.search) && 'Page--content'}>{this.props.children}</Content>
<Content className={contentCls}>{this.props.children}</Content>
</Layout>
</div>
);
}
}

// export for tests
export function mapStateToProps(state: { router: { location: Location } }) {
export function mapStateToProps(state: ReduxState) {
const { embedded } = state;
const { pathname, search } = state.router.location;
return { pathname, search };
return { embedded, pathname, search };
}

export default withRouter(connect(mapStateToProps)(PageImpl));
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/App/TraceIDSearchInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { withRouter } from 'react-router-dom';

import type { RouterHistory } from 'react-router-dom';

import prefixUrl from '../../utils/prefix-url';
import { getUrl } from '../../components/TracePage/url';

import './TraceIDSearchInput.css';

Expand All @@ -35,7 +35,7 @@ class TraceIDSearchInput extends React.PureComponent<Props> {
event.preventDefault();
const value = event.target.elements.idInput.value;
if (value) {
this.props.history.push(prefixUrl(`/trace/${value}`));
this.props.history.push(getUrl(value));
}
};

Expand Down
3 changes: 2 additions & 1 deletion packages/jaeger-ui/src/components/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ import JaegerAPI, { DEFAULT_API_ROOT } from '../../api/jaeger';
import configureStore from '../../utils/configure-store';
import prefixUrl from '../../utils/prefix-url';

import './index.css';
import '../common/vars.css';
import '../common/utils.css';
import './index.css';

const history = createHistory();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export default class ResultItemTitle extends React.PureComponent<Props> {
traceName,
disableComparision,
} = this.props;
// Use a div when the ResultItemTitle doesn't link to anything
let WrapperComponent = 'div';
const wrapperProps: { [string]: string } = { className: 'ResultItemTitle--item ub-flex-auto' };
if (linkTo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ limitations under the License.

.SearchResults--header {
border: 1px solid #e6e6e6;
color: rgba(0, 0, 0, 0.85);
color: var(--tx-color-title);
margin-bottom: 1.25rem;
}

.SearchResults--headerOverview {
align-items: center;
background-color: #f5f5f5;
border-top: 1px solid #e6e6e6;
display: flex;
padding: 1rem;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,39 @@
// limitations under the License.

import * as React from 'react';
import { Select, Button } from 'antd';
import { Select } from 'antd';
import { Link } from 'react-router-dom';
import { Field, reduxForm, formValueSelector } from 'redux-form';

import DiffSelection from './DiffSelection';
import * as markers from './index.markers';
import ResultItem from './ResultItem';
import ScatterPlot from './ScatterPlot';
import { getUrl } from '../url';
import LoadingIndicator from '../../common/LoadingIndicator';
import NewWindowIcon from '../../common/NewWindowIcon';
import { getLocation } from '../../TracePage/url';
import * as orderBy from '../../../model/order-by';
import { getPercentageOfDuration } from '../../../utils/date';
import prefixUrl from '../../../utils/prefix-url';
import { stripEmbeddedState } from '../../../utils/embedded-url';
import reduxFormFieldAdapter from '../../../utils/redux-form-field-adapter';
import { VERSION_API } from '../../../utils/embedded';

import type { FetchedTrace } from '../../../types';
import type { SearchQuery } from '../../../types/search';

import './index.css';

type SearchResultsProps = {
cohortAddTrace: string => void,
cohortRemoveTrace: string => void,
diffCohort: FetchedTrace[],
disableComparisons: boolean,
goToTrace: string => void,
embed?: boolean,
hideGraph?: boolean,
hideGraph: boolean,
loading: boolean,
getSearchURL: () => string,
maxTraceDuration: number,
queryOfResults: SearchQuery,
showStandaloneLink: boolean,
skipMessage?: boolean,
traces: TraceSummary[],
};
Expand All @@ -54,7 +59,7 @@ const Option = Select.Option;
*/
function SelectSortImpl() {
return (
<label className="ub-right">
<label>
Sort:{' '}
<Field name="sortBy" component={reduxFormFieldAdapter(Select)}>
<Option value={orderBy.MOST_RECENT}>Most Recent</Option>
Expand Down Expand Up @@ -90,17 +95,18 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp

render() {
const {
loading,
diffCohort,
skipMessage,
traces,
disableComparisons,
goToTrace,
embed,
hideGraph,
getSearchURL,
loading,
maxTraceDuration,
queryOfResults,
showStandaloneLink,
skipMessage,
traces,
} = this.props;
const diffSelection = !embed && (
const diffSelection = !disableComparisons && (
<DiffSelection toggleComparison={this.toggleComparison} traces={diffCohort} />
);
if (loading) {
Expand All @@ -124,11 +130,12 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
);
}
const cohortIds = new Set(diffCohort.map(datum => datum.id));
const searchUrl = getUrl(stripEmbeddedState(queryOfResults));
return (
<div>
<div>
<div className="SearchResults--header">
{(!embed || !hideGraph) && (
{!hideGraph && (
<div className="ub-p3">
<ScatterPlot
data={traces.map(t => ({
Expand All @@ -145,22 +152,20 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
</div>
)}
<div className="SearchResults--headerOverview">
<SelectSort />
{embed && (
<label className="ub-right">
<Button
className="ub-mr2 ub-items-center"
icon="export"
target="_blank"
href={getSearchURL()}
>
View Results
</Button>
</label>
)}
<h2 className="ub-m0">
<h2 className="ub-m0 u-flex-1">
{traces.length} Trace{traces.length > 1 && 's'}
</h2>
<SelectSort />
{showStandaloneLink && (
<Link
className="u-tx-inherit ub-nowrap ub-ml3"
to={searchUrl}
target="_blank"
rel="noopener noreferrer"
>
<NewWindowIcon />
</Link>
)}
</div>
</div>
</div>
Expand All @@ -172,16 +177,10 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
<ResultItem
durationPercent={getPercentageOfDuration(trace.duration, maxTraceDuration)}
isInDiffCohort={cohortIds.has(trace.traceID)}
linkTo={prefixUrl(
embed
? `/trace/${trace.traceID}?embed=${VERSION_API}&fromSearch=${encodeURIComponent(
getSearchURL()
)}`
: `/trace/${trace.traceID}`
)}
linkTo={getLocation(trace.traceID, { fromSearch: searchUrl })}
toggleComparison={this.toggleComparison}
trace={trace}
disableComparision={embed}
disableComparision={disableComparisons}
/>
</li>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ describe('<SearchResults>', () => {
expect(wrapper.find(ScatterPlot).length).toBe(0);
});

it('hide DiffSelection if is an embedded component', () => {
wrapper.setProps({ embed: true, getSearchURL: () => 'SEARCH_URL' });
it('hide DiffSelection when disableComparisons = true', () => {
wrapper.setProps({ disableComparisons: true });
expect(wrapper.find(DiffSelection).length).toBe(0);
});

Expand Down
Loading

0 comments on commit 53937eb

Please sign in to comment.