From 0869d26ec411f7f4b232dbd26b4965546b4b2b6a Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Wed, 14 Jun 2017 11:11:59 +0200 Subject: [PATCH] Moved highlighted nodes/edges info to selectors (#2584) * Use selector for highlighted nodes. * Use selector for highlighted edges. --- .../app/scripts/charts/nodes-chart-edges.js | 3 +- .../app/scripts/charts/nodes-chart-nodes.js | 3 +- .../scripts/reducers/__tests__/root-test.js | 4 +- client/app/scripts/reducers/root.js | 59 +------------- .../selectors/graph-view/decorators.js | 76 +++++++++++++++++++ client/app/scripts/utils/layouter-utils.js | 4 + 6 files changed, 91 insertions(+), 58 deletions(-) create mode 100644 client/app/scripts/selectors/graph-view/decorators.js diff --git a/client/app/scripts/charts/nodes-chart-edges.js b/client/app/scripts/charts/nodes-chart-edges.js index 760f03660..f8b2b7762 100644 --- a/client/app/scripts/charts/nodes-chart-edges.js +++ b/client/app/scripts/charts/nodes-chart-edges.js @@ -4,6 +4,7 @@ import { Map as makeMap } from 'immutable'; import { searchNodeMatchesSelector } from '../selectors/search'; import { selectedNetworkNodesIdsSelector } from '../selectors/node-networks'; +import { highlightedEdgeIdsSelector } from '../selectors/graph-view/decorators'; import { hasSelectedNode as hasSelectedNodeFn } from '../utils/topology-utils'; import EdgeContainer from './edge-container'; @@ -82,8 +83,8 @@ export default connect( hasSelectedNode: hasSelectedNodeFn(state), searchNodeMatches: searchNodeMatchesSelector(state), selectedNetworkNodesIds: selectedNetworkNodesIdsSelector(state), + highlightedEdgeIds: highlightedEdgeIdsSelector(state), searchQuery: state.get('searchQuery'), - highlightedEdgeIds: state.get('highlightedEdgeIds'), selectedNetwork: state.get('selectedNetwork'), selectedNodeId: state.get('selectedNodeId'), }) diff --git a/client/app/scripts/charts/nodes-chart-nodes.js b/client/app/scripts/charts/nodes-chart-nodes.js index 770048f0f..a057c64a1 100644 --- a/client/app/scripts/charts/nodes-chart-nodes.js +++ b/client/app/scripts/charts/nodes-chart-nodes.js @@ -4,6 +4,7 @@ import { Map as makeMap } from 'immutable'; import { nodeMetricSelector } from '../selectors/node-metric'; import { searchNodeMatchesSelector } from '../selectors/search'; +import { highlightedNodeIdsSelector } from '../selectors/graph-view/decorators'; import { nodeNetworksSelector, selectedNetworkNodesIdsSelector } from '../selectors/node-networks'; import { getAdjacentNodes } from '../utils/topology-utils'; import NodeContainer from './node-container'; @@ -126,7 +127,7 @@ function mapStateToProps(state) { searchNodeMatches: searchNodeMatchesSelector(state), selectedNetworkNodesIds: selectedNetworkNodesIdsSelector(state), neighborsOfSelectedNode: getAdjacentNodes(state), - highlightedNodeIds: state.get('highlightedNodeIds'), + highlightedNodeIds: highlightedNodeIdsSelector(state), mouseOverNodeId: state.get('mouseOverNodeId'), selectedNetwork: state.get('selectedNetwork'), selectedNodeId: state.get('selectedNodeId'), diff --git a/client/app/scripts/reducers/__tests__/root-test.js b/client/app/scripts/reducers/__tests__/root-test.js index a70df2df3..aec34cc77 100644 --- a/client/app/scripts/reducers/__tests__/root-test.js +++ b/client/app/scripts/reducers/__tests__/root-test.js @@ -3,6 +3,8 @@ import expect from 'expect'; import { TABLE_VIEW_MODE } from '../../constants/naming'; import { constructEdgeId } from '../../utils/layouter-utils'; +import { highlightedEdgeIdsSelector } from '../../selectors/graph-view/decorators'; + // Root reducer test suite using Jasmine matchers describe('RootReducer', () => { @@ -678,7 +680,7 @@ describe('RootReducer', () => { edgeId: constructEdgeId('abc123', 'def456') }; const nextState = reducer(initialState, action); - expect(nextState.get('highlightedEdgeIds').toJS()).toEqual([ + expect(highlightedEdgeIdsSelector(nextState).toJS()).toEqual([ constructEdgeId('abc123', 'def456'), constructEdgeId('def456', 'abc123') ]); diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index e5920517c..d8f345a3d 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -8,12 +8,10 @@ import { List as makeList, Map as makeMap, OrderedMap as makeOrderedMap, - Set as makeSet, } from 'immutable'; import ActionTypes from '../constants/action-types'; import { - EDGE_ID_SEPARATOR, GRAPH_VIEW_MODE, TABLE_VIEW_MODE, } from '../constants/naming'; @@ -26,7 +24,6 @@ import { consolidateNodesDeltas } from '../utils/nodes-delta-utils'; import { applyPinnedSearches } from '../utils/search-utils'; import { findTopologyById, - getAdjacentNodes, setTopologyUrlsById, updateTopologyIds, filterHiddenTopologies, @@ -54,9 +51,6 @@ export const initialState = makeMap({ forceRelayout: false, gridSortedBy: null, gridSortedDesc: null, - // TODO: Calculate these sets from selectors instead. - highlightedEdgeIds: makeSet(), - highlightedNodeIds: makeSet(), hostname: '...', hoveredMetricType: null, initialNodesLoaded: false, @@ -468,64 +462,19 @@ export function rootReducer(state = initialState, action) { } case ActionTypes.ENTER_EDGE: { - // highlight adjacent nodes - state = state.update('highlightedNodeIds', (highlightedNodeIds) => { - highlightedNodeIds = highlightedNodeIds.clear(); - return highlightedNodeIds.union(action.edgeId.split(EDGE_ID_SEPARATOR)); - }); - - // highlight edge - state = state.update('highlightedEdgeIds', (highlightedEdgeIds) => { - highlightedEdgeIds = highlightedEdgeIds.clear(); - highlightedEdgeIds = highlightedEdgeIds.add(action.edgeId); - const opposite = action.edgeId.split(EDGE_ID_SEPARATOR).reverse().join(EDGE_ID_SEPARATOR); - highlightedEdgeIds = highlightedEdgeIds.add(opposite); - return highlightedEdgeIds; - }); - - return state; + return state.set('mouseOverEdgeId', action.edgeId); } case ActionTypes.ENTER_NODE: { - const nodeId = action.nodeId; - const adjacentNodes = getAdjacentNodes(state, nodeId); - - state = state.set('mouseOverNodeId', nodeId); - - // highlight adjacent nodes - state = state.update('highlightedNodeIds', (highlightedNodeIds) => { - highlightedNodeIds = highlightedNodeIds.clear(); - highlightedNodeIds = highlightedNodeIds.add(nodeId); - return highlightedNodeIds.union(adjacentNodes); - }); - - // highlight edge - state = state.update('highlightedEdgeIds', (highlightedEdgeIds) => { - highlightedEdgeIds = highlightedEdgeIds.clear(); - if (adjacentNodes.size > 0) { - // all neighbour combinations because we dont know which direction exists - highlightedEdgeIds = highlightedEdgeIds.union(adjacentNodes.flatMap(adjacentId => [ - [adjacentId, nodeId].join(EDGE_ID_SEPARATOR), - [nodeId, adjacentId].join(EDGE_ID_SEPARATOR) - ])); - } - return highlightedEdgeIds; - }); - - return state; + return state.set('mouseOverNodeId', action.nodeId); } case ActionTypes.LEAVE_EDGE: { - state = state.update('highlightedEdgeIds', highlightedEdgeIds => highlightedEdgeIds.clear()); - state = state.update('highlightedNodeIds', highlightedNodeIds => highlightedNodeIds.clear()); - return state; + return state.set('mouseOverEdgeId', null); } case ActionTypes.LEAVE_NODE: { - state = state.set('mouseOverNodeId', null); - state = state.update('highlightedEdgeIds', highlightedEdgeIds => highlightedEdgeIds.clear()); - state = state.update('highlightedNodeIds', highlightedNodeIds => highlightedNodeIds.clear()); - return state; + return state.set('mouseOverNodeId', null); } case ActionTypes.DO_CONTROL_ERROR: { diff --git a/client/app/scripts/selectors/graph-view/decorators.js b/client/app/scripts/selectors/graph-view/decorators.js new file mode 100644 index 000000000..a0857e19a --- /dev/null +++ b/client/app/scripts/selectors/graph-view/decorators.js @@ -0,0 +1,76 @@ +import { createSelector } from 'reselect'; +import { Set as makeSet } from 'immutable'; + +import { constructEdgeId, getNodesFromEdgeId } from '../../utils/layouter-utils'; + + +const adjacentToHoveredNodeIdsSelector = createSelector( + [ + state => state.get('mouseOverNodeId'), + state => state.get('nodes'), + ], + (mouseOverNodeId, nodes) => { + let nodeIds = makeSet(); + + if (mouseOverNodeId) { + nodeIds = makeSet(nodes.getIn([mouseOverNodeId, 'adjacency'])); + // fill up set with reverse edges + nodes.forEach((node, id) => { + if (node.get('adjacency') && node.get('adjacency').includes(mouseOverNodeId)) { + nodeIds = nodeIds.add(id); + } + }); + } + + return nodeIds; + } +); + +export const highlightedNodeIdsSelector = createSelector( + [ + adjacentToHoveredNodeIdsSelector, + state => state.get('mouseOverNodeId'), + state => state.get('mouseOverEdgeId'), + ], + (adjacentToHoveredNodeIds, mouseOverNodeId, mouseOverEdgeId) => { + let highlightedNodeIds = makeSet(); + + if (mouseOverEdgeId) { + highlightedNodeIds = highlightedNodeIds.union(getNodesFromEdgeId(mouseOverEdgeId)); + } + + if (mouseOverNodeId) { + highlightedNodeIds = highlightedNodeIds.add(mouseOverNodeId); + highlightedNodeIds = highlightedNodeIds.union(adjacentToHoveredNodeIds); + } + + return highlightedNodeIds; + } +); + +export const highlightedEdgeIdsSelector = createSelector( + [ + adjacentToHoveredNodeIdsSelector, + state => state.get('mouseOverNodeId'), + state => state.get('mouseOverEdgeId'), + ], + (adjacentToHoveredNodeIds, mouseOverNodeId, mouseOverEdgeId) => { + let highlightedEdgeIds = makeSet(); + + if (mouseOverEdgeId) { + const [sourceNode, targetNode] = getNodesFromEdgeId(mouseOverEdgeId); + highlightedEdgeIds = highlightedEdgeIds.add(constructEdgeId(sourceNode, targetNode)); + highlightedEdgeIds = highlightedEdgeIds.add(constructEdgeId(targetNode, sourceNode)); + } + + if (mouseOverNodeId) { + // all neighbour combinations because we dont know which direction exists + highlightedEdgeIds = highlightedEdgeIds.union(adjacentToHoveredNodeIds.flatMap(adjacentId => [ + constructEdgeId(adjacentId, mouseOverNodeId), + constructEdgeId(mouseOverNodeId, adjacentId), + ])); + } + + return highlightedEdgeIds; + } +); diff --git a/client/app/scripts/utils/layouter-utils.js b/client/app/scripts/utils/layouter-utils.js index 9acf23cdb..a3a5ce523 100644 --- a/client/app/scripts/utils/layouter-utils.js +++ b/client/app/scripts/utils/layouter-utils.js @@ -3,6 +3,10 @@ import { Map as makeMap } from 'immutable'; import { EDGE_ID_SEPARATOR } from '../constants/naming'; +export function getNodesFromEdgeId(edgeId) { + return edgeId.split(EDGE_ID_SEPARATOR); +} + export function constructEdgeId(source, target) { return [source, target].join(EDGE_ID_SEPARATOR); }