From 89ad4ff085251b3f168374b87e6baa47a911a110 Mon Sep 17 00:00:00 2001 From: Daniele Date: Mon, 2 Mar 2020 16:47:58 +0100 Subject: [PATCH 1/2] Revert to leaf CallNode selection in ActivityGraph --- src/components/timeline/TrackThread.js | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/components/timeline/TrackThread.js b/src/components/timeline/TrackThread.js index 6a5cbf7d5c..9619a8ffa0 100644 --- a/src/components/timeline/TrackThread.js +++ b/src/components/timeline/TrackThread.js @@ -32,7 +32,6 @@ import { changeSelectedCallNode, focusCallTree, selectLeafCallNode, - selectBestAncestorCallNodeAndExpandCallTree, } from '../../actions/profile-view'; import { reportTrackThreadHeight } from '../../actions/app'; import EmptyThreadIndicator from './EmptyThreadIndicator'; @@ -81,7 +80,6 @@ type DispatchProps = {| +changeSelectedCallNode: typeof changeSelectedCallNode, +focusCallTree: typeof focusCallTree, +selectLeafCallNode: typeof selectLeafCallNode, - +selectBestAncestorCallNodeAndExpandCallTree: typeof selectBestAncestorCallNodeAndExpandCallTree, +reportTrackThreadHeight: typeof reportTrackThreadHeight, |}; @@ -92,8 +90,8 @@ type Props = {| class TimelineTrackThread extends PureComponent { /** - * Handle when a sample is clicked in the ThreadStackGraph. This will select - * the leaf-most stack frame or call node. + * Handle when a sample is clicked in the ThreadStackGraph and in the ThreadActivityGraph. + * This will select the leaf-most stack frame or call node. */ _onSampleClick = (sampleIndex: IndexIntoSamplesTable) => { const { threadIndex, selectLeafCallNode, focusCallTree } = this.props; @@ -101,22 +99,6 @@ class TimelineTrackThread extends PureComponent { focusCallTree(); }; - /** - * Handle when the ThreadActivityGraph is clicked. It uses a slightly different - * strategy of selecting the "best" ancestor call node for a given sample. - * This strategy should make for more interesting selections when clicking around - * the graph. - */ - _onActivitySampleClick = (sampleIndex: IndexIntoSamplesTable) => { - const { - threadIndex, - selectBestAncestorCallNodeAndExpandCallTree, - focusCallTree, - } = this.props; - selectBestAncestorCallNodeAndExpandCallTree(threadIndex, sampleIndex); - focusCallTree(); - }; - _onMarkerSelect = ( threadIndex: ThreadIndex, start: Milliseconds, @@ -206,7 +188,7 @@ class TimelineTrackThread extends PureComponent { fullThread={fullThread} rangeStart={rangeStart} rangeEnd={rangeEnd} - onSampleClick={this._onActivitySampleClick} + onSampleClick={this._onSampleClick} categories={categories} samplesSelectedStates={samplesSelectedStates} /> @@ -270,7 +252,6 @@ export default explicitConnect({ changeSelectedCallNode, focusCallTree, selectLeafCallNode, - selectBestAncestorCallNodeAndExpandCallTree, reportTrackThreadHeight, }, component: withSize(TimelineTrackThread), From 85f9010a10a161e11ca2b025bed263e1631fc4db Mon Sep 17 00:00:00 2001 From: Daniele Date: Tue, 3 Mar 2020 17:46:51 +0100 Subject: [PATCH 2/2] Adapt ThreadActivityGraph test to changed behavior --- src/test/components/ThreadActivityGraph.test.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/test/components/ThreadActivityGraph.test.js b/src/test/components/ThreadActivityGraph.test.js index e9d0d4084f..44b5f5805a 100644 --- a/src/test/components/ThreadActivityGraph.test.js +++ b/src/test/components/ThreadActivityGraph.test.js @@ -130,24 +130,18 @@ describe('ThreadActivityGraph', function() { * as once it's connected to the Redux store in the SelectedActivityGraph. */ describe('ThreadActivityGraph', function() { - it('can click a best ancestor call node', function() { + it('selects the full call node path when clicked', function() { const { clickActivityGraph, getCallNodePath } = setup(); // The full call node at this sample is: // A -> B -> C -> F -> G - // However, the best ancestor call node is: - // A -> B -> C -> F - // As this is the most common ancestor with the same category. clickActivityGraph(1, 0.2); - expect(getCallNodePath()).toEqual(['A', 'B', 'C', 'F']); + expect(getCallNodePath()).toEqual(['A', 'B', 'C', 'F', 'G']); // The full call node at this sample is: // A -> B -> H -> I - // However, the best ancestor call node is: - // A -> B -> H - // As this is the most common ancestor with the same category. clickActivityGraph(1, 0.8); - expect(getCallNodePath()).toEqual(['A', 'B', 'H']); + expect(getCallNodePath()).toEqual(['A', 'B', 'H', 'I']); }); it('will redraw even when there are no samples in range', function() {