From aeef7f724ea727d6288f586fc3e3062286fe8a0c Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 6 Sep 2018 18:47:36 -0400 Subject: [PATCH 1/3] fix layer ordering of scatter subtypes --- src/components/legend/style.js | 4 +- src/traces/scatter/plot.js | 125 +++++++++++------------ src/traces/scatter/style.js | 10 ++ test/jasmine/assets/custom_assertions.js | 22 ++++ test/jasmine/tests/cartesian_test.js | 40 ++++---- test/jasmine/tests/scatter_test.js | 93 +++++++++++++++++ test/jasmine/tests/select_test.js | 10 +- 7 files changed, 209 insertions(+), 95 deletions(-) diff --git a/src/components/legend/style.js b/src/components/legend/style.js index fd68381fcdd..21afb93c07f 100644 --- a/src/components/legend/style.js +++ b/src/components/legend/style.js @@ -208,7 +208,9 @@ module.exports = function style(s, gd) { var pts = ptgroup.selectAll('path.scatterpts') .data(showMarkers ? dMod : []); - pts.enter().append('path').classed('scatterpts', true) + // make sure marker is on the bottom, in case it enters after text + pts.enter().insert('path', ':first-child') + .classed('scatterpts', true) .attr('transform', 'translate(20,0)'); pts.exit().remove(); pts.call(Drawing.pointStyle, tMod, gd); diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index b0e14b2964a..7b029ce8495 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -13,6 +13,8 @@ var d3 = require('d3'); var Registry = require('../../registry'); var Lib = require('../../lib'); +var ensureSingle = Lib.ensureSingle; +var identity = Lib.identity; var Drawing = require('../../components/drawing'); var subTypes = require('./subtypes'); @@ -43,7 +45,7 @@ module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transition // the z-order of fill layers is correct. linkTraces(gd, plotinfo, cdscatter); - createFills(gd, scatterLayer, plotinfo); + createFills(gd, join, plotinfo); // Sort the traces, once created, so that the ordering is preserved even when traces // are shown and hidden. This is needed since we're not just wiping everything out @@ -52,10 +54,10 @@ module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transition uids[cdscatter[i][0].trace.uid] = i; } - scatterLayer.selectAll('g.trace').sort(function(a, b) { + join.sort(function(a, b) { var idx1 = uids[a[0].trace.uid]; var idx2 = uids[b[0].trace.uid]; - return idx1 > idx2 ? 1 : -1; + return idx1 - idx2; }); if(hasTransition) { @@ -97,51 +99,45 @@ module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transition scatterLayer.selectAll('path:not([d])').remove(); }; -function createFills(gd, scatterLayer, plotinfo) { - var trace; +function createFills(gd, traceJoin, plotinfo) { + traceJoin.each(function(d) { + var fills = ensureSingle(d3.select(this), 'g', 'fills'); + Drawing.setClipUrl(fills, plotinfo.layerClipId); - scatterLayer.selectAll('g.trace').each(function(d) { - var tr = d3.select(this); - - // Loop only over the traces being redrawn: - trace = d[0].trace; + var trace = d[0].trace; - // make the fill-to-next path now for the NEXT trace, so it shows - // behind both lines. + var fillData = []; + if(trace.fill && (trace.fill.substr(0, 6) === 'tozero' || trace.fill === 'toself' || + (trace.fill.substr(0, 2) === 'to' && !trace._prevtrace)) + ) { + fillData = ['_ownFill']; + } if(trace._nexttrace) { - trace._nextFill = tr.select('.js-fill.js-tonext'); - if(!trace._nextFill.size()) { + // make the fill-to-next path now for the NEXT trace, so it shows + // behind both lines. + fillData.push('_nextFill'); + } - // If there is an existing tozero fill, we must insert this *after* that fill: - var loc = ':first-child'; - if(tr.select('.js-fill.js-tozero').size()) { - loc += ' + *'; - } + var fillJoin = fills.selectAll('g') + .data(fillData, identity); - trace._nextFill = tr.insert('path', loc).attr('class', 'js-fill js-tonext'); - } - } else { - tr.selectAll('.js-fill.js-tonext').remove(); - trace._nextFill = null; - } + fillJoin.enter().append('g'); - if(trace.fill && (trace.fill.substr(0, 6) === 'tozero' || trace.fill === 'toself' || - (trace.fill.substr(0, 2) === 'to' && !trace._prevtrace))) { - trace._ownFill = tr.select('.js-fill.js-tozero'); - if(!trace._ownFill.size()) { - trace._ownFill = tr.insert('path', ':first-child').attr('class', 'js-fill js-tozero'); - } - } else { - tr.selectAll('.js-fill.js-tozero').remove(); - trace._ownFill = null; - } + fillJoin.exit() + .each(function(d) { trace[d] = null; }) + .remove(); - tr.selectAll('.js-fill').call(Drawing.setClipUrl, plotinfo.layerClipId); + fillJoin.order().each(function(d) { + // make a path element inside the fill group, just so + // we can give it its own data later on and the group can + // keep its simple '_*Fill' data + trace[d] = ensureSingle(d3.select(this), 'path', 'js-fill'); + }); }); } function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transitionOpts) { - var join, i; + var i; // Since this has been reorganized and we're executing this on individual traces, // we need to pass it the full list of cdscatter as well as this trace's index (idx) @@ -157,12 +153,17 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition var xa = plotinfo.xaxis, ya = plotinfo.yaxis; - var trace = cdscatter[0].trace, - line = trace.line, - tr = d3.select(element); + var trace = cdscatter[0].trace; + var line = trace.line; + var tr = d3.select(element); + + var errorBarGroup = ensureSingle(tr, 'g', 'errorbars'); + var lines = ensureSingle(tr, 'g', 'lines'); + var points = ensureSingle(tr, 'g', 'points'); + var text = ensureSingle(tr, 'g', 'text'); // error bars are at the bottom - Registry.getComponentMethod('errorbars', 'plot')(tr, plotinfo, transitionOpts); + Registry.getComponentMethod('errorbars', 'plot')(errorBarGroup, plotinfo, transitionOpts); if(trace.visible !== true) return; @@ -303,7 +304,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition }; } - var lineJoin = tr.selectAll('.js-line').data(segments); + var lineJoin = lines.selectAll('.js-line').data(segments); transition(lineJoin.exit()) .style('opacity', 0) @@ -325,6 +326,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition if(segments.length) { if(ownFillEl3) { + ownFillEl3.datum(cdscatter); if(pt0 && pt1) { if(ownFillDir) { if(ownFillDir === 'y') { @@ -404,11 +406,10 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition return false; } - function makePoints(d) { + function makePoints(points, text, cdscatter) { var join, selection, hasNode; - var trace = d[0].trace; - var s = d3.select(this); + var trace = cdscatter[0].trace; var showMarkers = subTypes.hasMarkers(trace); var showText = subTypes.hasText(trace); @@ -417,16 +418,16 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition var textFilter = hideFilter; if(showMarkers) { - markerFilter = (trace.marker.maxdisplayed || trace._needsCull) ? visFilter : Lib.identity; + markerFilter = (trace.marker.maxdisplayed || trace._needsCull) ? visFilter : identity; } if(showText) { - textFilter = (trace.marker.maxdisplayed || trace._needsCull) ? visFilter : Lib.identity; + textFilter = (trace.marker.maxdisplayed || trace._needsCull) ? visFilter : identity; } // marker points - selection = s.selectAll('path.point'); + selection = points.selectAll('path.point'); join = selection.data(markerFilter, keyFunc); @@ -478,7 +479,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition } // text points - selection = s.selectAll('g'); + selection = text.selectAll('g'); join = selection.data(textFilter, keyFunc); // each text needs to go in its own 'g' in case @@ -517,28 +518,16 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition join.exit().remove(); } - // NB: selectAll is evaluated on instantiation: - var pointSelection = tr.selectAll('.points'); - - // Join with new data - join = pointSelection.data([cdscatter]); - - // Transition existing, but don't defer this to an async .transition since - // there's no timing involved: - pointSelection.each(makePoints); - - join.enter().append('g') - .classed('points', true) - .each(makePoints); - - join.exit().remove(); + points.datum(cdscatter); + text.datum(cdscatter); + makePoints(points, text, cdscatter); // lastly, clip points groups of `cliponaxis !== false` traces // on `plotinfo._hasClipOnAxisFalse === true` subplots - join.each(function(d) { - var hasClipOnAxisFalse = d[0].trace.cliponaxis === false; - Drawing.setClipUrl(d3.select(this), hasClipOnAxisFalse ? null : plotinfo.layerClipId); - }); + var hasClipOnAxisFalse = trace.cliponaxis === false; + var clipUrl = hasClipOnAxisFalse ? null : plotinfo.layerClipId; + Drawing.setClipUrl(points, clipUrl); + Drawing.setClipUrl(text, clipUrl); } function selectMarkers(gd, idx, plotinfo, cdscatter, cdscatterAll) { diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js index b9090261e93..79d82c12fb0 100644 --- a/src/traces/scatter/style.js +++ b/src/traces/scatter/style.js @@ -26,6 +26,12 @@ function style(gd, cd) { stylePoints(sel, trace, gd); }); + s.selectAll('g.text').each(function(d) { + var sel = d3.select(this); + var trace = d.trace || d[0].trace; + styleText(sel, trace, gd); + }); + s.selectAll('g.trace path.js-line') .call(Drawing.lineGroupStyle); @@ -37,6 +43,9 @@ function style(gd, cd) { function stylePoints(sel, trace, gd) { Drawing.pointStyle(sel.selectAll('path.point'), trace, gd); +} + +function styleText(sel, trace, gd) { Drawing.textPointStyle(sel.selectAll('text'), trace, gd); } @@ -49,6 +58,7 @@ function styleOnSelect(gd, cd) { Drawing.selectedTextStyle(s.selectAll('text'), trace); } else { stylePoints(s, trace, gd); + styleText(s, trace, gd); } } diff --git a/test/jasmine/assets/custom_assertions.js b/test/jasmine/assets/custom_assertions.js index e288128f303..b220b479498 100644 --- a/test/jasmine/assets/custom_assertions.js +++ b/test/jasmine/assets/custom_assertions.js @@ -298,6 +298,22 @@ exports.assertNodeOrder = function(selectorBehind, selectorInFront, msg) { } }; +/** + * Ordering test for any number of nodes - calls assertNodeOrder n-1 times. + * Note that we only take the first matching node for each selector, and it's + * not necessary that the nodes be siblings or at the same level of nesting. + * + * @param {Array[string]} selectorArray: css selectors in the order they should + * appear in the document, from back to front. + * @param {string} msg: context for debugging + */ +exports.assertMultiNodeOrder = function(selectorArray, msg) { + for(var i = 0; i < selectorArray.length - 1; i++) { + var msgi = (msg ? msg + ' - ' : '') + 'entries ' + i + ' and ' + (i + 1); + exports.assertNodeOrder(selectorArray[i], selectorArray[i + 1], msgi); + } +}; + function getParents(node) { var parent = node.parentNode; if(parent) return getParents(parent).concat(node); @@ -310,3 +326,9 @@ function collectionToArray(collection) { for(var i = 0; i < len; i++) a[i] = collection[i]; return a; } + +exports.assertD3Data = function(selection, expectedData) { + var data = []; + selection.each(function(d) { data.push(d); }); + expect(data).toEqual(expectedData); +}; diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js index 21a018d7ff6..6f5c26b203b 100644 --- a/test/jasmine/tests/cartesian_test.js +++ b/test/jasmine/tests/cartesian_test.js @@ -7,6 +7,7 @@ var Drawing = require('@src/components/drawing'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var failTest = require('../assets/fail_test'); +var assertD3Data = require('../assets/custom_assertions').assertD3Data; describe('restyle', function() { describe('scatter traces', function() { @@ -21,37 +22,35 @@ describe('restyle', function() { it('reuses SVG fills', function(done) { var fills, firstToZero, secondToZero, firstToNext, secondToNext; var mock = Lib.extendDeep({}, require('@mocks/basic_area.json')); + function getFills() { + return d3.selectAll('g.trace.scatter .fills>g'); + } Plotly.plot(gd, mock.data, mock.layout).then(function() { - // Assert there are two fills: - fills = d3.selectAll('g.trace.scatter .js-fill')[0]; + fills = getFills(); - // First is tozero, second is tonext: - expect(d3.selectAll('g.trace.scatter .js-fill').size()).toEqual(2); - expect(fills[0]).toBeClassed(['js-fill', 'js-tozero']); - expect(fills[1]).toBeClassed(['js-fill', 'js-tonext']); + // Assert there are two fills, first is tozero, second is tonext + assertD3Data(fills, ['_ownFill', '_nextFill']); + + firstToZero = fills[0][0]; + firstToNext = fills[0][1]; - firstToZero = fills[0]; - firstToNext = fills[1]; - }).then(function() { return Plotly.restyle(gd, {visible: [false]}, [1]); }).then(function() { + fills = getFills(); // Trace 1 hidden leaves only trace zero's tozero fill: - expect(d3.selectAll('g.trace.scatter .js-fill').size()).toEqual(1); - expect(fills[0]).toBeClassed(['js-fill', 'js-tozero']); - }).then(function() { + assertD3Data(fills, ['_ownFill']); + return Plotly.restyle(gd, {visible: [true]}, [1]); }).then(function() { - // Reshow means two fills again AND order is preserved: - fills = d3.selectAll('g.trace.scatter .js-fill')[0]; + fills = getFills(); + // Reshow means two fills again AND order is preserved // First is tozero, second is tonext: - expect(d3.selectAll('g.trace.scatter .js-fill').size()).toEqual(2); - expect(fills[0]).toBeClassed(['js-fill', 'js-tozero']); - expect(fills[1]).toBeClassed(['js-fill', 'js-tonext']); + assertD3Data(fills, ['_ownFill', '_nextFill']); - secondToZero = fills[0]; - secondToNext = fills[1]; + secondToZero = fills[0][0]; + secondToNext = fills[0][1]; // The identity of the first is retained: expect(firstToZero).toBe(secondToZero); @@ -61,8 +60,7 @@ describe('restyle', function() { return Plotly.restyle(gd, 'visible', false); }).then(function() { - expect(d3.selectAll('g.trace.scatter').size()).toEqual(0); - + expect(d3.selectAll('g.trace.scatter').size()).toBe(0); }) .catch(failTest) .then(done); diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index dffed81956d..58b05d89a80 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -12,6 +12,7 @@ var failTest = require('../assets/fail_test'); var assertClip = customAssertions.assertClip; var assertNodeDisplay = customAssertions.assertNodeDisplay; +var assertMultiNodeOrder = customAssertions.assertMultiNodeOrder; var getOpacity = function(node) { return Number(node.style.opacity); }; var getFillOpacity = function(node) { return Number(node.style['fill-opacity']); }; @@ -629,6 +630,98 @@ describe('end-to-end scatter tests', function() { .then(done); }); + it('should keep layering correct as mode & fill change', function(done) { + var fillCase = {name: 'fill', edit: {mode: 'none', fill: 'tonexty'}}; + var i, j; + + var cases = [fillCase]; + var modeParts = ['lines', 'markers', 'text']; + for(i = 0; i < modeParts.length; i++) { + var modePart = modeParts[i]; + var prevCasesLength = cases.length; + + cases.push({name: modePart, edit: {mode: modePart, fill: 'none'}}); + for(j = 0; j < prevCasesLength; j++) { + var prevCase = cases[j]; + cases.push({ + name: prevCase.name + '_' + modePart, + edit: { + mode: (prevCase.edit.mode === 'none' ? '' : (prevCase.edit.mode + '+')) + modePart, + fill: prevCase.edit.fill + } + }); + } + } + + // visit each case N times, in an order that covers each *transition* + // from any case to any other case. + var indices = []; + var curIndex = 0; + for(i = 1; i < cases.length; i++) { + for(j = 0; j < cases.length; j++) { + indices.push(curIndex); + curIndex = (curIndex + i) % cases.length; + } + } + + var p = Plotly.plot(gd, [ + {y: [1, 2], text: 'a'}, + {y: [2, 3], text: 'b'}, + {y: [3, 4], text: 'c'} + ]); + + function setMode(i) { return function() { + return Plotly.restyle(gd, cases[indices[i]].edit); + }; } + + function testOrdering(i) { return function() { + var name = cases[indices[i]].name; + var hasFills = name.indexOf('fill') !== -1; + var hasLines = name.indexOf('lines') !== -1; + var hasMarkers = name.indexOf('markers') !== -1; + var hasText = name.indexOf('text') !== -1; + var tracei, prefix; + + // construct the expected ordering based on case name + var selectorArray = []; + for(tracei = 0; tracei < 3; tracei++) { + prefix = '.xy .trace:nth-child(' + (tracei + 1) + ') '; + + // two fills are attached to the first trace, one to the second + if(hasFills) { + if(tracei === 0) { + selectorArray.push( + prefix + 'g:first-child>.js-fill', + prefix + 'g:last-child>.js-fill'); + } + else if(tracei === 1) selectorArray.push(prefix + 'g:last-child>.js-fill'); + } + if(hasLines) selectorArray.push(prefix + '.js-line'); + if(hasMarkers) selectorArray.push(prefix + '.point'); + if(hasText) selectorArray.push(prefix + '.textpoint'); + } + + // ordering in the legend + for(tracei = 0; tracei < 3; tracei++) { + prefix = '.legend .traces:nth-child(' + (tracei + 1) + ') '; + if(hasFills) selectorArray.push(prefix + '.js-fill'); + if(hasLines) selectorArray.push(prefix + '.js-line'); + if(hasMarkers) selectorArray.push(prefix + '.scatterpts'); + if(hasText) selectorArray.push(prefix + '.pointtext'); + } + + var msg = i ? ('from ' + cases[indices[i - 1]].name + ' to ') : 'from default to '; + msg += name; + assertMultiNodeOrder(selectorArray, msg); + }; } + + for(i = 0; i < indices.length; i++) { + p = p.then(setMode(i)).then(testOrdering(i)); + } + + p.catch(failTest).then(done); + }); + function _assertNodes(ptStyle, txContent) { var pts = d3.selectAll('.point'); var txs = d3.selectAll('.textpoint'); diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 821c7380e96..b637e200ff6 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -1833,14 +1833,14 @@ describe('Test select box and lasso per trace:', function() { it('@flaky should work on scatter/bar traces with text nodes', function(done) { var assertSelectedPoints = makeAssertSelectedPoints(); - function assertFillOpacity(exp) { + function assertFillOpacity(exp, msg) { var txtPts = d3.select(gd).select('g.plot').selectAll('text'); - expect(txtPts.size()).toBe(exp.length, '# of text nodes'); + expect(txtPts.size()).toBe(exp.length, '# of text nodes: ' + msg); txtPts.each(function(_, i) { var act = Number(this.style['fill-opacity']); - expect(act).toBe(exp[i], 'node ' + i + ' fill opacity'); + expect(act).toBe(exp[i], 'node ' + i + ' fill opacity: ' + msg); }); } @@ -1867,13 +1867,13 @@ describe('Test select box and lasso per trace:', function() { [[10, 10], [100, 300]], function() { assertSelectedPoints({0: [0], 1: [0]}); - assertFillOpacity([1, 0.2, 0.2, 1, 0.2, 0.2]); + assertFillOpacity([1, 0.2, 0.2, 1, 0.2, 0.2], '_run'); }, null, BOXEVENTS, 'selecting first scatter/bar text nodes' ); }) .then(function() { - assertFillOpacity([1, 1, 1, 1, 1, 1]); + assertFillOpacity([1, 1, 1, 1, 1, 1], 'final'); }) .catch(failTest) .then(done); From 7c54fc2f0bd38ecac52c314b736e86ef035c3511 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 6 Sep 2018 20:47:55 -0400 Subject: [PATCH 2/3] put back text styling in scattergeo --- src/traces/scatter/style.js | 1 + src/traces/scattergeo/style.js | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js index 79d82c12fb0..4ce72bdf388 100644 --- a/src/traces/scatter/style.js +++ b/src/traces/scatter/style.js @@ -65,5 +65,6 @@ function styleOnSelect(gd, cd) { module.exports = { style: style, stylePoints: stylePoints, + styleText: styleText, styleOnSelect: styleOnSelect }; diff --git a/src/traces/scattergeo/style.js b/src/traces/scattergeo/style.js index c4d9a644321..6d89407e62f 100644 --- a/src/traces/scattergeo/style.js +++ b/src/traces/scattergeo/style.js @@ -12,7 +12,9 @@ var d3 = require('d3'); var Drawing = require('../../components/drawing'); var Color = require('../../components/color'); -var stylePoints = require('../scatter/style').stylePoints; +var scatterStyle = require('../scatter/style'); +var stylePoints = scatterStyle.stylePoints; +var styleText = scatterStyle.styleText; module.exports = function style(gd, calcTrace) { if(calcTrace) styleTrace(gd, calcTrace); @@ -25,6 +27,7 @@ function styleTrace(gd, calcTrace) { s.style('opacity', calcTrace[0].trace.opacity); stylePoints(s, trace, gd); + styleText(s, trace, gd); // this part is incompatible with Drawing.lineGroupStyle s.selectAll('path.js-line') From 69888187c9de68a3d8a6d75cd1bb84e6c0c66f1a Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 11 Sep 2018 00:59:05 -0400 Subject: [PATCH 3/3] better algorithm for covering all state transitions --- test/jasmine/assets/transitions.js | 35 ++++++++++++++++++++++++++++++ test/jasmine/tests/scatter_test.js | 10 ++------- 2 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 test/jasmine/assets/transitions.js diff --git a/test/jasmine/assets/transitions.js b/test/jasmine/assets/transitions.js new file mode 100644 index 00000000000..cf1be579d51 --- /dev/null +++ b/test/jasmine/assets/transitions.js @@ -0,0 +1,35 @@ +'use strict'; + +/** + * Given n states (denoted by their indices 0..n-1) this routine produces + * a sequence of indices such that you efficiently execute each transition + * from any state to any other state. + */ +module.exports = function transitions(n) { + var out = [0]; + var nextStates = []; + var i; + for(i = 0; i < n; i++) nextStates[i] = (i + 1) % n; + var finishedStates = 0; + var thisState = 0; + var nextState; + while(finishedStates < n) { + nextState = nextStates[thisState]; + if(nextState === thisState) { + // I don't actually know how to prove that this algorithm works, + // but I've never seen it fail for n>1 + // For prime n it's the same sequence as the one I started with + // (n transitions of +1 index, then n transitions +2 etc...) + // but this one works for non-prime n as well. + throw new Error('your transitions algo failed.'); + } + nextStates[thisState] = (nextStates[thisState] + 1) % n; + if(nextStates[thisState] === thisState) finishedStates++; + out.push(nextState); + thisState = nextState; + } + if(out.length !== n * (n - 1) + 1) { + throw new Error('your transitions algo failed.'); + } + return out; +}; diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index b24214b64ad..a49b8a69c3e 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -9,6 +9,7 @@ var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var customAssertions = require('../assets/custom_assertions'); var failTest = require('../assets/fail_test'); +var transitions = require('../assets/transitions'); var assertClip = customAssertions.assertClip; var assertNodeDisplay = customAssertions.assertNodeDisplay; @@ -655,14 +656,7 @@ describe('end-to-end scatter tests', function() { // visit each case N times, in an order that covers each *transition* // from any case to any other case. - var indices = []; - var curIndex = 0; - for(i = 1; i < cases.length; i++) { - for(j = 0; j < cases.length; j++) { - indices.push(curIndex); - curIndex = (curIndex + i) % cases.length; - } - } + var indices = transitions(cases.length); var p = Plotly.plot(gd, [ {y: [1, 2], text: 'a'},