Fixed row focusing for the bottom of a scrollable table

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

@@ -76,17 +76,34 @@ 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);
     this.onMouseDown = this.onMouseDown.bind(this);
     this.onMouseUp = this.onMouseUp.bind(this);
+    this.onMouseEnter = this.onMouseEnter.bind(this);
+    this.onMouseLeave = this.onMouseLeave.bind(this);
   }
 
   saveLabelElementRef(ref) {
     this.labelElement = ref;
   }
 
+  onMouseEnter() {
+    this.setState({ focused: true });
+    if (this.props.onMouseEnter) {
+      this.props.onMouseEnter();
+    }
+  }
+
+  onMouseLeave() {
+    this.setState({ focused: false });
+    if (this.props.onMouseLeave) {
+      this.props.onMouseLeave();
+    }
+  }
+
   onMouseDown(ev) {
     const { pageX, pageY } = ev;
     this.mouseDragOrigin = [pageX, pageY];
@@ -109,18 +126,22 @@ export default class NodeDetailsTableRow extends React.Component {
   }
 
   render() {
-    const { node, nodeIdKey, topologyId, columns, onClick, 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={this.props.onMouseEnter}
-        onMouseLeave={this.props.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,4 +1,3 @@
-import debug from 'debug';
 import React from 'react';
 import classNames from 'classnames';
 import { find, get, union, sortBy, groupBy, concat, debounce, findIndex } from 'lodash';
@@ -117,19 +116,32 @@ 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.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);
@@ -147,21 +159,23 @@ export default class NodeDetailsTable extends React.Component {
   }
 
   focusRow(rowIndex, node) {
-    this.setState({
+    // 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,
-      focusedNode: node
-    });
-    log(`Focused row ${rowIndex}`);
+      tableContentMinHeightConstraint: this.tableContent.scrollHeight,
+    };
   }
 
   unfocusRow() {
-    if (this.state.focusedRowIndex) {
-      this.setState({
-        focusedRowIndex: null,
-        focusedNode: null
-      });
-      log('Unfocused row');
-    }
+    // Reset the focus state
+    this.focusState = {};
   }
 
   onMouseEnterRow(rowIndex, node) {
@@ -174,33 +188,38 @@ export default class NodeDetailsTable extends React.Component {
     this.debouncedUnfocusRow();
   }
 
+  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 } = this.props;
-    const { focusedRowIndex, focusedNode } = this.state;
 
     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);
-    if (focusedRowIndex && focusedRowIndex < nodes.length) {
+
+    const { focusedNode, focusedRowIndex, tableContentMinHeightConstraint } = this.focusState;
+    if (Number.isInteger(focusedRowIndex) && focusedRowIndex < nodes.length) {
       const nodeRowIndex = findIndex(nodes, 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.
-        moveElement(nodes, nodeRowIndex, focusedRowIndex);
+        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.
-        insertElement(nodes, focusedRowIndex, focusedNode);
+        nodes = insertElement(nodes, focusedRowIndex, focusedNode);
       }
     }
 
@@ -229,6 +248,7 @@ export default class NodeDetailsTable extends React.Component {
             </thead>
             <tbody
               style={this.props.tbodyStyle}
+              ref={this.saveTableContentRef}
               onMouseEnter={onMouseEnter}
               onMouseLeave={onMouseLeave}>
               {nodes && nodes.map((node, index) => (
@@ -245,6 +265,7 @@ export default class NodeDetailsTable extends React.Component {
                   onMouseLeave={this.onMouseLeaveRow}
                   topologyId={topologyId} />
               ))}
+              {minHeightConstraint(tableContentMinHeightConstraint)}
             </tbody>
           </table>
           <ShowMore

client/app/scripts/constants/timer.js

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

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

@@ -1,18 +1,24 @@
 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');
 
   describe('uniformSelect', () => {
     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]);
@@ -47,26 +51,33 @@ describe('ArrayUtils', () => {
   });
 
   describe('insertElement', () => {
-    const f = (array, index, element) => {
-      ArrayUtils.insertElement(array, index, element);
-      return array;
-    };
+    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 = (array, from, to) => {
-      ArrayUtils.moveElement(array, from, to);
-      return array;
-    };
+    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']);

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;
@@ -11,12 +13,16 @@ export function uniformSelect(array, size) {
 }
 
 export function insertElement(array, index, element) {
-  array.splice(index, 0, 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) {
-    const removedElement = array.splice(from, 1)[0];
-    insertElement(array, to, removedElement);
+  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;
     }