Merge pull request #2115 from weaveworks/node-table-maintain-focus

Maintain focus on hovered node table rows

client/app/scripts/components/node-details/node-details-table-row.js

@@ -76,6 +76,7 @@ export default class NodeDetailsTableRow extends React.Component {
     // user is selecting some data in the row. In this case don't trigger the onClick event which
     // is most likely a details panel popping open.
     //
+    this.state = { focused: false };
     this.mouseDragOrigin = [0, 0];
 
     this.saveLabelElementRef = this.saveLabelElementRef.bind(this);
@@ -90,13 +91,17 @@ export default class NodeDetailsTableRow extends React.Component {
   }
 
   onMouseEnter() {
-    const { node, onMouseEnterRow } = this.props;
-    onMouseEnterRow(node);
+    this.setState({ focused: true });
+    if (this.props.onMouseEnter) {
+      this.props.onMouseEnter(this.props.index, this.props.node);
+    }
   }
 
   onMouseLeave() {
-    const { node, onMouseLeaveRow } = this.props;
-    onMouseLeaveRow(node);
+    this.setState({ focused: false });
+    if (this.props.onMouseLeave) {
+      this.props.onMouseLeave();
+    }
   }
 
   onMouseDown(ev) {
@@ -121,19 +126,22 @@ export default class NodeDetailsTableRow extends React.Component {
   }
 
   render() {
-    const { node, nodeIdKey, topologyId, columns, onClick, onMouseEnterRow, onMouseLeaveRow,
-      selected, colStyles } = this.props;
+    const { node, nodeIdKey, topologyId, columns, onClick, colStyles } = this.props;
     const [firstColumnStyle, ...columnStyles] = colStyles;
     const values = renderValues(node, columns, columnStyles);
     const nodeId = node[nodeIdKey];
-    const className = classNames('node-details-table-node', { selected });
+
+    const className = classNames('node-details-table-node', {
+      selected: this.props.selected,
+      focused: this.state.focused,
+    });
 
     return (
       <tr
         onMouseDown={onClick && this.onMouseDown}
         onMouseUp={onClick && this.onMouseUp}
-        onMouseEnter={onMouseEnterRow && this.onMouseEnter}
-        onMouseLeave={onMouseLeaveRow && this.onMouseLeave}
+        onMouseEnter={this.onMouseEnter}
+        onMouseLeave={this.onMouseLeave}
         className={className}>
         <td
           className="node-details-table-node-label truncate"

client/app/scripts/components/node-details/node-details-table.js

@@ -1,6 +1,6 @@
 import React from 'react';
 import classNames from 'classnames';
-import { find, get, union, sortBy, groupBy, concat } from 'lodash';
+import { find, get, union, sortBy, groupBy, concat, debounce } from 'lodash';
 
 import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits';
 
@@ -8,6 +8,8 @@ import ShowMore from '../show-more';
 import NodeDetailsTableRow from './node-details-table-row';
 import NodeDetailsTableHeaders from './node-details-table-headers';
 import { ipToPaddedString } from '../../utils/string-utils';
+import { moveElement, insertElement } from '../../utils/array-utils';
+import { TABLE_ROW_FOCUS_DEBOUNCE_INTERVAL } from '../../constants/timer';
 import {
   isIP, isNumber, defaultSortDesc, getTableColumnsStyles
 } from '../../utils/node-details-utils';
@@ -114,17 +116,36 @@ function getSortedNodes(nodes, sortedByHeader, sortedDesc) {
 }
 
 
+// By inserting this fake invisible row into the table, with the help of
+// some CSS trickery, we make the inner scrollable content of the table
+// have a minimal height. That prevents auto-scroll under a focus if the
+// number of table rows shrinks.
+function minHeightConstraint(height = 0) {
+  return <tr className="min-height-constraint" style={{height}} />;
+}
+
+
 export default class NodeDetailsTable extends React.Component {
 
   constructor(props, context) {
     super(props, context);
+
     this.state = {
       limit: props.limit || NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT,
       sortedDesc: this.props.sortedDesc,
       sortedBy: this.props.sortedBy
     };
-    this.handleLimitClick = this.handleLimitClick.bind(this);
+    this.focusState = {};
+
     this.updateSorted = this.updateSorted.bind(this);
+    this.handleLimitClick = this.handleLimitClick.bind(this);
+    this.onMouseLeaveRow = this.onMouseLeaveRow.bind(this);
+    this.onMouseEnterRow = this.onMouseEnterRow.bind(this);
+    this.saveTableContentRef = this.saveTableContentRef.bind(this);
+    // Use debouncing to prevent event flooding when e.g. crossing fast with mouse cursor
+    // over the whole table. That would be expensive as each focus causes table to rerender.
+    this.debouncedFocusRow = debounce(this.focusRow, TABLE_ROW_FOCUS_DEBOUNCE_INTERVAL);
+    this.debouncedBlurRow = debounce(this.blurRow, TABLE_ROW_FOCUS_DEBOUNCE_INTERVAL);
   }
 
   updateSorted(sortedBy, sortedDesc) {
@@ -137,20 +158,71 @@ export default class NodeDetailsTable extends React.Component {
     this.setState({ limit });
   }
 
+  focusRow(rowIndex, node) {
+    // Remember the focused row index, the node that was focused and
+    // the table content height so that we can keep the node row fixed
+    // without auto-scrolling happening.
+    // NOTE: It would be ideal to modify the real component state here,
+    // but that would cause whole table to rerender, which becomes to
+    // expensive with the current implementation if the table consists
+    // of 1000+ nodes.
+    this.focusState = {
+      focusedNode: node,
+      focusedRowIndex: rowIndex,
+      tableContentMinHeightConstraint: this.tableContent.scrollHeight,
+    };
+  }
+
+  blurRow() {
+    // Reset the focus state
+    this.focusState = {};
+  }
+
+  onMouseEnterRow(rowIndex, node) {
+    this.debouncedBlurRow.cancel();
+    this.debouncedFocusRow(rowIndex, node);
+  }
+
+  onMouseLeaveRow() {
+    this.debouncedFocusRow.cancel();
+    this.debouncedBlurRow();
+  }
+
+  saveTableContentRef(ref) {
+    this.tableContent = ref;
+  }
+
   getColumnHeaders() {
     const columns = this.props.columns || [];
     return [{id: 'label', label: this.props.label}].concat(columns);
   }
 
   render() {
-    const { nodeIdKey, columns, topologyId, onClickRow, onMouseEnter, onMouseLeave,
-      onMouseEnterRow, onMouseLeaveRow } = this.props;
+    const { nodeIdKey, columns, topologyId, onClickRow, onMouseEnter, onMouseLeave } = this.props;
 
     const sortedBy = this.state.sortedBy || getDefaultSortedBy(columns, this.props.nodes);
     const sortedByHeader = this.getColumnHeaders().find(h => h.id === sortedBy);
     const sortedDesc = this.state.sortedDesc || defaultSortDesc(sortedByHeader);
 
     let nodes = getSortedNodes(this.props.nodes, sortedByHeader, sortedDesc);
+
+    const { focusedNode, focusedRowIndex, tableContentMinHeightConstraint } = this.focusState;
+    if (Number.isInteger(focusedRowIndex) && focusedRowIndex < nodes.length) {
+      const nodeRowIndex = nodes.findIndex(node => node.id === focusedNode.id);
+      if (nodeRowIndex >= 0) {
+        // If the focused node still exists in the table, we move it
+        // to the hovered row, keeping the rest of the table sorted.
+        nodes = moveElement(nodes, nodeRowIndex, focusedRowIndex);
+      } else {
+        // Otherwise we insert the dead focused node there, pretending
+        // it's still alive. That enables the users to read off all the
+        // info they want and perhaps even open the details panel. Also,
+        // only if we do this, we can guarantee that mouse hover will
+        // always freeze the table row until we focus out.
+        nodes = insertElement(nodes, focusedRowIndex, focusedNode);
+      }
+    }
+
     const limited = nodes && this.state.limit > 0 && nodes.length > this.state.limit;
     const expanded = this.state.limit === 0;
     const notShown = nodes.length - this.state.limit;
@@ -176,22 +248,25 @@ export default class NodeDetailsTable extends React.Component {
             </thead>
             <tbody
               style={this.props.tbodyStyle}
+              ref={this.saveTableContentRef}
               onMouseEnter={onMouseEnter}
               onMouseLeave={onMouseLeave}>
-              {nodes && nodes.map(node => (
+              {nodes && nodes.map((node, index) => (
                 <NodeDetailsTableRow
                   key={node.id}
                   renderIdCell={this.props.renderIdCell}
                   selected={this.props.selectedNodeId === node.id}
                   node={node}
+                  index={index}
                   nodeIdKey={nodeIdKey}
                   colStyles={styles}
                   columns={columns}
                   onClick={onClickRow}
-                  onMouseLeaveRow={onMouseLeaveRow}
-                  onMouseEnterRow={onMouseEnterRow}
+                  onMouseEnter={this.onMouseEnterRow}
+                  onMouseLeave={this.onMouseLeaveRow}
                   topologyId={topologyId} />
               ))}
+              {minHeightConstraint(tableContentMinHeightConstraint)}
             </tbody>
           </table>
           <ShowMore

client/app/scripts/constants/timer.js

@@ -1,3 +1,4 @@
 /* Intervals in ms */
 export const API_INTERVAL = 30000;
 export const TOPOLOGY_INTERVAL = 5000;
+export const TABLE_ROW_FOCUS_DEBOUNCE_INTERVAL = 10;

client/app/scripts/utils/__tests__/array-utils-test.js

@@ -1,5 +1,11 @@
 import { range } from 'lodash';
 
+function testNotMutatingArray(f, array, ...otherArgs) {
+  const original = array.slice();
+  f(array, ...otherArgs);
+  expect(array).toEqual(original);
+}
+
 describe('ArrayUtils', () => {
   const ArrayUtils = require('../array-utils');
 
@@ -7,12 +13,12 @@ describe('ArrayUtils', () => {
     const f = ArrayUtils.uniformSelect;
 
     it('it should select the array elements uniformly, including the endpoints', () => {
+      testNotMutatingArray(f, ['A', 'B', 'C', 'D', 'E'], 3);
       {
         const arr = ['x', 'y'];
         expect(f(arr, 3)).toEqual(['x', 'y']);
         expect(f(arr, 2)).toEqual(['x', 'y']);
       }
-
       {
         const arr = ['A', 'B', 'C', 'D', 'E'];
         expect(f(arr, 6)).toEqual(['A', 'B', 'C', 'D', 'E']);
@@ -21,7 +27,6 @@ describe('ArrayUtils', () => {
         expect(f(arr, 3)).toEqual(['A', 'C', 'E']);
         expect(f(arr, 2)).toEqual(['A', 'E']);
       }
-
       {
         const arr = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
         expect(f(arr, 12)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]);
@@ -36,7 +41,6 @@ describe('ArrayUtils', () => {
         expect(f(arr, 3)).toEqual([1, 6, 11]);
         expect(f(arr, 2)).toEqual([1, 11]);
       }
-
       {
         const arr = range(1, 10001);
         expect(f(arr, 4)).toEqual([1, 3334, 6667, 10000]);
@@ -45,4 +49,47 @@ describe('ArrayUtils', () => {
       }
     });
   });
+
+  describe('insertElement', () => {
+    const f = ArrayUtils.insertElement;
+
+    it('it should insert an element into the array at the specified index', () => {
+      testNotMutatingArray(f, ['x', 'y', 'z'], 0, 'a');
+      expect(f(['x', 'y', 'z'], 0, 'a')).toEqual(['a', 'x', 'y', 'z']);
+      expect(f(['x', 'y', 'z'], 1, 'a')).toEqual(['x', 'a', 'y', 'z']);
+      expect(f(['x', 'y', 'z'], 2, 'a')).toEqual(['x', 'y', 'a', 'z']);
+      expect(f(['x', 'y', 'z'], 3, 'a')).toEqual(['x', 'y', 'z', 'a']);
+    });
+  });
+
+  describe('removeElement', () => {
+    const f = ArrayUtils.removeElement;
+
+    it('it should remove the element at the specified index from the array', () => {
+      testNotMutatingArray(f, ['x', 'y', 'z'], 0);
+      expect(f(['x', 'y', 'z'], 0)).toEqual(['y', 'z']);
+      expect(f(['x', 'y', 'z'], 1)).toEqual(['x', 'z']);
+      expect(f(['x', 'y', 'z'], 2)).toEqual(['x', 'y']);
+    });
+  });
+
+  describe('moveElement', () => {
+    const f = ArrayUtils.moveElement;
+
+    it('it should move an array element, modifying the array', () => {
+      testNotMutatingArray(f, ['x', 'y', 'z'], 0, 1);
+      expect(f(['x', 'y', 'z'], 0, 1)).toEqual(['y', 'x', 'z']);
+      expect(f(['x', 'y', 'z'], 1, 0)).toEqual(['y', 'x', 'z']);
+      expect(f(['x', 'y', 'z'], 0, 2)).toEqual(['y', 'z', 'x']);
+      expect(f(['x', 'y', 'z'], 2, 0)).toEqual(['z', 'x', 'y']);
+      expect(f(['x', 'y', 'z'], 1, 2)).toEqual(['x', 'z', 'y']);
+      expect(f(['x', 'y', 'z'], 2, 1)).toEqual(['x', 'z', 'y']);
+      expect(f(['x', 'y', 'z'], 0, 0)).toEqual(['x', 'y', 'z']);
+      expect(f(['x', 'y', 'z'], 1, 1)).toEqual(['x', 'y', 'z']);
+      expect(f(['x', 'y', 'z'], 2, 2)).toEqual(['x', 'y', 'z']);
+      expect(f(['a', 'b', 'c', 'd', 'e'], 4, 1)).toEqual(['a', 'e', 'b', 'c', 'd']);
+      expect(f(['a', 'b', 'c', 'd', 'e'], 1, 4)).toEqual(['a', 'c', 'd', 'e', 'b']);
+      expect(f(['a', 'b', 'c', 'd', 'e'], 1, 3)).toEqual(['a', 'c', 'd', 'b', 'e']);
+    });
+  });
 });

client/app/scripts/utils/array-utils.js

@@ -1,5 +1,7 @@
 import { range } from 'lodash';
 
+// NOTE: All the array operations defined here should be non-mutating.
+
 export function uniformSelect(array, size) {
   if (size > array.length) {
     return array;
@@ -9,3 +11,18 @@ export function uniformSelect(array, size) {
     array[parseInt(index * (array.length / (size - (1 - 1e-9))), 10)]
   );
 }
+
+export function insertElement(array, index, element) {
+  return array.slice(0, index).concat([element], array.slice(index));
+}
+
+export function removeElement(array, index) {
+  return array.slice(0, index).concat(array.slice(index + 1));
+}
+
+export function moveElement(array, from, to) {
+  if (from === to) {
+    return array;
+  }
+  return insertElement(removeElement(array, from), to, array[from]);
+}

client/app/styles/main.less

@@ -940,14 +940,21 @@ h2 {
       }
     }
 
+    tbody {
+      position: relative;
+
+      .min-height-constraint {
+        position: absolute;
+        width: 0 !important;
+        opacity: 0;
+        top: 0;
+      }
+    }
+
     &-node {
       font-size: 105%;
       line-height: 1.5;
 
-      &:hover, &.selected {
-        background-color: lighten(@background-color, 5%);
-      }
-
       > * {
         padding: 0 4px;
       }
@@ -990,6 +997,19 @@ h2 {
   }
 }
 
+// This part sets the styles only for the 'real' node details table, not applying
+// them to the nodes grid, because there we control hovering from the JS.
+// NOTE: Maybe it would be nice to separate the class names between the two places
+// where node tables are used - i.e. it doesn't make sense that node-details-table
+// can also refer to the tables in the nodes grid.
+.details-wrapper .node-details-table {
+  &-node {
+    &:hover, &.selected {
+      background-color: lighten(@background-color, 5%);
+    }
+  }
+}
+
 .node-control-button {
   .btn-opacity;
   padding: 6px;
@@ -1785,8 +1805,9 @@ h2 {
       padding: 3px 4px;
     }
 
+    // Keeping the row height fixed is important for locking the rows on hover.
     .node-details-table-node, thead tr {
-      height: 24px;
+      height: 28px;
     }
 
     tr:nth-child(even) {
@@ -1799,7 +1820,10 @@ h2 {
       cursor: pointer;
     }
 
-    tbody tr.selected, tbody tr:hover {
+    // We fully control hovering of the grid rows from JS,
+    // because we want consistent behaviour between the
+    // visual and row locking logic that happens on hover.
+    tbody tr.selected, tbody tr.focused {
       background-color: #d7ecf5;
       border: 1px solid @weave-blue;
     }