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.

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

Timeline Expand and Collapse Features #221

Merged
merged 17 commits into from
Jul 6, 2018
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
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.
*/

.TimelineCollapser {
float: right;
Copy link
Member

Choose a reason for hiding this comment

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

This is float right?

margin: 0 0.8rem 0 0;
display: inline-block;
}

.TimelineCollapser > * {
Copy link
Member

Choose a reason for hiding this comment

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

Selectors like this are not really preferred; I suggest using something less general, if possible.

margin-right: 0.3rem;
transform: rotate(90deg);
Copy link
Member

Choose a reason for hiding this comment

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

Why would you want all of the icons rotated 90 degrees?

}
Copy link
Member

Choose a reason for hiding this comment

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

The ExpandBtn class should be scoped to TimelineCollapser to prevent clashes with other styles.

Also, if written in the following way both classes don't need to be added to the component:

.TimelineCollapser--btn,
.TimelineCollapser--btn-expand {
  margin-right: 0.3rem;
}

.TimelineCollapser--btn-expand {
  transform: rotate(90deg);
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// @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 React from 'react';

import { Tooltip } from 'antd';

import './TimelineCollapser.css';
import type { Span } from '../../../../types';

type CollapserProps = {
onCollapseAll: (Span[]) => void,
onCollapseOne: (Span[]) => void,
onExpandOne: (Span[]) => void,
onExpandAll: () => void,
spans: Span[],
Copy link
Member

Choose a reason for hiding this comment

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

I suggest adding the spans in the TraceTimelineViewer component, so they don't need to be passed down.

};

export default function TimelineCollapser(props: CollapserProps) {
const { onExpandAll, onExpandOne, onCollapseAll, onCollapseOne, spans } = props;
const _onCollapseAll = () => onCollapseAll(spans);
const _onExpandOne = () => onExpandOne(spans);
const _onCollapseOne = () => onCollapseOne(spans);
return (
<span className="TimelineCollapser">
<Tooltip title="Expand +1">
<span className="anticon anticon-right" onClick={_onExpandOne} role={'button'} />
Copy link
Member

Choose a reason for hiding this comment

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

I recommend using the ant Icon component.

</Tooltip>
<Tooltip title="Collapse +1">
<span className="anticon anticon-left" onClick={_onCollapseOne} role={'button'} />
</Tooltip>
<Tooltip title="Expand All">
<span className="anticon anticon-double-right" onClick={onExpandAll} role={'button'} />
</Tooltip>
<Tooltip title="Collapse All">
<span className="anticon anticon-double-left" onClick={_onCollapseAll} role={'button'} />
</Tooltip>
</span>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ limitations under the License.
}

.TimelineHeaderRow--title {
display: inline-block;
overflow: hidden;
margin: 0 0.75rem 0 0.5rem;
margin: 0 0 0 0.5rem;
text-overflow: ellipsis;
white-space: nowrap;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,26 @@

import * as React from 'react';

import TimelineCollapser from './TimelineCollapser';
import TimelineColumnResizer from './TimelineColumnResizer';
import TimelineViewingLayer from './TimelineViewingLayer';
import Ticks from '../Ticks';
import TimelineRow from '../TimelineRow';
import type { ViewRangeTime, ViewRangeTimeUpdate } from '../../types';
import type { Span } from '../../../../types';

import './TimelineHeaderRow.css';

type TimelineHeaderRowProps = {
duration: number,
nameColumnWidth: number,
numTicks: number,
onCollapseAll: (Span[]) => void,
onCollapseOne: (Span[]) => void,
onColummWidthChange: number => void,
onExpandAll: () => void,
onExpandOne: (Span[]) => void,
spans: Span[],
updateNextViewRangeTime: ViewRangeTimeUpdate => void,
updateViewRangeTime: (number, number, ?string) => void,
viewRangeTime: ViewRangeTime,
Expand All @@ -39,7 +46,12 @@ export default function TimelineHeaderRow(props: TimelineHeaderRowProps) {
duration,
nameColumnWidth,
numTicks,
onCollapseAll,
onCollapseOne,
onColummWidthChange,
onExpandAll,
onExpandOne,
spans,
updateViewRangeTime,
updateNextViewRangeTime,
viewRangeTime,
Expand All @@ -49,6 +61,13 @@ export default function TimelineHeaderRow(props: TimelineHeaderRowProps) {
<TimelineRow className="TimelineHeaderRow">
<TimelineRow.Cell width={nameColumnWidth}>
<h3 className="TimelineHeaderRow--title">Service &amp; Operation</h3>
<TimelineCollapser
onCollapseAll={onCollapseAll}
onExpandAll={onExpandAll}
onCollapseOne={onCollapseOne}
onExpandOne={onExpandOne}
spans={spans}
/>
</TimelineRow.Cell>
<TimelineRow.Cell width={1 - nameColumnWidth}>
<TimelineViewingLayer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ export const actionTypes = generateActionTypes('@jaeger-ui/trace-timeline-viewer
'SET_TRACE',
'SET_SPAN_NAME_COLUMN_WIDTH',
'CHILDREN_TOGGLE',
'EXPAND_ALL',
'COLLAPSE_ALL',
'EXPAND_ONE',
'COLLAPSE_ONE',
'DETAIL_TOGGLE',
'DETAIL_TAGS_TOGGLE',
'DETAIL_PROCESS_TOGGLE',
Expand All @@ -61,6 +65,10 @@ const fullActions = createActions({
[actionTypes.SET_TRACE]: traceID => ({ traceID }),
[actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: width => ({ width }),
[actionTypes.CHILDREN_TOGGLE]: spanID => ({ spanID }),
[actionTypes.EXPAND_ALL]: () => ({}),
[actionTypes.EXPAND_ONE]: spans => ({ spans }),
[actionTypes.COLLAPSE_ALL]: spans => ({ spans }),
[actionTypes.COLLAPSE_ONE]: spans => ({ spans }),
[actionTypes.DETAIL_TOGGLE]: spanID => ({ spanID }),
[actionTypes.DETAIL_TAGS_TOGGLE]: spanID => ({ spanID }),
[actionTypes.DETAIL_PROCESS_TOGGLE]: spanID => ({ spanID }),
Expand Down Expand Up @@ -97,6 +105,70 @@ function childrenToggle(state, { payload }) {
return { ...state, childrenHiddenIDs };
}

function shouldDisableCollapse(allSpans, hiddenSpansIds) {
const allParentSpans = allSpans.filter(s => s.hasChildren);
return allParentSpans.length === hiddenSpansIds.size;
}

function expandAll(state) {
const childrenHiddenIDs = new Set();
return { ...state, childrenHiddenIDs };
}

function collapseAll(state, { payload }) {
Copy link
Member

Choose a reason for hiding this comment

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

Great work on these reducers, very simply implemented 👍

const { spans } = payload;
if (shouldDisableCollapse(spans, state.childrenHiddenIDs)) {
return state;
}
const childrenHiddenIDs = spans.reduce((res, s) => {
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, why prefer reduce over filter?

const childrenHiddenIDs = new Set(spans.filter(sp => sp.hasChildren));

Applies generally to these reducers.

if (s.hasChildren) {
res.add(s.spanID);
}
return res;
}, new Set());
return { ...state, childrenHiddenIDs };
}

function collapseOne(state, { payload }) {
const { spans } = payload;
if (shouldDisableCollapse(spans, state.childrenHiddenIDs)) {
return state;
}
let nearestCollapsedAncestor;
const childrenHiddenIDs = spans.reduce((res, curSpan) => {
if (nearestCollapsedAncestor && curSpan.depth <= nearestCollapsedAncestor.depth) {
res.add(nearestCollapsedAncestor.spanID);
nearestCollapsedAncestor = curSpan;
} else if (curSpan.hasChildren && !res.has(curSpan.spanID)) {
nearestCollapsedAncestor = curSpan;
}
return res;
}, new Set(state.childrenHiddenIDs));
childrenHiddenIDs.add(nearestCollapsedAncestor.spanID);
return { ...state, childrenHiddenIDs };
}

function expandOne(state, { payload }) {
const { spans } = payload;
if (state.childrenHiddenIDs.size === 0) {
return state;
}
let prevExpandedDepth = -1;
let expandNextHiddenSpan = true;
const childrenHiddenIDs = spans.reduce((res, s) => {
if (s.depth <= prevExpandedDepth) {
expandNextHiddenSpan = true;
}
if (expandNextHiddenSpan && res.has(s.spanID)) {
res.delete(s.spanID);
expandNextHiddenSpan = false;
prevExpandedDepth = s.depth;
}
return res;
}, new Set(state.childrenHiddenIDs));
return { ...state, childrenHiddenIDs };
}

function detailToggle(state, { payload }) {
const { spanID } = payload;
const detailStates = new Map(state.detailStates);
Expand Down Expand Up @@ -149,6 +221,10 @@ export default handleActions(
[actionTypes.SET_TRACE]: setTrace,
[actionTypes.SET_SPAN_NAME_COLUMN_WIDTH]: setColumnWidth,
[actionTypes.CHILDREN_TOGGLE]: childrenToggle,
[actionTypes.EXPAND_ALL]: expandAll,
[actionTypes.EXPAND_ONE]: expandOne,
[actionTypes.COLLAPSE_ALL]: collapseAll,
[actionTypes.COLLAPSE_ONE]: collapseOne,
[actionTypes.DETAIL_TOGGLE]: detailToggle,
[actionTypes.DETAIL_TAGS_TOGGLE]: detailTagsToggle,
[actionTypes.DETAIL_PROCESS_TOGGLE]: detailProcessToggle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@ import TimelineHeaderRow from './TimelineHeaderRow';
import VirtualizedTraceView from './VirtualizedTraceView';
import type { Accessors } from '../ScrollManager';
import type { ViewRange, ViewRangeTimeUpdate } from '../types';
import type { Trace } from '../../../types';
import type { Span, Trace } from '../../../types';

import './index.css';

type TraceTimelineViewerProps = {
registerAccessors: Accessors => void,
setSpanNameColumnWidth: number => void,
collapseAll: (Span[]) => void,
collapseOne: (Span[]) => void,
expandAll: () => void,
expandOne: (Span[]) => void,
spanNameColumnWidth: number,
textFilter: ?string,
trace: Trace,
Expand All @@ -47,14 +51,19 @@ const NUM_TICKS = 5;
*/
function TraceTimelineViewer(props: TraceTimelineViewerProps) {
const { setSpanNameColumnWidth, updateNextViewRangeTime, updateViewRangeTime, viewRange, ...rest } = props;
const { spanNameColumnWidth, trace } = rest;
const { spanNameColumnWidth, trace, expandAll, collapseAll, expandOne, collapseOne } = rest;
return (
<div className="TraceTimelineViewer">
<TimelineHeaderRow
duration={trace.duration}
nameColumnWidth={spanNameColumnWidth}
numTicks={NUM_TICKS}
onCollapseAll={collapseAll}
onCollapseOne={collapseOne}
onColummWidthChange={setSpanNameColumnWidth}
onExpandAll={expandAll}
onExpandOne={expandOne}
spans={trace.spans}
viewRangeTime={viewRange.time}
updateNextViewRangeTime={updateNextViewRangeTime}
updateViewRangeTime={updateViewRangeTime}
Expand All @@ -74,7 +83,23 @@ function mapDispatchToProps(dispatch) {
const action = actions.setSpanNameColumnWidth(...args);
return dispatch(action);
};
return { setSpanNameColumnWidth };
const expandAll = () => {
const action = actions.expandAll();
return dispatch(action);
};
const expandOne = spans => {
const action = actions.expandOne(spans);
return dispatch(action);
};
const collapseAll = spans => {
const action = actions.collapseAll(spans);
return dispatch(action);
};
const collapseOne = spans => {
const action = actions.collapseOne(spans);
Copy link
Member

Choose a reason for hiding this comment

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

Typically, bindActionCreators from redux would be used to do this. Not sure why it's not used on line 83. Please change to use bindActionCreators.

return dispatch(action);
};
return { setSpanNameColumnWidth, expandAll, expandOne, collapseAll, collapseOne };
}

export default connect(mapStateToProps, mapDispatchToProps)(TraceTimelineViewer);