From d097d6a77a3e5c81924973bc809fbb5f80a8911e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 May 2016 17:24:51 -0400 Subject: [PATCH 1/6] lint --- src/plots/cartesian/axis_defaults.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js index 45f34895978..37014dea277 100644 --- a/src/plots/cartesian/axis_defaults.js +++ b/src/plots/cartesian/axis_defaults.js @@ -148,7 +148,7 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, function setAutoType(ax, data) { // new logic: let people specify any type they want, // only autotype if type is '-' - if(ax.type!=='-') return; + if(ax.type !== '-') return; var id = ax._id, axLetter = id.charAt(0); @@ -163,7 +163,7 @@ function setAutoType(ax, data) { // should always default to a linear axis if(d0.type==='histogram' && axLetter === {v: 'y', h: 'x'}[d0.orientation || 'v']) { - ax.type='linear'; + ax.type = 'linear'; return; } From 92cbd0f6148dac11e29f539efc68c73f7c86192f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 May 2016 17:26:30 -0400 Subject: [PATCH 2/6] rewrite 'categoryorder' and 'categoryarray' defaults step: - trimmed down the logic - make sure that axis.categoryarray is filled in - use containerOut.type instead of containerIn.type (even though they are the same, we should always use the 'full' value of attributes after they get coerced) --- .../cartesian/category_order_defaults.js | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/plots/cartesian/category_order_defaults.js b/src/plots/cartesian/category_order_defaults.js index 39da91ba992..10f0fd20902 100644 --- a/src/plots/cartesian/category_order_defaults.js +++ b/src/plots/cartesian/category_order_defaults.js @@ -8,32 +8,25 @@ 'use strict'; -var layoutAttributes = require('./layout_attributes'); module.exports = function handleCategoryOrderDefaults(containerIn, containerOut, coerce) { + if(containerOut.type !== 'category') return; - if(containerIn.type !== 'category') return; + var arrayIn = containerIn.categoryarray, + orderDefault; - var validCategories = layoutAttributes.categoryorder.values; + var isValidArray = (Array.isArray(arrayIn) && arrayIn.length); - var propercategoryarray = Array.isArray(containerIn.categoryarray) && containerIn.categoryarray.length > 0; + // override default 'categoryorder' value when non-empty array is supplied + if(isValidArray) orderDefault = 'array'; - if(validCategories.indexOf(containerIn.categoryorder) === -1 && propercategoryarray) { + var order = coerce('categoryorder', orderDefault); - // when unspecified or invalid, use the default, unless categoryarray implies 'array' - coerce('categoryorder', 'array'); // promote to 'array' - - } else if(containerIn.categoryorder === 'array' && !propercategoryarray) { - - // when mode is 'array' but no list is given, revert to default - - containerIn.categoryorder = 'trace'; // revert to default - coerce('categoryorder'); - - } else { - - // otherwise use the supplied mode, or the default one if unsupplied or invalid - coerce('categoryorder'); + // coerce 'categoryarray' only in array order case + if(order === 'array') coerce('categoryarray'); + // cannot set 'categoryorder' to 'array' with an invalid 'categoryarray' + if(!isValidArray && order === 'array') { + containerOut.categoryorder = 'trace'; } }; From 05715bd1257a4991e7508978bbcf8993c80b2ace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 May 2016 17:26:58 -0400 Subject: [PATCH 3/6] add checks for xaxis.categoryarray in category defaults tests --- test/jasmine/tests/axes_test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index bc61b5603ed..8efbfad68fd 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -399,22 +399,27 @@ describe('Test axes', function() { it('should set categoryorder to default if categoryorder and categoryarray are not supplied', function() { PlotlyInternal.plot(gd, [{x: ['c','a','e','b','d'], y: [15,11,12,13,14]}], {xaxis: {type: 'category'}}); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categorarray).toBe(undefined); }); it('should set categoryorder to default even if type is not set to category explicitly', function() { PlotlyInternal.plot(gd, [{x: ['c','a','e','b','d'], y: [15,11,12,13,14]}]); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categorarray).toBe(undefined); }); it('should NOT set categoryorder to default if type is not category', function() { PlotlyInternal.plot(gd, [{x: ['c','a','e','b','d'], y: [15,11,12,13,14]}]); expect(gd._fullLayout.yaxis.categoryorder).toBe(undefined); + expect(gd._fullLayout.xaxis.categorarray).toBe(undefined); }); it('should set categoryorder to default if type is overridden to be category', function() { PlotlyInternal.plot(gd, [{x: [1,2,3,4,5], y: [15,11,12,13,14]}], {yaxis: {type: 'category'}}); expect(gd._fullLayout.xaxis.categoryorder).toBe(undefined); + expect(gd._fullLayout.yaxis.categorarray).toBe(undefined); expect(gd._fullLayout.yaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.yaxis.categorarray).toBe(undefined); }); }); @@ -426,6 +431,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'array', categoryarray: ['b','a','d','e','c']} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('array'); + expect(gd._fullLayout.xaxis.categoryarray).toEqual(['b','a','d','e','c']); }); it('should switch categoryorder on "array" if it is not supplied but categoryarray is supplied', function() { @@ -433,6 +439,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryarray: ['b','a','d','e','c']} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('array'); + expect(gd._fullLayout.xaxis.categoryarray).toEqual(['b','a','d','e','c']); }); it('should revert categoryorder to "trace" if "array" is supplied but there is no list', function() { @@ -440,6 +447,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'array'} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categorarray).toBe(undefined); }); }); @@ -451,6 +459,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'array', categoryarray: []} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categoryarray).toEqual([]); }); it('should not switch categoryorder on "array" if categoryarray is supplied but empty', function() { @@ -458,6 +467,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryarray: []} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categoryarray).toEqual(undefined); }); }); @@ -468,6 +478,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'trace', categoryarray: ['b','a','d','e','c']} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categoryarray).toBe(undefined); }); it('should use specified categoryorder if it is supplied even if categoryarray exists', function() { @@ -475,6 +486,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'category ascending', categoryarray: ['b','a','d','e','c']} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('category ascending'); + expect(gd._fullLayout.xaxis.categoryarray).toBe(undefined); }); it('should use specified categoryorder if it is supplied even if categoryarray exists', function() { @@ -482,6 +494,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'category descending', categoryarray: ['b','a','d','e','c']} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('category descending'); + expect(gd._fullLayout.xaxis.categoryarray).toBe(undefined); }); }); @@ -493,6 +506,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'invalid value'} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('trace'); + expect(gd._fullLayout.xaxis.categoryarray).toBe(undefined); }); it('should switch categoryorder to "array" if mode is supplied but invalid and list is supplied', function() { @@ -500,6 +514,7 @@ describe('Test axes', function() { xaxis: {type: 'category', categoryorder: 'invalid value', categoryarray: ['b','a','d','e','c']} }); expect(gd._fullLayout.xaxis.categoryorder).toBe('array'); + expect(gd._fullLayout.xaxis.categoryarray).toEqual(['b','a','d','e','c']); }); }); From 460e53a42450b8554ffa5c0046bfa5213ce32d2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 May 2016 17:30:47 -0400 Subject: [PATCH 4/6] use specs in fullLayout to compute the ordered categories, - part one in fixing the category relayout bug - by using the fullLayout (i.e. containerOur) specs, we ensure that ordered categories algo uses smartly-defaulted attributes. --- src/plots/cartesian/axis_defaults.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js index 37014dea277..14364cbfdb3 100644 --- a/src/plots/cartesian/axis_defaults.js +++ b/src/plots/cartesian/axis_defaults.js @@ -74,10 +74,6 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, } } - containerOut._initialCategories = axType === 'category' ? - orderedCategories(letter, containerIn.categoryorder, containerIn.categoryarray, options.data) : - []; - setConvert(containerOut); var dfltColor = coerce('color'); @@ -142,6 +138,11 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, delete containerOut.zerolinewidth; } + // fill in categories + containerOut._initialCategories = axType === 'category' ? + orderedCategories(letter, containerOut.categoryorder, containerOut.categoryarray, options.data) : + []; + return containerOut; }; From b1892ce5a605b7c1a26fb021e785bcf9cb0cc776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 May 2016 17:32:44 -0400 Subject: [PATCH 5/6] make 'categoryorder' / 'categoryarray' relayouts recompute calcdata: - part two in fixing the category relayout bug. - update the axis categories require a re-computation of the graphs calcdata. --- src/plot_api/plot_api.js | 6 ++++ test/jasmine/tests/cartesian_test.js | 54 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index dc6940341e0..2611911fecc 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2211,6 +2211,12 @@ Plotly.relayout = function relayout(gd, astr, val) { else if(/[xy]axis[0-9]*?$/.test(pleaf) && !Object.keys(vi || {}).length) { docalc = true; } + else if(/[xy]axis[0-9]*\.categoryorder$/.test(pleafPlus)) { + docalc = true; + } + else if(/[xy]axis[0-9]*\.categoryarray/.test(pleafPlus)) { + docalc = true; + } if(pleafPlus.indexOf('rangeslider') !== -1) { docalc = true; diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js index 38c09608cef..7fbe66fbef5 100644 --- a/test/jasmine/tests/cartesian_test.js +++ b/test/jasmine/tests/cartesian_test.js @@ -49,3 +49,57 @@ describe('zoom box element', function() { .toEqual(0); }); }); + +describe('relayout', function() { + + describe('axis category attributes', function() { + var mock = require('@mocks/basic_bar.json'); + + var gd, mockCopy; + + beforeEach(function() { + mockCopy = Lib.extendDeep({}, mock); + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('should response to \'categoryarray\' and \'categoryorder\' updates', function(done) { + function assertCategories(list) { + d3.selectAll('g.xtick').each(function(_, i) { + var tick = d3.select(this).select('text'); + expect(tick.html()).toEqual(list[i]); + }); + } + + Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { + assertCategories(['giraffes', 'orangutans', 'monkeys']); + + return Plotly.relayout(gd, 'xaxis.categoryorder', 'category descending'); + }).then(function() { + var list = ['orangutans', 'monkeys', 'giraffes']; + + expect(gd._fullLayout.xaxis._initialCategories).toEqual(list); + assertCategories(list); + + return Plotly.relayout(gd, 'xaxis.categoryorder', null); + }).then(function() { + assertCategories(['giraffes', 'orangutans', 'monkeys']); + + return Plotly.relayout(gd, { + 'xaxis.categoryarray': ['monkeys', 'giraffes', 'orangutans'] + }); + }).then(function() { + var list = ['monkeys', 'giraffes', 'orangutans']; + + expect(gd.layout.xaxis.categoryarray).toEqual(list); + expect(gd._fullLayout.xaxis.categoryarray).toEqual(list); + expect(gd._fullLayout.xaxis._initialCategories).toEqual(list); + assertCategories(list); + + done(); + }); + }); + }); + +}); From c6075cbcdd1d24368752644b0d92c756ae2d7ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 5 May 2016 11:30:53 -0400 Subject: [PATCH 6/6] sub .length for .length > 0 for clarity --- src/plots/cartesian/category_order_defaults.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/cartesian/category_order_defaults.js b/src/plots/cartesian/category_order_defaults.js index 10f0fd20902..0e711360d3b 100644 --- a/src/plots/cartesian/category_order_defaults.js +++ b/src/plots/cartesian/category_order_defaults.js @@ -15,7 +15,7 @@ module.exports = function handleCategoryOrderDefaults(containerIn, containerOut, var arrayIn = containerIn.categoryarray, orderDefault; - var isValidArray = (Array.isArray(arrayIn) && arrayIn.length); + var isValidArray = (Array.isArray(arrayIn) && arrayIn.length > 0); // override default 'categoryorder' value when non-empty array is supplied if(isValidArray) orderDefault = 'array';