Browse Source

Network: force array order when sorting hierarchical levels (#3576)

* Network: force array order when sorting hierarchical levels

Fixes #340r34.

If coordinates are not available to sort within a hierarchical level, sort to array order instead.

The previous fix on this issue was not good enough to circumvent this quirk in the chromium sorting. This should bury it.

* Added TimSort for sorting in DirectionStrategy

* Added TimSort to LayoutEngine

* Fixed typo
mbroad/code-climate-coverage-develop
wimrijnders 7 years ago
committed by Yotam Berkowitz
parent
commit
88f0a6a4c6
3 changed files with 17 additions and 9 deletions
  1. +3
    -2
      lib/network/modules/LayoutEngine.js
  2. +12
    -6
      lib/network/modules/components/DirectionStrategy.js
  3. +2
    -1
      package.json

+ 3
- 2
lib/network/modules/LayoutEngine.js View File

@ -1,4 +1,3 @@
'use strict';
/** /**
* There's a mix-up with terms in the code. Following are the formal definitions: * There's a mix-up with terms in the code. Following are the formal definitions:
* *
@ -30,6 +29,8 @@
* The layout is a way to arrange the nodes in the view; this can be done * The layout is a way to arrange the nodes in the view; this can be done
* on non-hierarchical networks as well. The converse is also possible. * on non-hierarchical networks as well. The converse is also possible.
*/ */
'use strict';
var TimSort = require('timsort');
let util = require('../../util'); let util = require('../../util');
var NetworkUtil = require('../NetworkUtil').default; var NetworkUtil = require('../NetworkUtil').default;
var {HorizontalStrategy, VerticalStrategy} = require('./components/DirectionStrategy.js'); var {HorizontalStrategy, VerticalStrategy} = require('./components/DirectionStrategy.js');
@ -1422,7 +1423,7 @@ class LayoutEngine {
result.push(Number(size)); result.push(Number(size));
}); });
result.sort(function(a, b) {
TimSort.sort(result, function(a, b) {
return b - a; return b - a;
}); });

+ 12
- 6
lib/network/modules/components/DirectionStrategy.js View File

@ -3,6 +3,7 @@
* *
* Strategy pattern for usage of direction methods for hierarchical layouts. * Strategy pattern for usage of direction methods for hierarchical layouts.
*/ */
var TimSort = require('timsort');
/** /**
@ -87,6 +88,15 @@ class DirectionInterface {
/** /**
* Sort array of nodes on the unfixed coordinates. * Sort array of nodes on the unfixed coordinates.
* *
* **Note:** chrome has non-stable sorting implementation, which
* has a tendency to change the order of the array items,
* even if the custom sort function returns 0.
*
* For this reason, an external sort implementation is used,
* which has the added benefit of being faster than the standard
* platforms implementation. This has been verified on `node.js`,
* `firefox` and `chrome` (all linux).
*
* @param {Array.<Node>} nodeArray array of nodes to sort * @param {Array.<Node>} nodeArray array of nodes to sort
*/ */
sort(nodeArray) { this.fake_use(nodeArray); this.abstract(); } sort(nodeArray) { this.fake_use(nodeArray); this.abstract(); }
@ -156,9 +166,7 @@ class VerticalStrategy extends DirectionInterface {
/** @inheritdoc */ /** @inheritdoc */
sort(nodeArray) { sort(nodeArray) {
nodeArray.sort(function(a, b) {
// Test on 'undefined' takes care of divergent behaviour in chrome
if (a.x === undefined || b.x === undefined) return 0; // THIS HAPPENS
TimSort.sort(nodeArray, function(a, b) {
return a.x - b.x; return a.x - b.x;
}); });
} }
@ -221,9 +229,7 @@ class HorizontalStrategy extends DirectionInterface {
/** @inheritdoc */ /** @inheritdoc */
sort(nodeArray) { sort(nodeArray) {
nodeArray.sort(function(a, b) {
// Test on 'undefined' takes care of divergent behaviour in chrome
if (a.y === undefined || b.y === undefined) return 0; // THIS HAPPENS
TimSort.sort(nodeArray, function(a, b) {
return a.y - b.y; return a.y - b.y;
}); });
} }

+ 2
- 1
package.json View File

@ -38,7 +38,8 @@
"hammerjs": "^2.0.8", "hammerjs": "^2.0.8",
"keycharm": "^0.2.0", "keycharm": "^0.2.0",
"moment": "^2.18.1", "moment": "^2.18.1",
"propagating-hammerjs": "^1.4.6"
"propagating-hammerjs": "^1.4.6",
"timsort": "^0.3.0"
}, },
"devDependencies": { "devDependencies": {
"async": "^2.5.0", "async": "^2.5.0",

Loading…
Cancel
Save