Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add uiFind icons, scroll to first match #367

Merged
merged 14 commits into from
May 3, 2019
Merged

add uiFind icons, scroll to first match #367

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a0e296a
WIP - add uiFind icons, scroll to first match
everett980 Apr 2, 2019
ebf9473
Use componentDidUpdate instead of setTimeout
everett980 Apr 11, 2019
37f9a20
Clarify focusUiFind function name, fix existing
everett980 Apr 11, 2019
bb66f09
Add scrollToFirstVisibleSpan tests
everett980 Apr 11, 2019
79f04af
Add scrollToFirstVisibleSpan tests
everett980 Apr 11, 2019
79eb6df
Bolster test coverage for files in PR
everett980 Apr 12, 2019
8a268c2
Improve TracePageSearchBar styling, choose icons
everett980 Apr 15, 2019
5a146ef
Move scroll indicator from set/map to standalone
everett980 Apr 23, 2019
43a8ca5
Update CopyIcon to support spanID deep link
everett980 Apr 24, 2019
5801d3e
Fix CopyIcon css, Swap in copy-to-clipboard
everett980 Apr 25, 2019
73e8691
Improve VirtualizedTraceView shouldComponentUpdate
everett980 Apr 25, 2019
7651dbc
Merge branch 'master' into issue-361-and-360-improve-uiFind-interactions
tiffon Apr 26, 2019
9b9f5c0
Remove unnecessary function calls, improve names
everett980 May 2, 2019
cdbdd9d
Remove nonfunctional focusUiFindMatches call
everett980 May 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ limitations under the License.
width: 40%;
}

.TracePageSearchBar--InputGroup {
justify-content: flex-end;
}

.TracePageSearchBar--bar {
max-width: 20rem;
transition: max-width 0.5s;
Expand All @@ -44,6 +40,6 @@ limitations under the License.
opacity: 0.5;
}

.TracePageSearchBar--btn.locate-btn {
.TracePageSearchBar--locateBtn {
padding: 1px 8px 4px;
}
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageSearchBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
return (
<div className="TracePageSearchBar">
{/* style inline because compact overwrites the display */}
<Input.Group className="TracePageSearchBar--InputGroup" compact style={{ display: 'flex' }}>
<Input.Group className="ub-justify-end" compact style={{ display: 'flex' }}>
<UiFindInput
inputProps={uiFindInputInputProps}
forwardedRef={forwardedRef}
Expand All @@ -68,7 +68,7 @@ export function TracePageSearchBarFn(props: TracePageSearchBarProps & { forwarde
{navigable && (
<>
<Button
className={cx(btnClass, 'locate-btn')}
className={cx(btnClass, 'TracePageSearchBar--locateBtn')}
disabled={!textFilter}
htmlType="button"
onClick={focusUiFindMatches}
Expand Down
2 changes: 2 additions & 0 deletions packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,11 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
}

shouldComponentUpdate(nextProps: VirtualizedTraceViewProps) {
// If any prop updates, VirtualizedTraceViewImpl should update.
const nextPropKeys = Object.keys(nextProps) as (keyof VirtualizedTraceViewProps)[];
for (let i = 0; i < nextPropKeys.length; i += 1) {
if (nextProps[nextPropKeys[i]] !== this.props[nextPropKeys[i]]) {
// Unless the only change was props.shouldScrollToFirstUiFindMatch changing to false.
if (nextPropKeys[i] === 'shouldScrollToFirstUiFindMatch') {
if (nextProps[nextPropKeys[i]]) return true;
tiffon marked this conversation as resolved.
Show resolved Hide resolved
} else {
Expand Down
6 changes: 3 additions & 3 deletions packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const fullActions = createActions<TActionTypes>({

export const actions = (fullActions as any).jaegerUi.traceTimelineViewer as TTimelineViewerActions;

function calculateHiddenIdsAndDetailStates(uiFind: string, spans: Span[]) {
function calculateFocusedFindRowStates(uiFind: string, spans: Span[]) {
const spansMap = new Map();
const childrenHiddenIDs: Set<string> = new Set();
const detailStates: Map<string, DetailState> = new Map();
Expand Down Expand Up @@ -127,7 +127,7 @@ function focusUiFindMatches(state: TTraceTimeline, { uiFind, trace }: TTraceUiFi
if (!uiFind) return state;
return {
...state,
...calculateHiddenIdsAndDetailStates(uiFind, trace.spans),
...calculateFocusedFindRowStates(uiFind, trace.spans),
};
}

Expand All @@ -147,7 +147,7 @@ function setTrace(state: TTraceTimeline, { uiFind, trace }: TTraceUiFindValue) {

return Object.assign(
{ ...newInitialState(), spanNameColumnWidth, traceID },
uiFind ? calculateHiddenIdsAndDetailStates(uiFind, spans) : null
uiFind ? calculateFocusedFindRowStates(uiFind, spans) : null
);
}

Expand Down
8 changes: 1 addition & 7 deletions packages/jaeger-ui/src/components/TracePage/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,28 +136,22 @@ describe('<TracePage>', () => {
});

describe('focusUiFindMatches', () => {
let scrollToFirstVisibleSpanSpy;

beforeEach(() => {
scrollToFirstVisibleSpanSpy = jest.spyOn(wrapper.instance()._scrollManager, 'scrollToFirstVisibleSpan');
scrollToFirstVisibleSpanSpy.mockReset();
defaultProps.focusUiFindMatches.mockReset();
});

it('calls _scrollManager.scrollToFirstVisibleSpan and props.focusUiFindMatches with props.trace.data and uiFind when props.trace.data is present', () => {
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);
expect(scrollToFirstVisibleSpanSpy).toHaveBeenCalled();
});

it('handles when props.trace.data is absent', () => {
const propFn = wrapper.find(TracePageHeader).prop('focusUiFindMatches');
wrapper.setProps({ trace: {} });
propFn();
expect(defaultProps.focusUiFindMatches).not.toHaveBeenCalled();
expect(scrollToFirstVisibleSpanSpy).not.toHaveBeenCalled();
});
});

Expand Down
4 changes: 3 additions & 1 deletion packages/jaeger-ui/src/components/TracePage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
if (this.props.trace && this.props.trace.data) {
this.traceDagEV = calculateTraceDagEV(this.props.trace.data);
}
if (traceGraphView) {
this.focusUiFindMatches();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems kind of strange. So, it will focus the UI find matches when you go back to the timeline view?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I had removed this... It doesn't work without more changes and as you said it may not be the desired functionality.
Will really remove and push.

this.setState({ traceGraphView: !traceGraphView });
};

Expand Down Expand Up @@ -308,7 +311,6 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
focusUiFindMatches = () => {
const { trace, focusUiFindMatches, uiFind } = this.props;
if (trace && trace.data) {
this._scrollManager.scrollToFirstVisibleSpan();
focusUiFindMatches(trace.data, uiFind);
}
};
Expand Down