From 66561e1d8ecd1c5b31a15abf433c8a7a2e3032f3 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 23 Aug 2016 11:23:30 +0200 Subject: [PATCH 1/4] Sort non-number columns ASCending by default. - And number columns DESCending by default. - default: when you sort by that column for the first time. --- .../scripts/components/node-details/node-details-table.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index 31db1870f..a87b4c0d1 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -163,9 +163,11 @@ export default class NodeDetailsTable extends React.Component { handleHeaderClick(ev, headerId) { ev.preventDefault(); - const sortedDesc = headerId === this.state.sortBy - ? !this.state.sortedDesc : this.state.sortedDesc; - const sortBy = headerId; + const header = this.getColumnHeaders().find(h => h.id === headerId); + const defaultSortDesc = header.dataType === 'number'; + const sortedDesc = header.id === this.state.sortBy + ? !this.state.sortedDesc : defaultSortDesc; + const sortBy = header.id; this.setState({sortBy, sortedDesc}); this.props.onSortChange(sortBy, sortedDesc); } From 66191bd6494c22c223fc73fc6b5385d29f6a8794 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 29 Aug 2016 14:12:21 +0200 Subject: [PATCH 2/4] Correct sort order of label column on initial table load - We still have a heuristic: sort first mertric column we find desc. - If there is no metric sort by label column ascending --- .../node-details/node-details-table.js | 57 ++++++++++++------- client/app/scripts/reducers/root.js | 2 +- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index a87b4c0d1..5af554aaf 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -18,6 +18,7 @@ const CW = { XXL: '170px', }; + const COLUMN_WIDTHS = { count: '70px', docker_container_created: CW.XL, @@ -45,7 +46,12 @@ function getDefaultSortBy(columns, nodes) { return defaultSortColumn.id; } // otherwise choose first metric - return _.get(nodes, [0, 'metrics', 0, 'id']); + const firstNodeWithMetrics = _.find(nodes, n => _.get(n, ['metrics', 0])); + if (firstNodeWithMetrics) { + return _.get(firstNodeWithMetrics, ['metrics', 0, 'id']); + } + + return 'label'; } @@ -90,10 +96,10 @@ function getMetaDataSorters(nodes) { } -function sortNodes(nodes, columns, sortBy, sortedDesc) { +function sortNodes(nodes, getValue, sortedDesc) { const sortedNodes = _.sortBy( nodes, - getValueForSortBy(sortBy || getDefaultSortBy(columns, nodes)), + getValue, 'label', getMetaDataSorters(nodes) ); @@ -104,14 +110,14 @@ function sortNodes(nodes, columns, sortBy, sortedDesc) { } -function getSortedNodes(nodes, columns, sortBy, sortedDesc) { - const getValue = getValueForSortBy(sortBy || getDefaultSortBy(columns, nodes)); +function getSortedNodes(nodes, sortBy, sortedDesc) { + const getValue = getValueForSortBy(sortBy); const withAndWithoutValues = _.groupBy(nodes, (n) => { const v = getValue(n); return v !== null && v !== undefined ? 'withValues' : 'withoutValues'; }); - const withValues = sortNodes(withAndWithoutValues.withValues, columns, sortBy, sortedDesc); - const withoutValues = sortNodes(withAndWithoutValues.withoutValues, columns, sortBy, sortedDesc); + const withValues = sortNodes(withAndWithoutValues.withValues, getValue, sortedDesc); + const withoutValues = sortNodes(withAndWithoutValues.withoutValues, getValue, sortedDesc); return _.concat(withValues, withoutValues); } @@ -148,6 +154,11 @@ function getColumnsStyles(headers) { } +function defaultSortDesc(header) { + return header.dataType === 'number'; +} + + export default class NodeDetailsTable extends React.Component { constructor(props, context) { @@ -161,13 +172,12 @@ export default class NodeDetailsTable extends React.Component { this.handleLimitClick = this.handleLimitClick.bind(this); } - handleHeaderClick(ev, headerId) { + handleHeaderClick(ev, headerId, currentSortBy, currentSortedDesc) { ev.preventDefault(); const header = this.getColumnHeaders().find(h => h.id === headerId); - const defaultSortDesc = header.dataType === 'number'; - const sortedDesc = header.id === this.state.sortBy - ? !this.state.sortedDesc : defaultSortDesc; const sortBy = header.id; + const sortedDesc = header.id === currentSortBy + ? !currentSortedDesc : defaultSortDesc(header); this.setState({sortBy, sortedDesc}); this.props.onSortChange(sortBy, sortedDesc); } @@ -182,22 +192,21 @@ export default class NodeDetailsTable extends React.Component { return [{id: 'label', label: this.props.label}].concat(columns); } - renderHeaders() { + renderHeaders(sortBy, sortedDesc) { if (this.props.nodes && this.props.nodes.length > 0) { const headers = this.getColumnHeaders(); const colStyles = getColumnsStyles(headers); - const defaultSortBy = getDefaultSortBy(this.props.columns, this.props.nodes); return ( {headers.map((header, i) => { const headerClasses = ['node-details-table-header', 'truncate']; const onHeaderClick = ev => { - this.handleHeaderClick(ev, header.id); + this.handleHeaderClick(ev, header.id, sortBy, sortedDesc); }; // sort by first metric by default - const isSorted = header.id === (this.state.sortBy || defaultSortBy); - const isSortedDesc = isSorted && this.state.sortedDesc; + const isSorted = header.id === sortBy; + const isSortedDesc = isSorted && sortedDesc; const isSortedAsc = isSorted && !isSortedDesc; if (isSorted) { @@ -222,11 +231,16 @@ export default class NodeDetailsTable extends React.Component { } render() { - const headers = this.renderHeaders(); const { nodeIdKey, columns, topologyId, onClickRow, onMouseEnter, onMouseLeave, onMouseEnterRow, onMouseLeaveRow } = this.props; - let nodes = getSortedNodes(this.props.nodes, this.props.columns, this.state.sortBy, - this.state.sortedDesc); + + const sortBy = this.state.sortBy || getDefaultSortBy(columns, this.props.nodes); + const header = this.getColumnHeaders().find(h => h.id === sortBy); + const sortedDesc = this.state.sortedDesc !== null ? + this.state.sortedDesc : + defaultSortDesc(header); + + let nodes = getSortedNodes(this.props.nodes, sortBy, sortedDesc); 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; @@ -242,7 +256,7 @@ export default class NodeDetailsTable extends React.Component {
- {headers} + {this.renderHeaders(sortBy, sortedDesc)} @@ -264,7 +278,7 @@ export default class NodeDetailsTable extends React.Component {
@@ -277,5 +291,4 @@ export default class NodeDetailsTable extends React.Component { NodeDetailsTable.defaultProps = { nodeIdKey: 'id', // key to identify a node in a row (used for topology links) onSortChange: () => {}, - sortedDesc: true, }; diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index ba9ef0dce..c415103af 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -30,7 +30,7 @@ export const initialState = makeMap({ forceRelayout: false, gridMode: false, gridSortBy: null, - gridSortedDesc: true, + gridSortedDesc: null, highlightedEdgeIds: makeSet(), highlightedNodeIds: makeSet(), hostname: '...', From 35ccc9acd106c7830f34b69281fb06695f53eb88 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 29 Aug 2016 19:38:23 +0200 Subject: [PATCH 3/4] Sory by label properly + sort case insensitivly! --- .../node-details/node-details-table.js | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index 5af554aaf..c08ecdcb7 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -55,24 +55,36 @@ function getDefaultSortBy(columns, nodes) { } +function toLower(value) { + if (!value || !value.toLowerCase) { + return value; + } + return value.toLowerCase(); +} + + function getValueForSortBy(sortBy) { // return the node's value based on the sortBy field return (node) => { if (sortBy !== null) { let field = _.union(node.metrics, node.metadata).find(f => f.id === sortBy); - if (!field && node.parents) { - field = node.parents.find(f => f.topologyId === sortBy); - if (field) { - return field.label; - } - } - if (field) { if (isNumberField(field)) { return parseFloat(field.value); } - return field.value; + return toLower(field.value); + } + + if (node.parents) { + field = node.parents.find(f => f.topologyId === sortBy); + if (field) { + return toLower(field.label); + } + } + + if (node[sortBy] !== undefined && node[sortBy] !== null) { + return toLower(node[sortBy]); } } @@ -100,7 +112,6 @@ function sortNodes(nodes, getValue, sortedDesc) { const sortedNodes = _.sortBy( nodes, getValue, - 'label', getMetaDataSorters(nodes) ); if (sortedDesc) { From a1dd4eb9967a07575e1c905780a4278179d5b601 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 30 Aug 2016 09:07:18 +0200 Subject: [PATCH 4/4] Small refactor of case insensitive table sorting. --- .../node-details/node-details-table.js | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index c08ecdcb7..11bceaf5e 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -55,7 +55,7 @@ function getDefaultSortBy(columns, nodes) { } -function toLower(value) { +function maybeToLower(value) { if (!value || !value.toLowerCase) { return value; } @@ -63,33 +63,35 @@ function toLower(value) { } -function getValueForSortBy(sortBy) { - // return the node's value based on the sortBy field - return (node) => { - if (sortBy !== null) { - let field = _.union(node.metrics, node.metadata).find(f => f.id === sortBy); +function getNodeValue(node, fieldId) { + if (fieldId !== null) { + let field = _.union(node.metrics, node.metadata).find(f => f.id === fieldId); + if (field) { + if (isNumberField(field)) { + return parseFloat(field.value); + } + return field.value; + } + + if (node.parents) { + field = node.parents.find(f => f.topologyId === fieldId); if (field) { - if (isNumberField(field)) { - return parseFloat(field.value); - } - return toLower(field.value); - } - - if (node.parents) { - field = node.parents.find(f => f.topologyId === sortBy); - if (field) { - return toLower(field.label); - } - } - - if (node[sortBy] !== undefined && node[sortBy] !== null) { - return toLower(node[sortBy]); + return field.label; } } - return null; - }; + if (node[fieldId] !== undefined && node[fieldId] !== null) { + return node[fieldId]; + } + } + + return null; +} + + +function getValueForSortBy(sortBy) { + return (node) => maybeToLower(getNodeValue(node, sortBy)); }