From 5829a0a8bc087d713398ae0996fd17c9bedf1afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 19 Dec 2016 17:52:08 -0500 Subject: [PATCH 1/4] reorg cartesian layout defaults: - define scoped coerce function once for all axes - only loop over xaxis list for rangeslider / rangeselector defaults --- src/plots/cartesian/layout_defaults.js | 75 ++++++++++++++------------ 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 625cb9e171f..fb2ab568801 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -115,33 +115,39 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { var bgColor = Color.combine(plot_bgcolor, layoutOut.paper_bgcolor); + var axLayoutIn, axLayoutOut; + + function coerce(attr, dflt) { + return Lib.coerce(axLayoutIn, axLayoutOut, layoutAttributes, attr, dflt); + } + axesList.forEach(function(axName) { - var axLetter = axName.charAt(0), - axLayoutIn = layoutIn[axName] || {}, - axLayoutOut = {}, - defaultOptions = { - letter: axLetter, - font: layoutOut.font, - outerTicks: outerTicks[axName], - showGrid: !noGrids[axName], - name: axName, - data: fullData, - bgColor: bgColor, - calendar: layoutOut.calendar - }, - positioningOptions = { - letter: axLetter, - counterAxes: {x: yaList, y: xaList}[axLetter].map(axisIds.name2id), - overlayableAxes: {x: xaList, y: yaList}[axLetter].filter(function(axName2) { - return axName2 !== axName && !(layoutIn[axName2] || {}).overlaying; - }).map(axisIds.name2id) - }; - - function coerce(attr, dflt) { - return Lib.coerce(axLayoutIn, axLayoutOut, layoutAttributes, attr, dflt); - } + var axLetter = axName.charAt(0); + + axLayoutIn = layoutIn[axName] || {}; + axLayoutOut = {}; + + var defaultOptions = { + letter: axLetter, + font: layoutOut.font, + outerTicks: outerTicks[axName], + showGrid: !noGrids[axName], + name: axName, + data: fullData, + bgColor: bgColor, + calendar: layoutOut.calendar + }; handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); + + var positioningOptions = { + letter: axLetter, + counterAxes: {x: yaList, y: xaList}[axLetter].map(axisIds.name2id), + overlayableAxes: {x: xaList, y: yaList}[axLetter].filter(function(axName2) { + return axName2 !== axName && !(layoutIn[axName2] || {}).overlaying; + }).map(axisIds.name2id) + }; + handlePositionDefaults(axLayoutIn, axLayoutOut, coerce, positioningOptions); layoutOut[axName] = axLayoutOut; @@ -158,17 +164,20 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { var rangeSliderDefaults = Registry.getComponentMethod('rangeslider', 'handleDefaults'), rangeSelectorDefaults = Registry.getComponentMethod('rangeselector', 'handleDefaults'); - axesList.forEach(function(axName) { - var axLetter = axName.charAt(0), - axLayoutIn = layoutIn[axName], - axLayoutOut = layoutOut[axName], - counterAxes = {x: yaList, y: xaList}[axLetter]; + xaList.forEach(function(axName) { + axLayoutIn = layoutIn[axName]; + axLayoutOut = layoutOut[axName]; - rangeSliderDefaults(layoutIn, layoutOut, axName, counterAxes); + rangeSliderDefaults(layoutIn, layoutOut, axName); - if(axLetter === 'x' && axLayoutOut.type === 'date') { - rangeSelectorDefaults(axLayoutIn, axLayoutOut, layoutOut, counterAxes, - axLayoutOut.calendar); + if(axLayoutOut.type === 'date') { + rangeSelectorDefaults( + axLayoutIn, + axLayoutOut, + layoutOut, + yaList, + axLayoutOut.calendar + ); } }); }; From df09650455714e2dadfdb80f032b213a3795fbbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 19 Dec 2016 17:54:30 -0500 Subject: [PATCH 2/4] rm hard `fixedrange: true` set in slider defaults - replace with 'smart' defaults in cartesian layout defaults so that user set `fixedrange: false` will now be honored. --- src/components/rangeslider/defaults.js | 10 +--------- src/plots/cartesian/axis_defaults.js | 2 -- src/plots/cartesian/layout_defaults.js | 17 +++++++++++++++++ 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/components/rangeslider/defaults.js b/src/components/rangeslider/defaults.js index ab448bb022a..4c307c2292d 100644 --- a/src/components/rangeslider/defaults.js +++ b/src/components/rangeslider/defaults.js @@ -12,7 +12,7 @@ var Lib = require('../../lib'); var attributes = require('./attributes'); -module.exports = function handleDefaults(layoutIn, layoutOut, axName, counterAxes) { +module.exports = function handleDefaults(layoutIn, layoutOut, axName) { if(!layoutIn[axName].rangeslider) return; // not super proud of this (maybe store _ in axis object instead @@ -47,14 +47,6 @@ module.exports = function handleDefaults(layoutIn, layoutOut, axName, counterAxe axOut._needsExpand = true; } - if(containerOut.visible) { - counterAxes.forEach(function(ax) { - var opposing = layoutOut[ax] || {}; - opposing.fixedrange = true; - layoutOut[ax] = opposing; - }); - } - // to map back range slider (auto) range containerOut._input = containerIn; }; diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js index a0ab9aed0b8..a26b08d7698 100644 --- a/src/plots/cartesian/axis_defaults.js +++ b/src/plots/cartesian/axis_defaults.js @@ -105,8 +105,6 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, coerce('range'); containerOut.cleanRange(); - coerce('fixedrange'); - handleTickValueDefaults(containerIn, containerOut, coerce, axType); handleTickLabelDefaults(containerIn, containerOut, coerce, axType, options); handleTickMarkDefaults(containerIn, containerOut, coerce, options); diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index fb2ab568801..381b0241b4b 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -179,5 +179,22 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { axLayoutOut.calendar ); } + + coerce('fixedrange'); + }); + + yaList.forEach(function(axName) { + axLayoutIn = layoutIn[axName]; + axLayoutOut = layoutOut[axName]; + + var anchoredAxis = layoutOut[axisIds.id2name(axLayoutOut.anchor)]; + + var fixedRangeDflt = ( + anchoredAxis && + anchoredAxis.rangeslider && + anchoredAxis.rangeslider.visible + ); + + coerce('fixedrange', fixedRangeDflt); }); }; From b9bc96d45d629cf1c58bbbb1aceae9a72b984689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 19 Dec 2016 17:55:14 -0500 Subject: [PATCH 3/4] rm 'fixedrange' from gl3d axis attributes - this was never implemented - it was in there *just* to make the call to axisDefaults work --- src/plots/gl3d/layout/axis_attributes.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plots/gl3d/layout/axis_attributes.js b/src/plots/gl3d/layout/axis_attributes.js index faad75ece28..122d971ed37 100644 --- a/src/plots/gl3d/layout/axis_attributes.js +++ b/src/plots/gl3d/layout/axis_attributes.js @@ -76,7 +76,6 @@ module.exports = { autorange: axesAttrs.autorange, rangemode: axesAttrs.rangemode, range: axesAttrs.range, - fixedrange: axesAttrs.fixedrange, // ticks tickmode: axesAttrs.tickmode, nticks: axesAttrs.nticks, From 32f98a677d2daa6e8c387d8ffe36be6f34997160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 19 Dec 2016 17:56:38 -0500 Subject: [PATCH 4/4] update range slider tests - rm now useless yaxis mocks - add better fixedrange logic test cases --- test/jasmine/tests/range_slider_test.js | 153 +++++++++++------------- 1 file changed, 69 insertions(+), 84 deletions(-) diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index 464c5a49776..0348c8cfaac 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -1,4 +1,5 @@ var Plotly = require('@lib/index'); +var Plots = require('@src/plots/plots'); var Lib = require('@src/lib'); var setConvert = require('@src/plots/cartesian/set_convert'); @@ -336,34 +337,28 @@ describe('the range slider', function() { describe('handleDefaults function', function() { it('should not coerce anything if rangeslider isn\'t set', function() { - var layoutIn = { xaxis: {}, yaxis: {}}, - layoutOut = { xaxis: {}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], - expected = { xaxis: {}, yaxis: {}}; + var layoutIn = { xaxis: {} }, + layoutOut = { xaxis: {} }, + expected = { xaxis: {} }; - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutIn).toEqual(expected); }); it('should not mutate layoutIn', function() { - var layoutIn = { xaxis: { rangeslider: { visible: true }}, yaxis: {}}, - layoutOut = { xaxis: { rangeslider: {}}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], - expected = { xaxis: { rangeslider: { visible: true }}, yaxis: {}}; + var layoutIn = { xaxis: { rangeslider: { visible: true }} }, + layoutOut = { xaxis: { rangeslider: {}} }, + expected = { xaxis: { rangeslider: { visible: true }} }; - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutIn).toEqual(expected); }); it('should set defaults if rangeslider is set to anything truthy', function() { - var layoutIn = { xaxis: { rangeslider: {}}, yaxis: {}}, - layoutOut = { xaxis: {}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], + var layoutIn = { xaxis: { rangeslider: {} }}, + layoutOut = { xaxis: {} }, expected = { xaxis: { rangeslider: { @@ -375,22 +370,17 @@ describe('the range slider', function() { _input: layoutIn.xaxis.rangeslider }, _needsExpand: true - }, - yaxis: { - fixedrange: true - }, + } }; - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutOut).toEqual(expected); }); it('should set defaults if rangeslider.visible is true', function() { - var layoutIn = { xaxis: { rangeslider: { visible: true }}, yaxis: {}}, - layoutOut = { xaxis: { rangeslider: {}}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], + var layoutIn = { xaxis: { rangeslider: { visible: true }} }, + layoutOut = { xaxis: { rangeslider: {}} }, expected = { xaxis: { rangeslider: { @@ -402,13 +392,10 @@ describe('the range slider', function() { _input: layoutIn.xaxis.rangeslider }, _needsExpand: true - }, - yaxis: { - fixedrange: true } }; - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutOut).toEqual(expected); }); @@ -420,10 +407,8 @@ describe('the range slider', function() { bgcolor: 42, bordercolor: 42, borderwidth: 'superfat' - }}, yaxis: {}}, - layoutOut = { xaxis: {}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], + }}}, + layoutOut = { xaxis: {} }, expected = { xaxis: { rangeslider: { @@ -435,48 +420,17 @@ describe('the range slider', function() { _input: layoutIn.xaxis.rangeslider }, _needsExpand: true - }, - yaxis: { - fixedrange: true } }; - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); - - expect(layoutOut).toEqual(expected); - }); - - it('should set all counterAxes to fixedrange', function() { - var layoutIn = { xaxis: { rangeslider: true }, yaxis: {}, yaxis2: {}}, - layoutOut = { xaxis: {}, yaxis: {}, yaxis2: {}}, - axName = 'xaxis', - counterAxes = ['yaxis', 'yaxis2'], - expected = { - xaxis: { - rangeslider: { - visible: true, - thickness: 0.15, - bgcolor: '#fff', - borderwidth: 0, - bordercolor: '#444', - _input: {} - }, - _needsExpand: true - }, - yaxis: { fixedrange: true}, - yaxis2: { fixedrange: true } - }; - - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutOut).toEqual(expected); }); it('should expand the rangeslider range to axis range', function() { - var layoutIn = { xaxis: { rangeslider: { range: [5, 6] } }, yaxis: {}}, - layoutOut = { xaxis: { range: [1, 10], type: 'linear'}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], + var layoutIn = { xaxis: { rangeslider: { range: [5, 6] } } }, + layoutOut = { xaxis: { range: [1, 10], type: 'linear'} }, expected = { xaxis: { rangeslider: { @@ -489,24 +443,21 @@ describe('the range slider', function() { _input: layoutIn.xaxis.rangeslider }, range: [1, 10] - }, - yaxis: { fixedrange: true } + } }; + setConvert(layoutOut.xaxis); - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); // don't compare the whole layout, because we had to run setConvert which // attaches all sorts of other stuff to xaxis expect(layoutOut.xaxis.rangeslider).toEqual(expected.xaxis.rangeslider); - expect(layoutOut.yaxis).toEqual(expected.yaxis); }); it('should set _needsExpand when an axis range is set', function() { - var layoutIn = { xaxis: { rangeslider: true }, yaxis: {}}, - layoutOut = { xaxis: { range: [2, 40]}, yaxis: {}}, - axName = 'xaxis', - counterAxes = ['yaxis'], + var layoutIn = { xaxis: { rangeslider: true } }, + layoutOut = { xaxis: { range: [2, 40]} }, expected = { xaxis: { rangeslider: { @@ -520,35 +471,69 @@ describe('the range slider', function() { range: [2, 40], _needsExpand: true }, - yaxis: { fixedrange: true } }; - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutOut).toEqual(expected); }); it('should default \'bgcolor\' to layout \'plot_bgcolor\'', function() { var layoutIn = { - xaxis: { rangeslider: true }, - yaxis: {}, + xaxis: { rangeslider: true } }; var layoutOut = { xaxis: { range: [2, 40]}, - yaxis: {}, plot_bgcolor: 'blue' }; - var axName = 'xaxis', - counterAxes = ['yaxis']; - - RangeSlider.handleDefaults(layoutIn, layoutOut, axName, counterAxes); + RangeSlider.handleDefaults(layoutIn, layoutOut, 'xaxis'); expect(layoutOut.xaxis.rangeslider.bgcolor).toEqual('blue'); }); }); + describe('anchored axes fixedrange', function() { + + it('should default to *true* when range slider is visible', function() { + var mock = { + layout: { + xaxis: { rangeslider: {} }, + yaxis: { anchor: 'x' }, + yaxis2: { anchor: 'x' }, + yaxis3: { anchor: 'free' } + } + }; + + Plots.supplyDefaults(mock); + + expect(mock._fullLayout.xaxis.rangeslider.visible).toBe(true); + expect(mock._fullLayout.yaxis.fixedrange).toBe(true); + expect(mock._fullLayout.yaxis2.fixedrange).toBe(true); + expect(mock._fullLayout.yaxis3.fixedrange).toBe(false); + }); + + it('should honor user settings', function() { + var mock = { + layout: { + xaxis: { rangeslider: {} }, + yaxis: { anchor: 'x', fixedrange: false }, + yaxis2: { anchor: 'x', fixedrange: false }, + yaxis3: { anchor: 'free' } + } + }; + + Plots.supplyDefaults(mock); + + expect(mock._fullLayout.xaxis.rangeslider.visible).toBe(true); + expect(mock._fullLayout.yaxis.fixedrange).toBe(false); + expect(mock._fullLayout.yaxis2.fixedrange).toBe(false); + expect(mock._fullLayout.yaxis3.fixedrange).toBe(false); + }); + + }); + describe('in general', function() { beforeEach(function() {