Polished the metric selection logic (#2468)

* Polished the metric selection code

* Fixed hovering advantage over pinning.

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);

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());

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 (
       <div
-        key={id}
+        key={type}
         className={className}
         onMouseOver={this.onMouseOver}
         onClick={this.onMouseClick}>
-        {metric.get('label')}
+        {type}
         {isPinned && <span className="fa fa-thumb-tack" />}
       </div>
     );
@@ -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);

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 => (
             <MetricSelectorItem
               key={metric.get('id')}
-              alwaysPinned={alwaysPinned}
               metric={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);

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)}
         </div>
-        <MetricSelector alwaysPinned={isResourceViewMode} />
+        <MetricSelector />
       </div>
     );
   }

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',

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)) {

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();
   }
 );

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';