From 5cf882b1832c0d628eb81682bc1d9090569184f8 Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Wed, 4 Oct 2017 09:03:24 +0200 Subject: [PATCH 1/5] Network: Retain constraint values in label font handling Fixes #3517. Due to changed logic in the label font handling, the option values for `widthConstraint` and `heightConstraint` were overwritten. The fix is in effect a reversal of two code lines: parsing constraint options should come *after* parsing (multi)font options. Further changes: - Additional 1-liner fix: constraint values were not copied for edge-instance specific options. - Small refactoriing of `Label#constrain()` in order to separate concerns - Added unit test for regression testing of this issue. This leads to the curious observation that, while the actual change is two lines of source code, this resulted in a +-150 line regression test. --- lib/network/modules/components/Edge.js | 3 +- .../modules/components/shared/Label.js | 39 ++-- test/Label.test.js | 174 ++++++++++++++++++ 3 files changed, 200 insertions(+), 16 deletions(-) diff --git a/lib/network/modules/components/Edge.js b/lib/network/modules/components/Edge.js index f66fc53db..529b57bdb 100644 --- a/lib/network/modules/components/Edge.js +++ b/lib/network/modules/components/Edge.js @@ -132,7 +132,8 @@ class Edge { 'value', 'width', 'font', - 'chosen' + 'chosen', + 'widthConstraint' ]; // only deep extend the items in the field array. These do not have shorthand. diff --git a/lib/network/modules/components/shared/Label.js b/lib/network/modules/components/shared/Label.js index abbc31da2..f810a244a 100644 --- a/lib/network/modules/components/shared/Label.js +++ b/lib/network/modules/components/shared/Label.js @@ -141,48 +141,57 @@ class Label { /** * Set the width and height constraints based on 'nearest' value + * * @param {Array} pile array of option objects to consider + * @returns {object} the actual constraint values to use * @private */ constrain(pile) { - this.fontOptions.constrainWidth = false; - this.fontOptions.maxWdt = -1; - this.fontOptions.minWdt = -1; + // Note that of these fields, only 'constrainWidth' and 'maxWdt' + // will be set for edge labels. + // Node labels can set all the fields + let fontOptions = { + constrainWidth: false, + maxWdt: -1, + minWdt: -1, + constrainHeight: false, + minHgt: -1, + valign: 'middle', + } let widthConstraint = util.topMost(pile, 'widthConstraint'); if (typeof widthConstraint === 'number') { - this.fontOptions.maxWdt = Number(widthConstraint); - this.fontOptions.minWdt = Number(widthConstraint); + fontOptions.maxWdt = Number(widthConstraint); + fontOptions.minWdt = Number(widthConstraint); } else if (typeof widthConstraint === 'object') { let widthConstraintMaximum = util.topMost(pile, ['widthConstraint', 'maximum']); if (typeof widthConstraintMaximum === 'number') { - this.fontOptions.maxWdt = Number(widthConstraintMaximum); + fontOptions.maxWdt = Number(widthConstraintMaximum); } let widthConstraintMinimum = util.topMost(pile, ['widthConstraint', 'minimum']) if (typeof widthConstraintMinimum === 'number') { - this.fontOptions.minWdt = Number(widthConstraintMinimum); + fontOptions.minWdt = Number(widthConstraintMinimum); } } - this.fontOptions.constrainHeight = false; - this.fontOptions.minHgt = -1; - this.fontOptions.valign = 'middle'; let heightConstraint = util.topMost(pile, 'heightConstraint'); if (typeof heightConstraint === 'number') { - this.fontOptions.minHgt = Number(heightConstraint); + fontOptions.minHgt = Number(heightConstraint); } else if (typeof heightConstraint === 'object') { let heightConstraintMinimum = util.topMost(pile, ['heightConstraint', 'minimum']); if (typeof heightConstraintMinimum === 'number') { - this.fontOptions.minHgt = Number(heightConstraintMinimum); + fontOptions.minHgt = Number(heightConstraintMinimum); } let heightConstraintValign = util.topMost(pile, ['heightConstraint', 'valign']); if (typeof heightConstraintValign === 'string') { - if ((heightConstraintValign === 'top')||(heightConstraintValign === 'bottom')) { - this.fontOptions.valign = heightConstraintValign; + if ((heightConstraintValign === 'top')|| (heightConstraintValign === 'bottom')) { + fontOptions.valign = heightConstraintValign; } } } + + return fontOptions; } @@ -194,8 +203,8 @@ class Label { */ update(options, pile) { this.setOptions(options, true); - this.constrain(pile); this.propagateFonts(pile); + util.deepExtend(this.fontOptions, this.constrain(pile)); this.fontOptions.chooser = ComponentUtil.choosify('label', pile); } diff --git a/test/Label.test.js b/test/Label.test.js index c29b39bae..14c2712c4 100644 --- a/test/Label.test.js +++ b/test/Label.test.js @@ -1407,4 +1407,178 @@ describe('Shorthand Font Options', function() { done(); }); + + + /** + * + * The test network is taken verbatim from example `network/nodeStyles/widthHeight.html`, + * where the associated issue (i.e. widthConstraint values not copied) was most poignant. + * + * If you want to be really pedantically paranoid about it, widthConstraint should also + * be tested when set in groups. perhaps TODO. + * + * perhaps TODO: check boolean shorthand values for widthConstraint. + * perhaps TODO: check shorthand values for heightConstraint. + */ + it('Sets the width/height constraints in the font label options', function (done) { + var nodes = [ + { id: 100, label: 'This node has no fixed, minimum or maximum width or height', x: -50, y: -300 }, + { id: 210, widthConstraint: { minimum: 120 }, label: 'This node has a mimimum width', x: -400, y: -200 }, + { id: 211, widthConstraint: { minimum: 120 }, label: '...so does this', x: -500, y: -50 }, + { id: 220, widthConstraint: { maximum: 170 }, label: 'This node has a maximum width and breaks have been automatically inserted into the label', x: -150, y: -150 }, + { id: 221, widthConstraint: { maximum: 170 }, label: '...this too', x: -100, y: 0 }, + { id: 200, font: { multi: true }, widthConstraint: 150, label: 'This node has a fixed width and breaks have been automatically inserted into the label', x: -300, y: 50 }, + { id: 201, widthConstraint: 150, label: '...this as well', x: -300, y: 200 }, + { id: 300, heightConstraint: { minimum: 70 }, label: 'This node\nhas a\nminimum\nheight', x: 100, y: -150 }, + { id: 301, heightConstraint: { minimum: 70 }, label: '...this one here too', x: 100, y: 0 }, + { id: 400, heightConstraint: { minimum: 100, valign: 'top' }, label: 'Minimum height\nvertical alignment\nmay be top', x: 300, y: -200 }, + { id: 401, heightConstraint: { minimum: 100, valign: 'middle' }, label: 'Minimum height\nvertical alignment\nmay be middle\n(default)', x: 300, y: 0 }, + { id: 402, heightConstraint: { minimum: 100, valign: 'bottom' }, label: 'Minimum height\nvertical alignment\nmay be bottom', x: 300, y: 200 } + ]; + + var edges = [ + { from: 100, to: 210, label: "unconstrained to minimum width"}, + { from: 210, to: 211, label: "more minimum width"}, + { from: 100, to: 220, label: "unconstrained to maximum width"}, + { from: 220, to: 221, label: "more maximum width"}, + { from: 210, to: 200, label: "minimum width to fixed width"}, + { from: 220, to: 200, label: "maximum width to fixed width"}, + { from: 200, to: 201, label: "more fixed width"}, + { from: 100, to: 300, label: "unconstrained to minimum height"}, + { from: 300, to: 301, label: "more minimum height"}, + { from: 100, to: 400, label: "unconstrained to top valign"}, + { from: 400, to: 401, label: "top valign to middle valign"}, + { from: 401, to: 402, widthConstraint: { maximum: 150 }, label: "middle valign to bottom valign"}, + ]; + + var container = document.getElementById('mynetwork'); + var data = { + nodes: nodes, + edges: edges + }; + var options = { + edges: { + font: { + size: 12 + }, + widthConstraint: { + maximum: 90 + } + }, + nodes: { + shape: 'box', + margin: 10, + widthConstraint: { + maximum: 200 + } + }, + physics: { + enabled: false + } + }; + var network = new vis.Network(container, data, options); + + // + // Check the internal constraint values + // + + let assertConstraints = (item, fontOptions, expectedMaxWidthDefault) => { + if (item.widthConstraint === undefined) { + assert.equal(fontOptions.minWdt, -1); + assert.equal(fontOptions.maxWdt, expectedMaxWidthDefault); + } else { + let constraint = item.widthConstraint; + + // Check shorthand + if (typeof constraint === 'number') { + assert.equal(fontOptions.minWdt, constraint); + assert.equal(fontOptions.maxWdt, constraint); + return; + } + + // check max value + if (constraint.maximum !== undefined) { + assert.equal(fontOptions.maxWdt, constraint.maximum); + } else { + // Check the global default + assert.equal(fontOptions.maxWdt, expectedMaxWidthDefault); + } + + // check min value + if (constraint.minimum !== undefined) { + assert.equal(fontOptions.minWdt, constraint.minimum); + } else { + // Check the global default - there is none defined, so it should be default -1 + assert.equal(fontOptions.minWdt, -1); + } + } + + + if (item.heightConstraint === undefined) { + assert.equal(fontOptions.minHgt, -1); // No value set for this test + assert.equal(fontOptions.valign, 'middle'); + } else { + let constraint = item.heightConstraint; + + // Check shorthand + if (typeof constraint === 'number') { + assert.equal(fontOptions.maxWdt, constraint); + assert.equal(fontOptions.valign, 'middle'); // Default for shorthand + return; + } + + // NOTE: there is no max value for heightConstraint + + // check min value + if (constraint.minimum !== undefined) { + assert.equal(fontOptions.minHgt, constraint.minimum); + } else { + // Check the global default - there is none defined, so it should be default -1 + assert.equal(fontOptions.minHgt, -1); + } + + // check valign value + if (constraint.valign !== undefined) { + assert.equal(fontOptions.valign, constraint.valign); + } else { + // Check the global default - there is none defined so should be 'middle' + assert.equal(fontOptions.valign, 'middle'); + } + + } + + }; + + + // Check nodes + util.forEach(nodes, function(node) { + let networkNode = network.body.nodes[node.id]; + assert(networkNode !== undefined && networkNode !== null); + let fontOptions = networkNode.labelModule.fontOptions; + assertConstraints(node, fontOptions, 200); + }); + + + // Check edges + util.forEach(edges, function(edge) { + // No id's defined, search network edge by from/to + let networkEdge; + let findCount = 0; + util.forEach(network.body.edges, function(inEdge) { + //console.log(inEdge); + if (inEdge.from.id === edge.from && inEdge.to.id === edge.to) { + networkEdge = inEdge; + findCount++; + } + }); + assert(networkEdge !== undefined); + assert(findCount === 1, "Expecting exactly one match for edge search"); + + let fontOptions = networkEdge.labelModule.fontOptions; + + assertConstraints(edge, fontOptions, 90); + }); + + done(); + }); }); From 481a5560fdac6dcd6b9ab5199fe0738814e0a876 Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Thu, 5 Oct 2017 05:31:34 +0200 Subject: [PATCH 2/5] Made unit test more linear, removed tabs --- test/Label.test.js | 137 ++++++++++++++++++--------------------------- 1 file changed, 56 insertions(+), 81 deletions(-) diff --git a/test/Label.test.js b/test/Label.test.js index f8dd1aa85..cfaafd409 100644 --- a/test/Label.test.js +++ b/test/Label.test.js @@ -32,7 +32,7 @@ class DummyContext { class DummyLayoutEngine { - positionInitially() {} + positionInitially() {} } /************************************************************** @@ -72,7 +72,7 @@ describe('Network Label', function() { let showBlocks = () => { return '\nreturned: ' + JSON.stringify(returned, null, 2) + '\n' + 'expected: ' + JSON.stringify(expected, null, 2); - } + } assert.equal(expected.lines.length, returned.lines.length, 'Number of lines does not match, ' + showBlocks()); @@ -121,7 +121,7 @@ describe('Network Label', function() { "OnereallylongwordthatshouldgooverwidthConstraint.maximumifdefined", "One really long sentence that should go over widthConstraint.maximum if defined", "Reallyoneenormouslylargelabel withtwobigwordsgoingoverwayovermax" - ] + ] var html_text = [ "label with some multi tags", @@ -917,7 +917,7 @@ describe('Shorthand Font Options', function() { } } }); - } + } function dynamicAdd2(network, data) { @@ -926,7 +926,7 @@ describe('Shorthand Font Options', function() { font: '7 Font7 #070707' // Note: this kills the font.multi, bold and ital settings! } }); - } + } it('deals with dynamic data and option updates for shorthand', function (done) { @@ -1477,105 +1477,80 @@ describe('Shorthand Font Options', function() { }; var network = new vis.Network(container, data, options); - // - // Check the internal constraint values - // - - let assertConstraints = (item, fontOptions, expectedMaxWidthDefault) => { - if (item.widthConstraint === undefined) { - assert.equal(fontOptions.minWdt, -1); - assert.equal(fontOptions.maxWdt, expectedMaxWidthDefault); - } else { - let constraint = item.widthConstraint; - - // Check shorthand - if (typeof constraint === 'number') { - assert.equal(fontOptions.minWdt, constraint); - assert.equal(fontOptions.maxWdt, constraint); - return; - } - - // check max value - if (constraint.maximum !== undefined) { - assert.equal(fontOptions.maxWdt, constraint.maximum); - } else { - // Check the global default - assert.equal(fontOptions.maxWdt, expectedMaxWidthDefault); - } - - // check min value - if (constraint.minimum !== undefined) { - assert.equal(fontOptions.minWdt, constraint.minimum); - } else { - // Check the global default - there is none defined, so it should be default -1 - assert.equal(fontOptions.minWdt, -1); - } - } - - - if (item.heightConstraint === undefined) { - assert.equal(fontOptions.minHgt, -1); // No value set for this test - assert.equal(fontOptions.valign, 'middle'); - } else { - let constraint = item.heightConstraint; - - // Check shorthand - if (typeof constraint === 'number') { - assert.equal(fontOptions.maxWdt, constraint); - assert.equal(fontOptions.valign, 'middle'); // Default for shorthand - return; - } + var nodes_expected = [ + { nodeId: 100, minWdt: -1, maxWdt: 200, minHgt: -1, valign: 'middle'}, + { nodeId: 210, minWdt: 120, maxWdt: 200, minHgt: -1, valign: 'middle'}, + { nodeId: 211, minWdt: 120, maxWdt: 200, minHgt: -1, valign: 'middle'}, + { nodeId: 220, minWdt: -1, maxWdt: 170, minHgt: -1, valign: 'middle'}, + { nodeId: 221, minWdt: -1, maxWdt: 170, minHgt: -1, valign: 'middle'}, + { nodeId: 200, minWdt: 150, maxWdt: 150, minHgt: -1, valign: 'middle'}, + { nodeId: 201, minWdt: 150, maxWdt: 150, minHgt: -1, valign: 'middle'}, + { nodeId: 300, minWdt: -1, maxWdt: 200, minHgt: 70, valign: 'middle'}, + { nodeId: 301, minWdt: -1, maxWdt: 200, minHgt: 70, valign: 'middle'}, + { nodeId: 400, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'top'}, + { nodeId: 401, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'middle'}, + { nodeId: 402, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'bottom'}, + ]; - // NOTE: there is no max value for heightConstraint - // check min value - if (constraint.minimum !== undefined) { - assert.equal(fontOptions.minHgt, constraint.minimum); - } else { - // Check the global default - there is none defined, so it should be default -1 - assert.equal(fontOptions.minHgt, -1); - } - - // check valign value - if (constraint.valign !== undefined) { - assert.equal(fontOptions.valign, constraint.valign); - } else { - // Check the global default - there is none defined so should be 'middle' - assert.equal(fontOptions.valign, 'middle'); - } + // For edge labels, only maxWdt is set. We check the rest anyway, be it for + // checking incorrect settings or for future code changes. + // + // There is a lot of repetitiveness here. Perhaps using a direct copy of the + // example should be let go. + var edges_expected = [ + { from: 100, to: 210, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 210, to: 211, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 100, to: 220, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 220, to: 221, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 210, to: 200, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 220, to: 200, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 100, to: 300, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 300, to: 301, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 100, to: 400, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 400, to: 401, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { from: 401, to: 402, minWdt: -1, maxWdt: 150, minHgt: -1, valign: 'middle'}, + ]; - } - }; + let assertConstraints = (expected, fontOptions, label) => { + assert.equal(expected.minWdt, fontOptions.minWdt, 'Incorrect min width' + label); + assert.equal(expected.maxWdt, fontOptions.maxWdt, 'Incorrect max width' + label); + assert.equal(expected.minHgt, fontOptions.minHgt, 'Incorrect min height' + label); + assert.equal(expected.valign, fontOptions.valign, 'Incorrect valign' + label); + } // Check nodes - util.forEach(nodes, function(node) { - let networkNode = network.body.nodes[node.id]; + util.forEach(nodes_expected, function(expected) { + let networkNode = network.body.nodes[expected.nodeId]; assert(networkNode !== undefined && networkNode !== null); let fontOptions = networkNode.labelModule.fontOptions; - assertConstraints(node, fontOptions, 200); + + var label = ' for node id: ' + expected.nodeId; + assertConstraints(expected, fontOptions, label); }); // Check edges - util.forEach(edges, function(edge) { + util.forEach(edges_expected, function(expected) { // No id's defined, search network edge by from/to let networkEdge; let findCount = 0; - util.forEach(network.body.edges, function(inEdge) { + util.forEach(network.body.edges, function(edge) { //console.log(inEdge); - if (inEdge.from.id === edge.from && inEdge.to.id === edge.to) { - networkEdge = inEdge; + if (expected.from === edge.from.id && expected.to === edge.to.id) { + networkEdge = edge; findCount++; } }); - assert(networkEdge !== undefined); + + var label = ' for edge from: ' + expected.from + ', to: ' + expected.to; + assert(networkEdge !== undefined, 'Edge not found' + label); assert(findCount === 1, "Expecting exactly one match for edge search"); let fontOptions = networkEdge.labelModule.fontOptions; - - assertConstraints(edge, fontOptions, 90); + assertConstraints(expected, fontOptions, label); }); done(); From f24697893c992464feb712bf3a01baac28a5203d Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Thu, 5 Oct 2017 07:18:17 +0200 Subject: [PATCH 3/5] Made 'enhanced subset' of unit test --- .../modules/components/shared/Label.js | 4 +- test/Label.test.js | 110 +++++++++--------- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/lib/network/modules/components/shared/Label.js b/lib/network/modules/components/shared/Label.js index 2917457f0..1e4002125 100644 --- a/lib/network/modules/components/shared/Label.js +++ b/lib/network/modules/components/shared/Label.js @@ -150,8 +150,8 @@ class Label { * @private */ constrain(pile) { - // Note that of these fields, only 'constrainWidth' and 'maxWdt' - // will be set for edge labels. + // NOTE: constrainWidth and constrainHeight never set! + // NOTE: for edge labels, only 'maxWdt' set // Node labels can set all the fields let fontOptions = { constrainWidth: false, diff --git a/test/Label.test.js b/test/Label.test.js index cfaafd409..81ce9ae6b 100644 --- a/test/Label.test.js +++ b/test/Label.test.js @@ -1410,44 +1410,37 @@ describe('Shorthand Font Options', function() { /** * - * The test network is taken verbatim from example `network/nodeStyles/widthHeight.html`, + * The test network is derived from example `network/nodeStyles/widthHeight.html`, * where the associated issue (i.e. widthConstraint values not copied) was most poignant. * - * If you want to be really pedantically paranoid about it, widthConstraint should also - * be tested when set in groups. perhaps TODO. - * - * perhaps TODO: check boolean shorthand values for widthConstraint. + * NOTE: boolean shorthand values for widthConstraint and heightConstraint do nothing. * perhaps TODO: check shorthand values for heightConstraint. */ it('Sets the width/height constraints in the font label options', function (done) { var nodes = [ - { id: 100, label: 'This node has no fixed, minimum or maximum width or height', x: -50, y: -300 }, - { id: 210, widthConstraint: { minimum: 120 }, label: 'This node has a mimimum width', x: -400, y: -200 }, - { id: 211, widthConstraint: { minimum: 120 }, label: '...so does this', x: -500, y: -50 }, - { id: 220, widthConstraint: { maximum: 170 }, label: 'This node has a maximum width and breaks have been automatically inserted into the label', x: -150, y: -150 }, - { id: 221, widthConstraint: { maximum: 170 }, label: '...this too', x: -100, y: 0 }, - { id: 200, font: { multi: true }, widthConstraint: 150, label: 'This node has a fixed width and breaks have been automatically inserted into the label', x: -300, y: 50 }, - { id: 201, widthConstraint: 150, label: '...this as well', x: -300, y: 200 }, - { id: 300, heightConstraint: { minimum: 70 }, label: 'This node\nhas a\nminimum\nheight', x: 100, y: -150 }, - { id: 301, heightConstraint: { minimum: 70 }, label: '...this one here too', x: 100, y: 0 }, - { id: 400, heightConstraint: { minimum: 100, valign: 'top' }, label: 'Minimum height\nvertical alignment\nmay be top', x: 300, y: -200 }, - { id: 401, heightConstraint: { minimum: 100, valign: 'middle' }, label: 'Minimum height\nvertical alignment\nmay be middle\n(default)', x: 300, y: 0 }, - { id: 402, heightConstraint: { minimum: 100, valign: 'bottom' }, label: 'Minimum height\nvertical alignment\nmay be bottom', x: 300, y: 200 } + { id: 100, label: 'node 100'}, + { id: 210, group: 'group1', label: 'node 210'}, + { id: 211, widthConstraint: { minimum: 120 }, label: 'node 211'}, + { id: 212, widthConstraint: { minimum: 120, maximum: 140 }, group: 'group1', label: 'node 212'}, // group override + { id: 220, widthConstraint: { maximum: 170 }, label: 'node 220'}, + { id: 221, widthConstraint: { maximum: 170 }, label: 'node 221'}, + { id: 200, font: { multi: true }, widthConstraint: 150, label: 'node 200'}, + { id: 201, widthConstraint: 150, label: 'node 201'}, + { id: 202, group: 'group2', label: 'node 202'}, + { id: 203, heightConstraint: { minimum: 75, valign: 'bottom'}, group: 'group2', label: 'node 203'}, // group override + { id: 204, heightConstraint: 80, group: 'group2', label: 'node 204'}, // group override + { id: 300, heightConstraint: { minimum: 70 }, label: 'node 300'}, + { id: 301, heightConstraint: { minimum: 70 }, label: 'node 301'}, + { id: 400, heightConstraint: { minimum: 100, valign: 'top' }, label: 'node 400'}, + { id: 401, heightConstraint: { minimum: 100, valign: 'middle' }, label: 'node 401'}, + { id: 402, heightConstraint: { minimum: 100, valign: 'bottom' }, label: 'node 402'} ]; var edges = [ - { from: 100, to: 210, label: "unconstrained to minimum width"}, - { from: 210, to: 211, label: "more minimum width"}, - { from: 100, to: 220, label: "unconstrained to maximum width"}, - { from: 220, to: 221, label: "more maximum width"}, - { from: 210, to: 200, label: "minimum width to fixed width"}, - { from: 220, to: 200, label: "maximum width to fixed width"}, - { from: 200, to: 201, label: "more fixed width"}, - { from: 100, to: 300, label: "unconstrained to minimum height"}, - { from: 300, to: 301, label: "more minimum height"}, - { from: 100, to: 400, label: "unconstrained to top valign"}, - { from: 400, to: 401, label: "top valign to middle valign"}, - { from: 401, to: 402, widthConstraint: { maximum: 150 }, label: "middle valign to bottom valign"}, + { id: 1, from: 100, to: 210, label: "edge 1"}, + { id: 2, widthConstraint: 80, from: 210, to: 211, label: "edge 2"}, + { id: 3, heightConstraint: 90, from: 100, to: 220, label: "edge 3"}, + { id: 4, from: 401, to: 402, widthConstraint: { maximum: 150 }, label: "edge 12"}, ]; var container = document.getElementById('mynetwork'); @@ -1471,6 +1464,26 @@ describe('Shorthand Font Options', function() { maximum: 200 } }, + groups: { + group1: { + shape: 'dot', + widthConstraint: { + maximum: 130 + } + }, + // Following group serves to test all font options + group2: { + shape: 'dot', + widthConstraint: { + minimum: 150, + maximum: 180, + }, + heightConstraint: { + minimum: 210, + valign: 'top', + } + }, + }, physics: { enabled: false } @@ -1479,12 +1492,16 @@ describe('Shorthand Font Options', function() { var nodes_expected = [ { nodeId: 100, minWdt: -1, maxWdt: 200, minHgt: -1, valign: 'middle'}, - { nodeId: 210, minWdt: 120, maxWdt: 200, minHgt: -1, valign: 'middle'}, + { nodeId: 210, minWdt: -1, maxWdt: 130, minHgt: -1, valign: 'middle'}, { nodeId: 211, minWdt: 120, maxWdt: 200, minHgt: -1, valign: 'middle'}, + { nodeId: 212, minWdt: 120, maxWdt: 140, minHgt: -1, valign: 'middle'}, { nodeId: 220, minWdt: -1, maxWdt: 170, minHgt: -1, valign: 'middle'}, { nodeId: 221, minWdt: -1, maxWdt: 170, minHgt: -1, valign: 'middle'}, { nodeId: 200, minWdt: 150, maxWdt: 150, minHgt: -1, valign: 'middle'}, { nodeId: 201, minWdt: 150, maxWdt: 150, minHgt: -1, valign: 'middle'}, + { nodeId: 202, minWdt: 150, maxWdt: 180, minHgt: 210, valign: 'top'}, + { nodeId: 203, minWdt: 150, maxWdt: 180, minHgt: 75, valign: 'bottom'}, + { nodeId: 204, minWdt: 150, maxWdt: 180, minHgt: 80, valign: 'middle'}, { nodeId: 300, minWdt: -1, maxWdt: 200, minHgt: 70, valign: 'middle'}, { nodeId: 301, minWdt: -1, maxWdt: 200, minHgt: 70, valign: 'middle'}, { nodeId: 400, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'top'}, @@ -1499,17 +1516,10 @@ describe('Shorthand Font Options', function() { // There is a lot of repetitiveness here. Perhaps using a direct copy of the // example should be let go. var edges_expected = [ - { from: 100, to: 210, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 210, to: 211, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 100, to: 220, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 220, to: 221, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 210, to: 200, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 220, to: 200, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 100, to: 300, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 300, to: 301, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 100, to: 400, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 400, to: 401, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, - { from: 401, to: 402, minWdt: -1, maxWdt: 150, minHgt: -1, valign: 'middle'}, + { id: 1, minWdt: -1, maxWdt: 90, minHgt: -1, valign: 'middle'}, + { id: 2, minWdt: 80, maxWdt: 80, minHgt: -1, valign: 'middle'}, + { id: 3, minWdt: -1, maxWdt: 90, minHgt: 90, valign: 'middle'}, + { id: 4, minWdt: -1, maxWdt: 150, minHgt: -1, valign: 'middle'}, ]; @@ -1524,7 +1534,7 @@ describe('Shorthand Font Options', function() { // Check nodes util.forEach(nodes_expected, function(expected) { let networkNode = network.body.nodes[expected.nodeId]; - assert(networkNode !== undefined && networkNode !== null); + assert(networkNode !== undefined && networkNode !== null, 'node not found for id: ' + expected.nodeId); let fontOptions = networkNode.labelModule.fontOptions; var label = ' for node id: ' + expected.nodeId; @@ -1534,20 +1544,10 @@ describe('Shorthand Font Options', function() { // Check edges util.forEach(edges_expected, function(expected) { - // No id's defined, search network edge by from/to - let networkEdge; - let findCount = 0; - util.forEach(network.body.edges, function(edge) { - //console.log(inEdge); - if (expected.from === edge.from.id && expected.to === edge.to.id) { - networkEdge = edge; - findCount++; - } - }); - - var label = ' for edge from: ' + expected.from + ', to: ' + expected.to; + let networkEdge = network.body.edges[expected.id]; + + var label = ' for edge id: ' + expected.id; assert(networkEdge !== undefined, 'Edge not found' + label); - assert(findCount === 1, "Expecting exactly one match for edge search"); let fontOptions = networkEdge.labelModule.fontOptions; assertConstraints(expected, fontOptions, label); From 175e74106735bfad5be68b025220a7ac1bbc91fc Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Thu, 5 Oct 2017 07:19:34 +0200 Subject: [PATCH 4/5] Removed TODO from comment --- test/Label.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Label.test.js b/test/Label.test.js index 81ce9ae6b..b2f449928 100644 --- a/test/Label.test.js +++ b/test/Label.test.js @@ -1414,7 +1414,6 @@ describe('Shorthand Font Options', function() { * where the associated issue (i.e. widthConstraint values not copied) was most poignant. * * NOTE: boolean shorthand values for widthConstraint and heightConstraint do nothing. - * perhaps TODO: check shorthand values for heightConstraint. */ it('Sets the width/height constraints in the font label options', function (done) { var nodes = [ From 4cd7286b030a6c7f6b28858a5904b42916ec4200 Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Thu, 5 Oct 2017 07:22:11 +0200 Subject: [PATCH 5/5] Culled redundant nodes from unit test --- test/Label.test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/Label.test.js b/test/Label.test.js index b2f449928..e92b43c76 100644 --- a/test/Label.test.js +++ b/test/Label.test.js @@ -1422,14 +1422,12 @@ describe('Shorthand Font Options', function() { { id: 211, widthConstraint: { minimum: 120 }, label: 'node 211'}, { id: 212, widthConstraint: { minimum: 120, maximum: 140 }, group: 'group1', label: 'node 212'}, // group override { id: 220, widthConstraint: { maximum: 170 }, label: 'node 220'}, - { id: 221, widthConstraint: { maximum: 170 }, label: 'node 221'}, { id: 200, font: { multi: true }, widthConstraint: 150, label: 'node 200'}, { id: 201, widthConstraint: 150, label: 'node 201'}, { id: 202, group: 'group2', label: 'node 202'}, { id: 203, heightConstraint: { minimum: 75, valign: 'bottom'}, group: 'group2', label: 'node 203'}, // group override { id: 204, heightConstraint: 80, group: 'group2', label: 'node 204'}, // group override { id: 300, heightConstraint: { minimum: 70 }, label: 'node 300'}, - { id: 301, heightConstraint: { minimum: 70 }, label: 'node 301'}, { id: 400, heightConstraint: { minimum: 100, valign: 'top' }, label: 'node 400'}, { id: 401, heightConstraint: { minimum: 100, valign: 'middle' }, label: 'node 401'}, { id: 402, heightConstraint: { minimum: 100, valign: 'bottom' }, label: 'node 402'} @@ -1495,14 +1493,12 @@ describe('Shorthand Font Options', function() { { nodeId: 211, minWdt: 120, maxWdt: 200, minHgt: -1, valign: 'middle'}, { nodeId: 212, minWdt: 120, maxWdt: 140, minHgt: -1, valign: 'middle'}, { nodeId: 220, minWdt: -1, maxWdt: 170, minHgt: -1, valign: 'middle'}, - { nodeId: 221, minWdt: -1, maxWdt: 170, minHgt: -1, valign: 'middle'}, { nodeId: 200, minWdt: 150, maxWdt: 150, minHgt: -1, valign: 'middle'}, { nodeId: 201, minWdt: 150, maxWdt: 150, minHgt: -1, valign: 'middle'}, { nodeId: 202, minWdt: 150, maxWdt: 180, minHgt: 210, valign: 'top'}, { nodeId: 203, minWdt: 150, maxWdt: 180, minHgt: 75, valign: 'bottom'}, { nodeId: 204, minWdt: 150, maxWdt: 180, minHgt: 80, valign: 'middle'}, { nodeId: 300, minWdt: -1, maxWdt: 200, minHgt: 70, valign: 'middle'}, - { nodeId: 301, minWdt: -1, maxWdt: 200, minHgt: 70, valign: 'middle'}, { nodeId: 400, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'top'}, { nodeId: 401, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'middle'}, { nodeId: 402, minWdt: -1, maxWdt: 200, minHgt: 100, valign: 'bottom'},