From 3dee25c4e1e1f35bfaab06554bec434a23f81348 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 20 Jun 2018 21:38:39 -0400 Subject: [PATCH] use handleArrayContainerDefaults for various array containers: rangeselector.buttons updatemenus.buttons parcoords & splom.dimensions (and DRY these a bit) --- src/components/rangeselector/defaults.js | 46 +++++------ src/components/updatemenus/defaults.js | 36 +++------ src/traces/parcoords/defaults.js | 93 +++++++++-------------- src/traces/parcoords/merge_length.js | 36 +++++++++ src/traces/splom/defaults.js | 54 ++++--------- test/jasmine/tests/parcoords_test.js | 16 ++-- test/jasmine/tests/range_selector_test.js | 12 +-- 7 files changed, 134 insertions(+), 159 deletions(-) create mode 100644 src/traces/parcoords/merge_length.js diff --git a/src/components/rangeselector/defaults.js b/src/components/rangeselector/defaults.js index 1e3624f2870..0110ec05ae6 100644 --- a/src/components/rangeselector/defaults.js +++ b/src/components/rangeselector/defaults.js @@ -11,6 +11,7 @@ var Lib = require('../../lib'); var Color = require('../color'); var Template = require('../../plot_api/plot_template'); +var handleArrayContainerDefaults = require('../../plots/array_container_defaults'); var attributes = require('./attributes'); var buttonAttrs = require('./button_attributes'); @@ -25,7 +26,11 @@ module.exports = function handleDefaults(containerIn, containerOut, layout, coun return Lib.coerce(selectorIn, selectorOut, attributes, attr, dflt); } - var buttons = buttonsDefaults(selectorIn, selectorOut, calendar); + var buttons = handleArrayContainerDefaults(selectorIn, selectorOut, { + name: 'buttons', + handleItemDefaults: buttonDefaults, + calendar: calendar + }); var visible = coerce('visible', buttons.length > 0); if(!visible) return; @@ -46,43 +51,30 @@ module.exports = function handleDefaults(containerIn, containerOut, layout, coun coerce('borderwidth'); }; -function buttonsDefaults(containerIn, containerOut, calendar) { - var buttonsIn = containerIn.buttons || [], - buttonsOut = containerOut.buttons = []; - - var buttonIn, buttonOut; +function buttonDefaults(buttonIn, buttonOut, selectorOut, opts, itemOpts) { + var calendar = opts.calendar; function coerce(attr, dflt) { return Lib.coerce(buttonIn, buttonOut, buttonAttrs, attr, dflt); } - for(var i = 0; i < buttonsIn.length; i++) { - buttonIn = buttonsIn[i]; - buttonOut = {}; - - var visible = coerce('visible', Lib.isPlainObject(buttonIn)); + var visible = coerce('visible', !itemOpts.itemIsNotPlainObject); - if(visible) { - var step = coerce('step'); - if(step !== 'all') { - if(calendar && calendar !== 'gregorian' && (step === 'month' || step === 'year')) { - buttonOut.stepmode = 'backward'; - } - else { - coerce('stepmode'); - } - - coerce('count'); + if(visible) { + var step = coerce('step'); + if(step !== 'all') { + if(calendar && calendar !== 'gregorian' && (step === 'month' || step === 'year')) { + buttonOut.stepmode = 'backward'; + } + else { + coerce('stepmode'); } - coerce('label'); + coerce('count'); } - buttonOut._index = i; - buttonsOut.push(buttonOut); + coerce('label'); } - - return buttonsOut; } function getPosDflt(containerOut, layout, counterAxes) { diff --git a/src/components/updatemenus/defaults.js b/src/components/updatemenus/defaults.js index 38dedd0d1c3..96fd5e0dc54 100644 --- a/src/components/updatemenus/defaults.js +++ b/src/components/updatemenus/defaults.js @@ -33,7 +33,10 @@ function menuDefaults(menuIn, menuOut, layoutOut) { return Lib.coerce(menuIn, menuOut, attributes, attr, dflt); } - var buttons = buttonsDefaults(menuIn, menuOut); + var buttons = handleArrayContainerDefaults(menuIn, menuOut, { + name: 'buttons', + handleItemDefaults: buttonDefaults + }); var visible = coerce('visible', buttons.length > 0); if(!visible) return; @@ -62,32 +65,17 @@ function menuDefaults(menuIn, menuOut, layoutOut) { coerce('borderwidth'); } -function buttonsDefaults(menuIn, menuOut) { - var buttonsIn = menuIn.buttons || [], - buttonsOut = menuOut.buttons = []; - - var buttonIn, buttonOut; - +function buttonDefaults(buttonIn, buttonOut, selectorOut, opts, itemOpts) { function coerce(attr, dflt) { return Lib.coerce(buttonIn, buttonOut, buttonAttrs, attr, dflt); } - for(var i = 0; i < buttonsIn.length; i++) { - buttonIn = buttonsIn[i]; - buttonOut = {}; - - var visible = coerce('visible', Lib.isPlainObject(buttonIn) && - (buttonIn.method === 'skip' || Array.isArray(buttonIn.args))); - if(visible) { - coerce('method'); - coerce('args'); - coerce('label'); - coerce('execute'); - } - - buttonOut._index = i; - buttonsOut.push(buttonOut); + var visible = coerce('visible', !itemOpts.itemIsNotPlainObject && + (buttonIn.method === 'skip' || Array.isArray(buttonIn.args))); + if(visible) { + coerce('method'); + coerce('args'); + coerce('label'); + coerce('execute'); } - - return buttonsOut; } diff --git a/src/traces/parcoords/defaults.js b/src/traces/parcoords/defaults.js index 07d1e97b0a8..3ce8cf0a168 100644 --- a/src/traces/parcoords/defaults.js +++ b/src/traces/parcoords/defaults.js @@ -9,12 +9,15 @@ 'use strict'; var Lib = require('../../lib'); -var attributes = require('./attributes'); var hasColorscale = require('../../components/colorscale/has_colorscale'); var colorscaleDefaults = require('../../components/colorscale/defaults'); -var maxDimensionCount = require('./constants').maxDimensionCount; var handleDomainDefaults = require('../../plots/domain').defaults; +var handleArrayContainerDefaults = require('../../plots/array_container_defaults'); + +var attributes = require('./attributes'); var axisBrush = require('./axisbrush'); +var maxDimensionCount = require('./constants').maxDimensionCount; +var mergeLength = require('./merge_length'); function handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce) { var lineColor = coerce('line.color', defaultColor); @@ -26,67 +29,39 @@ function handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce) { // TODO: I think it would be better to keep showing lines beyond the last line color // but I'm not sure what color to give these lines - probably black or white // depending on the background color? - traceOut._length = Math.min(traceOut._length, lineColor.length); + return lineColor.length; } else { traceOut.line.color = defaultColor; } } + return Infinity; } -function dimensionsDefaults(traceIn, traceOut) { - var dimensionsIn = traceIn.dimensions || [], - dimensionsOut = traceOut.dimensions = []; - - var dimensionIn, dimensionOut, i; - var commonLength = Infinity; - - if(dimensionsIn.length > maxDimensionCount) { - Lib.log('parcoords traces support up to ' + maxDimensionCount + ' dimensions at the moment'); - dimensionsIn.splice(maxDimensionCount); - } - +function dimensionDefaults(dimensionIn, dimensionOut, traceOut, opts, itemOpts) { function coerce(attr, dflt) { return Lib.coerce(dimensionIn, dimensionOut, attributes.dimensions, attr, dflt); } - for(i = 0; i < dimensionsIn.length; i++) { - dimensionIn = dimensionsIn[i]; - dimensionOut = {}; - - if(!Lib.isPlainObject(dimensionIn)) { - continue; - } - - var values = coerce('values'); - var visible = coerce('visible'); - if(!(values && values.length)) { - visible = dimensionOut.visible = false; - } - - if(visible) { - coerce('label'); - coerce('tickvals'); - coerce('ticktext'); - coerce('tickformat'); - coerce('range'); - - coerce('multiselect'); - var constraintRange = coerce('constraintrange'); - if(constraintRange) { - dimensionOut.constraintrange = axisBrush.cleanRanges(constraintRange, dimensionOut); - } + var values = coerce('values'); + var visible = coerce('visible'); + if(!(values && values.length && !itemOpts.itemIsNotPlainObject)) { + visible = dimensionOut.visible = false; + } - commonLength = Math.min(commonLength, values.length); + if(visible) { + coerce('label'); + coerce('tickvals'); + coerce('ticktext'); + coerce('tickformat'); + coerce('range'); + + coerce('multiselect'); + var constraintRange = coerce('constraintrange'); + if(constraintRange) { + dimensionOut.constraintrange = axisBrush.cleanRanges(constraintRange, dimensionOut); } - - dimensionOut._index = i; - dimensionsOut.push(dimensionOut); } - - traceOut._length = commonLength; - - return dimensionsOut; } module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) { @@ -94,9 +69,18 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout return Lib.coerce(traceIn, traceOut, attributes, attr, dflt); } - var dimensions = dimensionsDefaults(traceIn, traceOut); + var dimensionsIn = traceIn.dimensions; + if(Array.isArray(dimensionsIn) && dimensionsIn.length > maxDimensionCount) { + Lib.log('parcoords traces support up to ' + maxDimensionCount + ' dimensions at the moment'); + dimensionsIn.splice(maxDimensionCount); + } + + var dimensions = handleArrayContainerDefaults(traceIn, traceOut, { + name: 'dimensions', + handleItemDefaults: dimensionDefaults + }); - handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce); + var len = handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce); handleDomainDefaults(traceOut, layout, coerce); @@ -104,12 +88,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout traceOut.visible = false; } - // since we're not slicing uneven arrays anymore, stash the length in each dimension - // but we can't do this in dimensionsDefaults (yet?) because line.color can also - // truncate - for(var i = 0; i < dimensions.length; i++) { - if(dimensions[i].visible) dimensions[i]._length = traceOut._length; - } + mergeLength(traceOut, dimensions, 'values', len); // make default font size 10px (default is 12), // scale linearly with global font size diff --git a/src/traces/parcoords/merge_length.js b/src/traces/parcoords/merge_length.js new file mode 100644 index 00000000000..eccb5326c7b --- /dev/null +++ b/src/traces/parcoords/merge_length.js @@ -0,0 +1,36 @@ +/** +* Copyright 2012-2018, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +/** + * mergeLength: set trace length as the minimum of all dimension data lengths + * and propagates this length into each dimension + * + * @param {object} traceOut: the fullData trace + * @param {Array(object)} dimensions: array of dimension objects + * @param {string} dataAttr: the attribute of each dimension containing the data + * @param {integer} len: an already-existing length from other attributes + */ +module.exports = function(traceOut, dimensions, dataAttr, len) { + if(!len) len = Infinity; + var i, dimi; + for(i = 0; i < dimensions.length; i++) { + dimi = dimensions[i]; + if(dimi.visible) len = Math.min(len, dimi[dataAttr].length); + } + if(len === Infinity) len = 0; + + traceOut._length = len; + for(i = 0; i < dimensions.length; i++) { + dimi = dimensions[i]; + if(dimi.visible) dimi._length = len; + } + + return len; +}; diff --git a/src/traces/splom/defaults.js b/src/traces/splom/defaults.js index 8e5f930d085..72665747321 100644 --- a/src/traces/splom/defaults.js +++ b/src/traces/splom/defaults.js @@ -9,10 +9,12 @@ 'use strict'; var Lib = require('../../lib'); +var handleArrayContainerDefaults = require('../../plots/array_container_defaults'); var attributes = require('./attributes'); var subTypes = require('../scatter/subtypes'); var handleMarkerDefaults = require('../scatter/marker_defaults'); +var mergeLength = require('../parcoords/merge_length'); var OPEN_RE = /-open/; module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) { @@ -20,12 +22,17 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout return Lib.coerce(traceIn, traceOut, attributes, attr, dflt); } - var dimLength = handleDimensionsDefaults(traceIn, traceOut); + var dimensions = handleArrayContainerDefaults(traceIn, traceOut, { + name: 'dimensions', + handleItemDefaults: dimensionDefaults + }); var showDiag = coerce('diagonal.visible'); var showUpper = coerce('showupperhalf'); var showLower = coerce('showlowerhalf'); + var dimLength = mergeLength(traceOut, dimensions, 'values'); + if(!dimLength || (!showDiag && !showUpper && !showLower)) { traceOut.visible = false; return; @@ -44,51 +51,18 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout Lib.coerceSelectionMarkerOpacity(traceOut, coerce); }; -function handleDimensionsDefaults(traceIn, traceOut) { - var dimensionsIn = traceIn.dimensions; - if(!Array.isArray(dimensionsIn)) return 0; - - var dimLength = dimensionsIn.length; - var commonLength = 0; - var dimensionsOut = traceOut.dimensions = new Array(dimLength); - var dimIn; - var dimOut; - var i; - +function dimensionDefaults(dimIn, dimOut, traceOut, opts, itemOpts) { function coerce(attr, dflt) { return Lib.coerce(dimIn, dimOut, attributes.dimensions, attr, dflt); } - for(i = 0; i < dimLength; i++) { - dimIn = dimensionsIn[i]; - dimOut = dimensionsOut[i] = {}; + coerce('label'); + coerce('visible'); + var values = coerce('values'); - // coerce label even if dimensions may be `visible: false`, - // to fill in axis title defaults - coerce('label'); - - // wait until plot step to filter out visible false dimensions - var visible = coerce('visible'); - if(!visible) continue; - - var values = coerce('values'); - if(!values || !values.length) { - dimOut.visible = false; - continue; - } - - commonLength = Math.max(commonLength, values.length); - dimOut._index = i; - } - - for(i = 0; i < dimLength; i++) { - dimOut = dimensionsOut[i]; - if(dimOut.visible) dimOut._length = commonLength; + if(!(values && values.length && !itemOpts.itemIsNotPlainObject)) { + dimOut.visible = false; } - - traceOut._length = commonLength; - - return dimensionsOut.length; } function handleAxisDefaults(traceIn, traceOut, layout, coerce) { diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index a4ea3e41e86..0f696eaff5a 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -136,14 +136,14 @@ describe('parcoords initialization tests', function() { alienProperty: 'Alpha Centauri' }] }); - expect(fullTrace.dimensions).toEqual([{ + expect(fullTrace.dimensions).toEqual([jasmine.objectContaining({ values: [1], visible: true, tickformat: '3s', multiselect: true, _index: 0, _length: 1 - }]); + })]); }); it('\'dimension.visible\' should be set to false, and other props just passed through if \'values\' is not provided', function() { @@ -152,7 +152,9 @@ describe('parcoords initialization tests', function() { alienProperty: 'Alpha Centauri' }] }); - expect(fullTrace.dimensions).toEqual([{visible: false, _index: 0}]); + expect(fullTrace.dimensions).toEqual([jasmine.objectContaining({ + visible: false, _index: 0 + })]); }); it('\'dimension.visible\' should be set to false, and other props just passed through if \'values\' is an empty array', function() { @@ -162,7 +164,9 @@ describe('parcoords initialization tests', function() { alienProperty: 'Alpha Centauri' }] }); - expect(fullTrace.dimensions).toEqual([{visible: false, values: [], _index: 0}]); + expect(fullTrace.dimensions).toEqual([jasmine.objectContaining({ + visible: false, values: [], _index: 0 + })]); }); it('\'dimension.visible\' should be set to false, and other props just passed through if \'values\' is not an array', function() { @@ -172,7 +176,9 @@ describe('parcoords initialization tests', function() { alienProperty: 'Alpha Centauri' }] }); - expect(fullTrace.dimensions).toEqual([{visible: false, _index: 0}]); + expect(fullTrace.dimensions).toEqual([jasmine.objectContaining({ + visible: false, _index: 0 + })]); }); it('\'dimension.values\' should get truncated to a common shortest *nonzero* length', function() { diff --git a/test/jasmine/tests/range_selector_test.js b/test/jasmine/tests/range_selector_test.js index 5ff819d6218..c63e17039cf 100644 --- a/test/jasmine/tests/range_selector_test.js +++ b/test/jasmine/tests/range_selector_test.js @@ -55,13 +55,13 @@ describe('range selector defaults:', function() { supply(containerIn, containerOut); expect(containerIn.rangeselector.buttons).toEqual([{}]); - expect(containerOut.rangeselector.buttons).toEqual([{ + expect(containerOut.rangeselector.buttons).toEqual([jasmine.objectContaining({ visible: true, step: 'month', stepmode: 'backward', count: 1, _index: 0 - }]); + })]); }); it('should skip over non-object buttons', function() { @@ -103,8 +103,8 @@ describe('range selector defaults:', function() { expect(containerOut.rangeselector.visible).toBe(true); expect(containerOut.rangeselector.buttons).toEqual([ - { visible: true, step: 'year', stepmode: 'backward', count: 10, _index: 0 }, - { visible: true, step: 'month', stepmode: 'backward', count: 6, _index: 1 } + jasmine.objectContaining({ visible: true, step: 'year', stepmode: 'backward', count: 10, _index: 0 }), + jasmine.objectContaining({ visible: true, step: 'month', stepmode: 'backward', count: 6, _index: 1 }) ]); }); @@ -121,12 +121,12 @@ describe('range selector defaults:', function() { supply(containerIn, containerOut); - expect(containerOut.rangeselector.buttons).toEqual([{ + expect(containerOut.rangeselector.buttons).toEqual([jasmine.objectContaining({ visible: true, step: 'all', label: 'full range', _index: 0 - }]); + })]); }); it('should use axis and counter axis to determine \'x\' and \'y\' defaults (case 1 y)', function() {