From 23fb14d0be590a51c602cdcb60c067dfb804e63e Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Wed, 26 Apr 2017 16:18:42 +0200 Subject: [PATCH] Polished the metric selection logic (#2468) * Polished the metric selection code * Fixed hovering advantage over pinning. --- client/app/scripts/actions/app-actions.js | 50 ++++++++++++------- client/app/scripts/components/app.js | 2 - .../components/metric-selector-item.js | 40 +++++++-------- .../app/scripts/components/metric-selector.js | 12 ++--- .../scripts/components/view-mode-selector.js | 2 +- client/app/scripts/constants/action-types.js | 3 +- client/app/scripts/reducers/root.js | 28 +++-------- client/app/scripts/selectors/node-metric.js | 37 +++++++++++--- client/app/scripts/utils/metric-utils.js | 10 ++-- 9 files changed, 102 insertions(+), 82 deletions(-) diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index 86d75f0db..4085b8e7f 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -24,7 +24,11 @@ import { import { getCurrentTopologyUrl } from '../utils/topology-utils'; import { storageSet } from '../utils/storage-utils'; import { loadTheme } from '../utils/contrast-utils'; -import { availableMetricsSelector, pinnedMetricSelector } from '../selectors/node-metric'; +import { + availableMetricTypesSelector, + selectedMetricTypeSelector, + pinnedMetricSelector, +} from '../selectors/node-metric'; import { activeTopologyOptionsSelector, isResourceViewModeSelector, @@ -122,19 +126,24 @@ export function unpinNetwork(networkId) { // Metrics // - -export function selectMetric(metricId) { +export function hoverMetric(metricType) { return { - type: ActionTypes.SELECT_METRIC, - metricId + type: ActionTypes.HOVER_METRIC, + metricType, }; } -export function pinMetric(metricId) { +export function unhoverMetric() { + return { + type: ActionTypes.UNHOVER_METRIC, + }; +} + +export function pinMetric(metricType) { return (dispatch, getState) => { dispatch({ type: ActionTypes.PIN_METRIC, - metricId, + metricType, }); updateRoute(getState); }; @@ -142,22 +151,25 @@ export function pinMetric(metricId) { export function unpinMetric() { return (dispatch, getState) => { - dispatch({ - type: ActionTypes.UNPIN_METRIC, - }); - updateRoute(getState); + // We always have to keep metrics pinned in the resource view. + if (!isResourceViewModeSelector(getState())) { + dispatch({ + type: ActionTypes.UNPIN_METRIC, + }); + updateRoute(getState); + } }; } export function pinNextMetric(delta) { return (dispatch, getState) => { const state = getState(); - const metrics = availableMetricsSelector(state).map(m => m.get('id')); - const currentIndex = metrics.indexOf(state.get('selectedMetric')); - const nextIndex = modulo(currentIndex + delta, metrics.count()); - const nextMetric = metrics.get(nextIndex); + const metricTypes = availableMetricTypesSelector(state); + const currentIndex = metricTypes.indexOf(selectedMetricTypeSelector(state)); + const nextIndex = modulo(currentIndex + delta, metricTypes.count()); + const nextMetricType = metricTypes.get(nextIndex); - dispatch(pinMetric(nextMetric)); + dispatch(pinMetric(nextMetricType)); }; } @@ -303,8 +315,10 @@ export function setResourceView() { viewMode: RESOURCE_VIEW_MODE, }); // Pin the first metric if none of the visible ones is pinned. - if (!pinnedMetricSelector(getState())) { - dispatch({ type: ActionTypes.PIN_METRIC }); + const state = getState(); + if (!pinnedMetricSelector(state)) { + const firstAvailableMetricType = availableMetricTypesSelector(state).first(); + dispatch(pinMetric(firstAvailableMetricType)); } getResourceViewNodesSnapshot(getState, dispatch); updateRoute(getState); diff --git a/client/app/scripts/components/app.js b/client/app/scripts/components/app.js index 36d7e3614..a98bbbdcb 100644 --- a/client/app/scripts/components/app.js +++ b/client/app/scripts/components/app.js @@ -19,7 +19,6 @@ import { hitEnter, hitEsc, unpinMetric, - selectMetric, toggleHelp, setGraphView, setTableView, @@ -112,7 +111,6 @@ class App extends React.Component { dispatch(setResourceView()); } else if (char === 'q') { dispatch(unpinMetric()); - dispatch(selectMetric(null)); } else if (char === '/') { ev.preventDefault(); dispatch(focusSearch()); diff --git a/client/app/scripts/components/metric-selector-item.js b/client/app/scripts/components/metric-selector-item.js index 0abf0d065..11f2cb227 100644 --- a/client/app/scripts/components/metric-selector-item.js +++ b/client/app/scripts/components/metric-selector-item.js @@ -2,11 +2,11 @@ import React from 'react'; import classNames from 'classnames'; import { connect } from 'react-redux'; -import { selectMetric, pinMetric, unpinMetric } from '../actions/app-actions'; -import { pinnedMetricSelector } from '../selectors/node-metric'; +import { hoverMetric, pinMetric, unpinMetric } from '../actions/app-actions'; +import { selectedMetricTypeSelector } from '../selectors/node-metric'; + class MetricSelectorItem extends React.Component { - constructor(props, context) { super(props, context); @@ -15,37 +15,37 @@ class MetricSelectorItem extends React.Component { } onMouseOver() { - const k = this.props.metric.get('id'); - this.props.selectMetric(k); + const metricType = this.props.metric.get('label'); + this.props.hoverMetric(metricType); } onMouseClick() { - const k = this.props.metric.get('id'); - const pinnedMetric = this.props.pinnedMetric; + const metricType = this.props.metric.get('label'); + const pinnedMetricType = this.props.pinnedMetricType; - if (k !== pinnedMetric) { - this.props.pinMetric(k); - } else if (!this.props.alwaysPinned) { - this.props.unpinMetric(k); + if (metricType !== pinnedMetricType) { + this.props.pinMetric(metricType); + } else { + this.props.unpinMetric(); } } render() { - const { metric, selectedMetric, pinnedMetric } = this.props; - const id = metric.get('id'); - const isPinned = (id === pinnedMetric); - const isSelected = (id === selectedMetric); + const { metric, selectedMetricType, pinnedMetricType } = this.props; + const type = metric.get('label'); + const isPinned = (type === pinnedMetricType); + const isSelected = (type === selectedMetricType); const className = classNames('metric-selector-action', { 'metric-selector-action-selected': isSelected }); return (
- {metric.get('label')} + {type} {isPinned && }
); @@ -54,12 +54,12 @@ class MetricSelectorItem extends React.Component { function mapStateToProps(state) { return { - selectedMetric: state.get('selectedMetric'), - pinnedMetric: pinnedMetricSelector(state), + selectedMetricType: selectedMetricTypeSelector(state), + pinnedMetricType: state.get('pinnedMetricType'), }; } export default connect( mapStateToProps, - { selectMetric, pinMetric, unpinMetric } + { hoverMetric, pinMetric, unpinMetric } )(MetricSelectorItem); diff --git a/client/app/scripts/components/metric-selector.js b/client/app/scripts/components/metric-selector.js index cd04c94a3..d16ad22d8 100644 --- a/client/app/scripts/components/metric-selector.js +++ b/client/app/scripts/components/metric-selector.js @@ -1,8 +1,8 @@ import React from 'react'; import { connect } from 'react-redux'; -import { selectMetric } from '../actions/app-actions'; -import { availableMetricsSelector, pinnedMetricSelector } from '../selectors/node-metric'; +import { unhoverMetric } from '../actions/app-actions'; +import { availableMetricsSelector } from '../selectors/node-metric'; import MetricSelectorItem from './metric-selector-item'; class MetricSelector extends React.Component { @@ -13,11 +13,11 @@ class MetricSelector extends React.Component { } onMouseOut() { - this.props.selectMetric(this.props.pinnedMetric); + this.props.unhoverMetric(); } render() { - const { alwaysPinned, availableMetrics } = this.props; + const { availableMetrics } = this.props; const hasMetrics = !availableMetrics.isEmpty(); return ( @@ -26,7 +26,6 @@ class MetricSelector extends React.Component { {availableMetrics.map(metric => ( ))} @@ -39,11 +38,10 @@ class MetricSelector extends React.Component { function mapStateToProps(state) { return { availableMetrics: availableMetricsSelector(state), - pinnedMetric: pinnedMetricSelector(state), }; } export default connect( mapStateToProps, - { selectMetric } + { unhoverMetric } )(MetricSelector); diff --git a/client/app/scripts/components/view-mode-selector.js b/client/app/scripts/components/view-mode-selector.js index d4c6403bf..4e8cfdaa9 100644 --- a/client/app/scripts/components/view-mode-selector.js +++ b/client/app/scripts/components/view-mode-selector.js @@ -47,7 +47,7 @@ class ViewModeSelector extends React.Component { {Item('fa fa-bar-chart', 'Resources', isResourceViewMode, this.props.setResourceView, hasResourceView)} - + ); } diff --git a/client/app/scripts/constants/action-types.js b/client/app/scripts/constants/action-types.js index a793e0fa8..ad3c8eaa2 100644 --- a/client/app/scripts/constants/action-types.js +++ b/client/app/scripts/constants/action-types.js @@ -28,6 +28,8 @@ const ACTION_TYPES = [ 'ENTER_NODE', 'FOCUS_SEARCH', 'HIDE_HELP', + 'HOVER_METRIC', + 'UNHOVER_METRIC', 'LEAVE_EDGE', 'LEAVE_NODE', 'PIN_METRIC', @@ -48,7 +50,6 @@ const ACTION_TYPES = [ 'RECEIVE_ERROR', 'RESET_LOCAL_VIEW_STATE', 'ROUTE_TOPOLOGY', - 'SELECT_METRIC', 'SHOW_HELP', 'SET_VIEWPORT_DIMENSIONS', 'SET_EXPORTING_GRAPH', diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index 2c6b79462..b3c2d5f12 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -15,7 +15,6 @@ import { isResourceViewModeSelector, } from '../selectors/topology'; import { activeTopologyZoomCacheKeyPathSelector } from '../selectors/zooming'; -import { availableMetricsSelector, pinnedMetricSelector } from '../selectors/node-metric'; import { applyPinnedSearches } from '../utils/search-utils'; import { findTopologyById, @@ -51,6 +50,7 @@ export const initialState = makeMap({ highlightedEdgeIds: makeSet(), highlightedNodeIds: makeSet(), hostname: '...', + hoveredMetricType: null, initialNodesLoaded: false, mouseOverEdgeId: null, mouseOverNodeId: null, @@ -68,7 +68,6 @@ export const initialState = makeMap({ routeSet: false, searchFocused: false, searchQuery: null, - selectedMetric: null, selectedNetwork: null, selectedNodeId: null, showingHelp: false, @@ -379,19 +378,16 @@ export function rootReducer(state = initialState, action) { // metrics // - case ActionTypes.SELECT_METRIC: { - return state.set('selectedMetric', action.metricId); + case ActionTypes.HOVER_METRIC: { + return state.set('hoveredMetricType', action.metricType); + } + + case ActionTypes.UNHOVER_METRIC: { + return state.set('hoveredMetricType', null); } case ActionTypes.PIN_METRIC: { - const canvasMetrics = availableMetricsSelector(state); - const metricTypes = makeMap(canvasMetrics.map(m => [m.get('id'), m.get('label')])); - // Pin the first metric if no metric ID was explicitly given. - const metricId = action.metricId || (canvasMetrics.first() || makeMap()).get('id'); - return state.merge({ - pinnedMetricType: metricTypes.get(metricId), - selectedMetric: metricId, - }); + return state.set('pinnedMetricType', action.metricType); } case ActionTypes.UNPIN_METRIC: { @@ -612,14 +608,6 @@ export function rootReducer(state = initialState, action) { // apply pinned searches, filters nodes that dont match state = applyPinnedSearches(state); - // if something in the current topo is not already selected, select it. - if (!availableMetricsSelector(state) - .map(m => m.get('id')) - .toSet() - .has(state.get('selectedMetric'))) { - state = state.set('selectedMetric', pinnedMetricSelector(state)); - } - // Update the nodes cache only if we're not in the resource view mode, as we // intentionally want to keep it static before we figure how to keep it up-to-date. if (!isResourceViewModeSelector(state)) { diff --git a/client/app/scripts/selectors/node-metric.js b/client/app/scripts/selectors/node-metric.js index 8c17c9ced..2a81ecd05 100644 --- a/client/app/scripts/selectors/node-metric.js +++ b/client/app/scripts/selectors/node-metric.js @@ -1,5 +1,5 @@ import { createSelector } from 'reselect'; -import { createMapSelector } from 'reselect-map'; +import { createMapSelector, createListSelector } from 'reselect-map'; import { fromJS, Map as makeMap, List as makeList } from 'immutable'; import { isGraphViewModeSelector, isResourceViewModeSelector } from '../selectors/topology'; @@ -39,15 +39,36 @@ export const availableMetricsSelector = createSelector( } ); +export const availableMetricTypesSelector = createListSelector( + [ + availableMetricsSelector, + ], + metric => metric.get('label') +); + export const pinnedMetricSelector = createSelector( [ availableMetricsSelector, state => state.get('pinnedMetricType'), ], - (availableMetrics, pinnedMetricType) => { - const metric = availableMetrics.find(m => m.get('label') === pinnedMetricType); - return metric && metric.get('id'); - } + (availableMetrics, metricType) => availableMetrics.find(m => m.get('label') === metricType) +); + +export const selectedMetricTypeSelector = createSelector( + [ + state => state.get('pinnedMetricType'), + state => state.get('hoveredMetricType'), + ], + (pinnedMetricType, hoveredMetricType) => hoveredMetricType || pinnedMetricType +); + +const selectedMetricIdSelector = createSelector( + [ + availableMetricsSelector, + selectedMetricTypeSelector, + ], + (availableMetrics, metricType) => + (availableMetrics.find(m => m.get('label') === metricType) || makeMap()).get('id') ); const topCardNodeSelector = createSelector( @@ -60,14 +81,14 @@ const topCardNodeSelector = createSelector( export const nodeMetricSelector = createMapSelector( [ state => state.get('nodes'), - state => state.get('selectedMetric'), + selectedMetricIdSelector, topCardNodeSelector, ], - (node, selectedMetric, topCardNode) => { + (node, selectedMetricId, topCardNode) => { const isHighlighted = topCardNode && topCardNode.details && topCardNode.id === node.get('id'); const sourceNode = isHighlighted ? fromJS(topCardNode.details) : node; return sourceNode.get('metrics') && sourceNode.get('metrics') - .filter(m => m.get('id') === selectedMetric) + .filter(m => m.get('id') === selectedMetricId) .first(); } ); diff --git a/client/app/scripts/utils/metric-utils.js b/client/app/scripts/utils/metric-utils.js index 69edfedd0..bf14432e8 100644 --- a/client/app/scripts/utils/metric-utils.js +++ b/client/app/scripts/utils/metric-utils.js @@ -52,15 +52,15 @@ export function getMetricValue(metric) { export function getMetricColor(metric) { - const selectedMetric = metric && metric.get('id'); - if (/mem/.test(selectedMetric)) { + const metricId = metric && metric.get('id'); + if (/mem/.test(metricId)) { return 'steelBlue'; - } else if (/cpu/.test(selectedMetric)) { + } else if (/cpu/.test(metricId)) { return colors('cpu'); - } else if (/files/.test(selectedMetric)) { + } else if (/files/.test(metricId)) { // purple return '#9467bd'; - } else if (/load/.test(selectedMetric)) { + } else if (/load/.test(metricId)) { return colors('load'); } return 'steelBlue';