From a8ec02f8fce25fc6fc3bef606ea5da3adab96f44 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 8 Nov 2018 14:29:05 -0500 Subject: [PATCH 1/7] prevent NaN to be used in 3d axis ticks --- src/plots/cartesian/axes.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index c2352fcdc33..3c7e4c09913 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -755,6 +755,9 @@ axes.autoTicks = function(ax, roughDTick) { // prevent infinite loops if(ax.dtick === 0) ax.dtick = 1; + // prevent issue https://github.com/plotly/plotly.js/issues/3224 + if (Number.isNaN(ax.dtick)) ax.dtick = 1; + // TODO: this is from log axis histograms with autorange off if(!isNumeric(ax.dtick) && typeof ax.dtick !== 'string') { var olddtick = ax.dtick; From fbb4bc983b60b951db7f8410c182bcb8fa572092 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 8 Nov 2018 14:39:26 -0500 Subject: [PATCH 2/7] prevent NaN to be used in 3d axis ticks - fixed syntax --- src/plots/cartesian/axes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 3c7e4c09913..332844ac259 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -756,7 +756,7 @@ axes.autoTicks = function(ax, roughDTick) { if(ax.dtick === 0) ax.dtick = 1; // prevent issue https://github.com/plotly/plotly.js/issues/3224 - if (Number.isNaN(ax.dtick)) ax.dtick = 1; + if(Number.isNaN(ax.dtick)) ax.dtick = 1; // TODO: this is from log axis histograms with autorange off if(!isNumeric(ax.dtick) && typeof ax.dtick !== 'string') { From 965345ee1c26d040022ef28228fdc0ac8cee09d5 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 8 Nov 2018 17:46:40 -0500 Subject: [PATCH 3/7] fixing cases where axes._length is NaN --- src/plots/cartesian/axes.js | 6 ++---- src/plots/gl3d/layout/tick_marks.js | 6 ++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 332844ac259..747fe097313 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -492,7 +492,8 @@ axes.prepTicks = function(ax) { } else { minPx = ax._id.charAt(0) === 'y' ? 40 : 80; - nt = Lib.constrain(ax._length / minPx, 4, 9) + 1; + if(Number.isNaN(ax._length)) nt = 1; // or whatever default it should be + else nt = Lib.constrain(ax._length / minPx, 4, 9) + 1; } // radial axes span half their domain, @@ -755,9 +756,6 @@ axes.autoTicks = function(ax, roughDTick) { // prevent infinite loops if(ax.dtick === 0) ax.dtick = 1; - // prevent issue https://github.com/plotly/plotly.js/issues/3224 - if(Number.isNaN(ax.dtick)) ax.dtick = 1; - // TODO: this is from log axis histograms with autorange off if(!isNumeric(ax.dtick) && typeof ax.dtick !== 'string') { var olddtick = ax.dtick; diff --git a/src/plots/gl3d/layout/tick_marks.js b/src/plots/gl3d/layout/tick_marks.js index 97b4e20fed5..09194a58c56 100644 --- a/src/plots/gl3d/layout/tick_marks.js +++ b/src/plots/gl3d/layout/tick_marks.js @@ -66,8 +66,10 @@ function computeTickMarks(scene) { var tickModeCached = axes.tickmode; if(axes.tickmode === 'auto') { axes.tickmode = 'linear'; - var nticks = axes.nticks || Lib.constrain((axes._length / 40), 4, 9); - Axes.autoTicks(axes, Math.abs(axes.range[1] - axes.range[0]) / nticks); + if(!Number.isNaN(axes._length)) { + var nticks = axes.nticks || Lib.constrain((axes._length / 40), 4, 9); + Axes.autoTicks(axes, Math.abs(axes.range[1] - axes.range[0]) / nticks); + } } var dataTicks = Axes.calcTicks(axes); for(var j = 0; j < dataTicks.length; ++j) { From 9f8f7850101f793121ea6c6359ad8ca6a2bea2bb Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 8 Nov 2018 18:17:29 -0500 Subject: [PATCH 4/7] clean fix --- src/plots/cartesian/axes.js | 3 +-- src/plots/gl3d/layout/tick_marks.js | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 747fe097313..c2352fcdc33 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -492,8 +492,7 @@ axes.prepTicks = function(ax) { } else { minPx = ax._id.charAt(0) === 'y' ? 40 : 80; - if(Number.isNaN(ax._length)) nt = 1; // or whatever default it should be - else nt = Lib.constrain(ax._length / minPx, 4, 9) + 1; + nt = Lib.constrain(ax._length / minPx, 4, 9) + 1; } // radial axes span half their domain, diff --git a/src/plots/gl3d/layout/tick_marks.js b/src/plots/gl3d/layout/tick_marks.js index 09194a58c56..a4b14d8f56b 100644 --- a/src/plots/gl3d/layout/tick_marks.js +++ b/src/plots/gl3d/layout/tick_marks.js @@ -47,7 +47,9 @@ function computeTickMarks(scene) { axes._length = (glRange[i].hi - glRange[i].lo) * glRange[i].pixelsPerDataUnit / scene.dataScale[i]; - if(Math.abs(axes._length) === Infinity) { + if(Math.abs(axes._length) === Infinity || + Math.abs(axes._length) === -Infinity || + Number.isNaN(axes._length)) { ticks[i] = []; } else { axes._input_range = axes.range.slice(); @@ -66,10 +68,8 @@ function computeTickMarks(scene) { var tickModeCached = axes.tickmode; if(axes.tickmode === 'auto') { axes.tickmode = 'linear'; - if(!Number.isNaN(axes._length)) { - var nticks = axes.nticks || Lib.constrain((axes._length / 40), 4, 9); - Axes.autoTicks(axes, Math.abs(axes.range[1] - axes.range[0]) / nticks); - } + var nticks = axes.nticks || Lib.constrain((axes._length / 40), 4, 9); + Axes.autoTicks(axes, Math.abs(axes.range[1] - axes.range[0]) / nticks); } var dataTicks = Axes.calcTicks(axes); for(var j = 0; j < dataTicks.length; ++j) { From 69393b9b27f4873593f3d76f30e39f684a3e995a Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 8 Nov 2018 19:13:42 -0500 Subject: [PATCH 5/7] added the jasmine test to lock issue 3224 --- src/plots/gl3d/layout/tick_marks.js | 2 +- test/jasmine/tests/gl3d_plot_interact_test.js | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/plots/gl3d/layout/tick_marks.js b/src/plots/gl3d/layout/tick_marks.js index a4b14d8f56b..21d01c831e3 100644 --- a/src/plots/gl3d/layout/tick_marks.js +++ b/src/plots/gl3d/layout/tick_marks.js @@ -49,7 +49,7 @@ function computeTickMarks(scene) { if(Math.abs(axes._length) === Infinity || Math.abs(axes._length) === -Infinity || - Number.isNaN(axes._length)) { + isNaN(axes._length)) { ticks[i] = []; } else { axes._input_range = axes.range.slice(); diff --git a/test/jasmine/tests/gl3d_plot_interact_test.js b/test/jasmine/tests/gl3d_plot_interact_test.js index 7cdb1e36e7f..0eb6b4fd859 100644 --- a/test/jasmine/tests/gl3d_plot_interact_test.js +++ b/test/jasmine/tests/gl3d_plot_interact_test.js @@ -558,6 +558,36 @@ describe('Test gl3d plots', function() { .catch(failTest) .then(done); }); + + it('@gl axis ticks should not be set when axis _length is NaN', function(done) { + Plotly.plot(gd, + { + data: [{ + type: 'scatter3d', + mode: 'markers', + x: [1, 2], + y: [3, 4], + z: [5, 6] + }], + layout: { + scene: { + camera:{ + eye: {x: 1, y: 1, z: 0}, + center: {x: 0.5, y: 0.5, z: 1}, + up: {x: 0, y: 0, z: 1} + } + } + } + } + ) + .then(function() { + var zaxis = gd._fullLayout.scene.zaxis; + expect(isNaN(zaxis._length)).toBe(true); + expect(zaxis.dtick === undefined).toBe(true); + }) + .catch(failTest) + .then(done); + }); }); describe('Test gl3d modebar handlers', function() { From 3b8b4902e7be94fc015e2f5470f15d6ef2b63946 Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com> Date: Thu, 8 Nov 2018 20:52:18 -0500 Subject: [PATCH 6/7] Update gl3d_plot_interact_test.js --- test/jasmine/tests/gl3d_plot_interact_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/gl3d_plot_interact_test.js b/test/jasmine/tests/gl3d_plot_interact_test.js index 0eb6b4fd859..7c29eba4bac 100644 --- a/test/jasmine/tests/gl3d_plot_interact_test.js +++ b/test/jasmine/tests/gl3d_plot_interact_test.js @@ -571,7 +571,7 @@ describe('Test gl3d plots', function() { }], layout: { scene: { - camera:{ + camera: { eye: {x: 1, y: 1, z: 0}, center: {x: 0.5, y: 0.5, z: 1}, up: {x: 0, y: 0, z: 1} From 8b670e6858054071062e81ef04f1c1e78cfab6ee Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com> Date: Thu, 8 Nov 2018 22:11:10 -0500 Subject: [PATCH 7/7] removed -Infinity case of absolute value --- src/plots/gl3d/layout/tick_marks.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plots/gl3d/layout/tick_marks.js b/src/plots/gl3d/layout/tick_marks.js index 21d01c831e3..8999e4886c0 100644 --- a/src/plots/gl3d/layout/tick_marks.js +++ b/src/plots/gl3d/layout/tick_marks.js @@ -48,7 +48,6 @@ function computeTickMarks(scene) { glRange[i].pixelsPerDataUnit / scene.dataScale[i]; if(Math.abs(axes._length) === Infinity || - Math.abs(axes._length) === -Infinity || isNaN(axes._length)) { ticks[i] = []; } else {