From 948f493c8542ed7588cf7d4c12725b590eb366d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Tusz?= Date: Thu, 31 Mar 2016 15:03:26 -0400 Subject: [PATCH 1/3] GL rangeslider saftey check --- src/components/rangeslider/create_slider.js | 8 +++---- src/components/rangeslider/index.js | 8 +++++-- test/jasmine/tests/range_slider_test.js | 23 +++++++++++++++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/components/rangeslider/create_slider.js b/src/components/rangeslider/create_slider.js index 25254d5018d..0aaf2b98315 100644 --- a/src/components/rangeslider/create_slider.js +++ b/src/components/rangeslider/create_slider.js @@ -249,8 +249,8 @@ module.exports = function createSlider(gd, minStart, maxStart) { ]); sliderContainer.data([0]) - .enter().append(function() { - options.setRange = setRange; - return slider; - }); + .enter().append(function() { + options.setRange = setRange; + return slider; + }); }; diff --git a/src/components/rangeslider/index.js b/src/components/rangeslider/index.js index d74eefdbd1a..77a4c092430 100644 --- a/src/components/rangeslider/index.js +++ b/src/components/rangeslider/index.js @@ -41,11 +41,15 @@ function draw(gd, minStart, maxStart) { var height = (fullLayout.height - fullLayout.margin.b - fullLayout.margin.t) * options.thickness, offsetShift = Math.floor(options.borderwidth / 2); - if(sliderContainer[0].length === 0) createSlider(gd, minStart, maxStart); + if(sliderContainer[0].length === 0 && !fullLayout._hasGL2D) createSlider(gd, minStart, maxStart); + + // Need to default to 0 for when making gl plots + var bb = fullLayout.xaxis._boundingBox ? + fullLayout.xaxis._boundingBox.height : 0; Plots.autoMargin(gd, 'range-slider', { x: 0, y: 0, l: 0, r: 0, t: 0, - b: height + fullLayout.margin.b + fullLayout.xaxis._boundingBox.height, + b: height + fullLayout.margin.b + bb, pad: 15 + offsetShift * 2 }); } diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index 1c0dc2ddced..e0b689d18dc 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -308,6 +308,29 @@ describe('the range slider', function() { expect(layoutOut).toEqual(expected); }); }); + + fdescribe('when used with gl traces', function() { + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('should not be drawn with GL2d', function(done) { + var data = [{ + x: [1,2,3], + y: [2,3,4], + type: 'scattergl' + }]; + + Plotly.plot(gd, data, mock.layout).then(function() { + rangeSlider = document.getElementsByClassName('range-slider')[0]; + expect(rangeSlider).not.toBeDefined(); + done(); + }); + }); + }); }); From 5297bf16a9c7a30f04b9aba9f4a33f134d5ef1f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Tusz?= Date: Thu, 31 Mar 2016 15:12:24 -0400 Subject: [PATCH 2/3] Fixup! remove fdescribe and add second trace to test --- test/jasmine/tests/range_slider_test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index e0b689d18dc..e11ff8d0ff9 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -309,7 +309,7 @@ describe('the range slider', function() { }); }); - fdescribe('when used with gl traces', function() { + describe('when used with gl traces', function() { beforeEach(function() { gd = createGraphDiv(); @@ -322,6 +322,10 @@ describe('the range slider', function() { x: [1,2,3], y: [2,3,4], type: 'scattergl' + }, { + x: [1,2,3], + y: [2,3,4], + type: 'scattergl' }]; Plotly.plot(gd, data, mock.layout).then(function() { From 839db41189ac6a6ba2b6fbb6f812360ae5856f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Tusz?= Date: Thu, 31 Mar 2016 15:46:29 -0400 Subject: [PATCH 3/3] Move tests to gl-specific file --- test/jasmine/tests/gl_plot_interact_test.js | 35 +++++++++++++++++++++ test/jasmine/tests/range_slider_test.js | 27 ---------------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index 92607cf5d42..6f6cdd96336 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -414,3 +414,38 @@ describe('Test gl plot interactions', function() { }); }); }); + +describe('Test gl plot side effects', function() { + describe('when present with rangeslider', function() { + + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('should not draw the rangeslider', function(done) { + var data = [{ + x: [1,2,3], + y: [2,3,4], + type: 'scattergl' + }, { + x: [1,2,3], + y: [2,3,4], + type: 'scatter' + }]; + + var layout = { + xaxis: { rangeslider: { visible: true } } + }; + + Plotly.plot(gd, data, layout).then(function() { + var rangeSlider = document.getElementsByClassName('range-slider')[0]; + expect(rangeSlider).not.toBeDefined(); + done(); + }); + }); + }); +}); diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index e11ff8d0ff9..1c0dc2ddced 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -308,33 +308,6 @@ describe('the range slider', function() { expect(layoutOut).toEqual(expected); }); }); - - describe('when used with gl traces', function() { - - beforeEach(function() { - gd = createGraphDiv(); - }); - - afterEach(destroyGraphDiv); - - it('should not be drawn with GL2d', function(done) { - var data = [{ - x: [1,2,3], - y: [2,3,4], - type: 'scattergl' - }, { - x: [1,2,3], - y: [2,3,4], - type: 'scattergl' - }]; - - Plotly.plot(gd, data, mock.layout).then(function() { - rangeSlider = document.getElementsByClassName('range-slider')[0]; - expect(rangeSlider).not.toBeDefined(); - done(); - }); - }); - }); });