diff --git a/client/app/scripts/components/node-details/node-details-table-row.js b/client/app/scripts/components/node-details/node-details-table-row.js index a26bee551..0e32e4508 100644 --- a/client/app/scripts/components/node-details/node-details-table-row.js +++ b/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 ( ; +} + + 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 { - {nodes && nodes.map(node => ( + {nodes && nodes.map((node, index) => ( ))} + {minHeightConstraint(tableContentMinHeightConstraint)} { 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']); + }); + }); }); diff --git a/client/app/scripts/utils/array-utils.js b/client/app/scripts/utils/array-utils.js index c07891398..0f962755c 100644 --- a/client/app/scripts/utils/array-utils.js +++ b/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]); +} diff --git a/client/app/styles/main.less b/client/app/styles/main.less index b209661cd..9a9c64396 100644 --- a/client/app/styles/main.less +++ b/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; }