Skip to content

Commit

Permalink
Span Search - Add result count, navigation and clear buttons (jaegert…
Browse files Browse the repository at this point in the history
…racing#234)

* Add result count, navigation and clear buttons

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Correct imports

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Move and delete files

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Review fixes

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Fix TracePageHeader test

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Lighten buttons when disabled

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Review fixes

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Fix tests

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Add shortcuts

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Fix merge changes

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Fix TracePageHeader and TracePageSearchBar tests

Signed-off-by: Davit Yeghshatyan <[email protected]>

* Delay TracePageHeader testing until release of Enzyme v3.5.0

Signed-off-by: Davit Yeghshatyan <[email protected]>

Signed-off-by: vvvprabhakar <[email protected]>
  • Loading branch information
davit-y authored and yurishkuro committed Sep 26, 2018
1 parent eff9c27 commit dc4d9f0
Show file tree
Hide file tree
Showing 16 changed files with 260 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const descriptions = {
expandAll: 'Expand All',
collapseOne: 'Collapse One Level',
expandOne: 'Expand One Level',
searchSpans: 'Search Spans',
clearSearch: 'Clear Search',
};

function convertKeys(keyConfig: string | string[]): string[][] {
Expand Down
40 changes: 27 additions & 13 deletions packages/jaeger-ui/src/components/TracePage/TracePageHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import IoChevronRight from 'react-icons/lib/io/chevron-right';
import IoIosFilingOutline from 'react-icons/lib/io/ios-filing-outline';
import { Link } from 'react-router-dom';

import * as markers from './TracePageHeader.markers';
import { trackAltViewOpen } from './TracePageHeader.track';
import KeyboardShortcutsHelp from './KeyboardShortcutsHelp';
import { trackAltViewOpen } from './TracePageHeader.track';
import TracePageSearchBar from './TracePageSearchBar';
import LabeledList from '../common/LabeledList';
import { FALLBACK_TRACE_NAME } from '../../constants';
import { formatDatetime, formatDuration } from '../../utils/date';
Expand All @@ -37,7 +37,12 @@ type TracePageHeaderProps = {
slimView: boolean,
onSlimViewClicked: () => void,
updateTextFilter: string => void,
textFilter: ?string,
textFilter: string,
prevResult: () => void,
nextResult: () => void,
clearSearch: () => void,
forwardedRef: { current: Input | null },
resultCount: number,
archiveButtonVisible: boolean,
onArchiveClicked: () => void,
// these props are used by the `HEADER_ITEMS`
Expand Down Expand Up @@ -86,7 +91,7 @@ export const HEADER_ITEMS = [
},
];

export default function TracePageHeader(props: TracePageHeaderProps) {
export function TracePageHeaderFn(props: TracePageHeaderProps) {
const {
archiveButtonVisible,
onArchiveClicked,
Expand All @@ -101,6 +106,11 @@ export default function TracePageHeader(props: TracePageHeaderProps) {
onSlimViewClicked,
updateTextFilter,
textFilter,
prevResult,
nextResult,
clearSearch,
resultCount,
forwardedRef,
} = props;

if (!traceID) {
Expand Down Expand Up @@ -170,15 +180,15 @@ export default function TracePageHeader(props: TracePageHeaderProps) {
</h1>
</a>
<KeyboardShortcutsHelp className="ub-mr2" />
<div className="ub-mr2">
<Input
name="search"
placeholder="Search..."
onChange={event => updateTextFilter(event.target.value)}
defaultValue={textFilter}
data-test={markers.IN_TRACE_SEARCH}
/>
</div>
<TracePageSearchBar
updateTextFilter={updateTextFilter}
textFilter={textFilter}
prevResult={prevResult}
nextResult={nextResult}
clearSearch={clearSearch}
resultCount={resultCount}
ref={forwardedRef}
/>
<Dropdown overlay={viewMenu}>
<Button className="ub-mr2">
View Options <Icon type="down" />
Expand All @@ -195,3 +205,7 @@ export default function TracePageHeader(props: TracePageHeaderProps) {
</header>
);
}

// 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) => <TracePageHeaderFn {...props} forwardedRef={ref} />);
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@
// limitations under the License.

import React from 'react';
import sinon from 'sinon';
import { shallow, mount } from 'enzyme';

import TracePageHeader, { HEADER_ITEMS } from './TracePageHeader';
import * as markers from './TracePageHeader.markers';
import { TracePageHeaderFn as TracePageHeader, HEADER_ITEMS } from './TracePageHeader';

describe('<TracePageHeader>', () => {
const defaultProps = {
Expand Down Expand Up @@ -53,16 +51,4 @@ describe('<TracePageHeader>', () => {
expect(item.contains(HEADER_ITEMS[i].renderer(defaultProps.trace))).toBeTruthy();
});
});

it('calls updateTextFilter() function for onChange of the input', () => {
const updateTextFilter = sinon.spy();
const props = { ...defaultProps, updateTextFilter };
wrapper = shallow(<TracePageHeader {...props} />);
const event = { target: { value: 'my new value' } };
wrapper
.find(`[data-test="${markers.IN_TRACE_SEARCH}"]`)
.first()
.simulate('change', event);
expect(updateTextFilter.calledWith('my new value')).toBeTruthy();
});
});
36 changes: 36 additions & 0 deletions packages/jaeger-ui/src/components/TracePage/TracePageSearchBar.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
Copyright (c) 2018 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.
*/

.TracePageSearchBar {
max-width: 20rem;
}

.TracePageSearchBar--bar {
display: flex;
max-width: 20rem;
}

.TracePageSearchBar--count {
opacity: 0.6;
}

.TracePageSearchBar--btn {
transition: 0.2s;
}

.TracePageSearchBar--btn.is-disabled {
opacity: 0.5;
}
82 changes: 82 additions & 0 deletions packages/jaeger-ui/src/components/TracePage/TracePageSearchBar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// @flow

// Copyright (c) 2018 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 { Button, Input } from 'antd';

import * as markers from './TracePageSearchBar.markers';

import './TracePageSearchBar.css';

type TracePageSearchBarProps = {
updateTextFilter: string => void,
textFilter: string,
prevResult: () => void,
nextResult: () => void,
clearSearch: () => void,
resultCount: number,
forwardedRef: { current: Input | null },
};

export function TracePageSearchBarFn(props: TracePageSearchBarProps) {
const {
prevResult,
nextResult,
clearSearch,
resultCount,
updateTextFilter,
textFilter,
forwardedRef,
} = props;

const count = textFilter ? <span className="TracePageSearchBar--count">{resultCount}</span> : null;

const updateFilter = event => updateTextFilter(event.target.value);
const onKeyDown = e => {
if (e.keyCode === 27) clearSearch();
};

const btnClass = `TracePageSearchBar--btn${textFilter ? '' : ' is-disabled'}`;

return (
<div className="ub-flex-auto ub-mr2 TracePageSearchBar">
{/* style inline because compact overwrites the display */}
<Input.Group compact style={{ display: 'flex' }}>
<Input
name="search"
className="TracePageSearchBar--bar ub-flex-auto"
placeholder="Search..."
onChange={updateFilter}
value={textFilter}
data-test={markers.IN_TRACE_SEARCH}
suffix={count}
ref={forwardedRef}
onKeyDown={onKeyDown}
onPressEnter={nextResult}
/>
<Button className={btnClass} disabled={!textFilter} icon="up" onClick={prevResult} />
<Button className={btnClass} disabled={!textFilter} icon="down" onClick={nextResult} />
<Button className={btnClass} disabled={!textFilter} icon="close" onClick={clearSearch} />
</Input.Group>
</div>
);
}

// 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) => (
<TracePageSearchBarFn {...props} forwardedRef={ref} />
));
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// 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 React from 'react';
import { shallow } from 'enzyme';

import * as markers from './TracePageSearchBar.markers';
import { TracePageSearchBarFn as TracePageSearchBar } from './TracePageSearchBar';

describe('<TracePageSearchBar>', () => {
const defaultProps = {
updateTextFilter: () => {},
textFilter: 'something',
prevResult: () => {},
nextResult: () => {},
resultCount: 0,
};

let wrapper;

beforeEach(() => {
wrapper = shallow(<TracePageSearchBar {...defaultProps} />);
});

it('calls updateTextFilter() function for onChange of the input', () => {
const updateTextFilter = jest.fn();
const props = { ...defaultProps, updateTextFilter };
wrapper = shallow(<TracePageSearchBar {...props} />);
const event = { target: { value: 'my new value' } };
wrapper
.find(`[data-test="${markers.IN_TRACE_SEARCH}"]`)
.first()
.simulate('change', event);
expect(updateTextFilter.mock.calls.length).toBe(1);
});

it('renders the search bar', () => {
expect(wrapper.find('Input').length).toBe(1);
});

it('renders the buttons', () => {
expect(wrapper.find('Button').length).toBe(3);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,11 @@ type VirtualizedTraceViewProps = {
detailStates: Map<string, ?DetailState>,
detailTagsToggle: string => void,
detailToggle: string => void,
find: (?Trace, ?string) => void,
findMatchesIDs: Set<string>,
registerAccessors: Accessors => void,
setSpanNameColumnWidth: number => void,
setTrace: (?string) => void,
spanNameColumnWidth: number,
textFilter: ?string,
trace: Trace,
};

Expand Down Expand Up @@ -142,40 +140,23 @@ export class VirtualizedTraceViewImpl extends React.PureComponent<VirtualizedTra
this.clippingCssClasses = getCssClasses(currentViewRangeTime);
this.rowStates = generateRowStates(trace.spans, childrenHiddenIDs, detailStates);

const { find, setTrace, textFilter } = props;
const { setTrace } = props;
const traceID = trace ? trace.traceID : null;
setTrace(traceID);
if (textFilter) {
find(trace, textFilter);
}
}

componentWillUpdate(nextProps: VirtualizedTraceViewProps) {
const {
childrenHiddenIDs,
detailStates,
registerAccessors,
textFilter,
trace,
currentViewRangeTime,
} = this.props;
const { childrenHiddenIDs, detailStates, registerAccessors, trace, currentViewRangeTime } = this.props;
const {
currentViewRangeTime: nextViewRangeTime,
childrenHiddenIDs: nextHiddenIDs,
detailStates: nextDetailStates,
find,
registerAccessors: nextRegisterAccessors,
setTrace,
textFilter: nextTextFilter,
trace: nextTrace,
} = nextProps;
if (trace !== nextTrace) {
setTrace(nextTrace ? nextTrace.traceID : null);
if (nextTextFilter) {
find(nextTrace, nextTextFilter);
}
} else if (textFilter !== nextTextFilter) {
find(nextTrace, nextTextFilter);
}
if (trace !== nextTrace || childrenHiddenIDs !== nextHiddenIDs || detailStates !== nextDetailStates) {
this.rowStates = nextTrace ? generateRowStates(nextTrace.spans, nextHiddenIDs, nextDetailStates) : [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,11 @@ describe('<VirtualizedTraceViewImpl>', () => {
detailStates: new Map(),
detailTagsToggle: jest.fn(),
detailToggle: jest.fn(),
find: jest.fn(),
findMatchesIDs: null,
registerAccessors: jest.fn(),
setSpanNameColumnWidth: jest.fn(),
setTrace: jest.fn(),
spanNameColumnWidth: 0.5,
textFilter: null,
};

function expandRow(rowIndex) {
Expand Down Expand Up @@ -110,30 +108,6 @@ describe('<VirtualizedTraceViewImpl>', () => {
expect(props.setTrace.mock.calls).toEqual([[traceID]]);
});

describe('applies searchText to gloabl state.traceTimeline', () => {
it('ctor invokes find() if there is a textFilter', () => {
const textFilter = 'some-text';
wrapper = shallow(<VirtualizedTraceViewImpl {...props} textFilter={textFilter} />);
expect(props.find.mock.calls).toEqual([[trace, textFilter]]);
});

it('handles textFiter updates', () => {
const textFilter = 'different-text';
wrapper.setProps({ textFilter });
expect(props.find.mock.calls).toEqual([[trace, textFilter]]);
});

it('propagates textFilter if the trace changes', () => {
const textFilter = 'some-text';
wrapper.setProps({ textFilter });
props.find.mockReset();
const traceID = 'some-other-id';
const _trace = { ...trace, traceID };
wrapper.setProps({ trace: _trace });
expect(props.find.mock.calls).toEqual([[_trace, textFilter]]);
});
});

describe('props.registerAccessors', () => {
let lv;
let expectedArg;
Expand Down
Loading

0 comments on commit dc4d9f0

Please sign in to comment.