From 9eb6bc4f5472527886593b17785bf87ecaafbf16 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Tue, 26 Dec 2023 23:53:55 -0300 Subject: [PATCH 1/9] Add switch to specify major/minor in arrayTicks --- src/plots/cartesian/axes.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index dfdb0e5166d..5b1d92a886f 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -949,10 +949,10 @@ axes.calcTicks = function calcTicks(ax, opts) { if(mockAx.tickmode === 'array') { if(major) { tickVals = []; - ticksOut = arrayTicks(ax); + ticksOut = arrayTicks(ax, major); } else { minorTickVals = []; - minorTicks = arrayTicks(ax); + minorTicks = arrayTicks(ax, false); } continue; } @@ -1261,7 +1261,7 @@ function syncTicks(ax) { return ticksOut; } -function arrayTicks(ax) { +function arrayTicks(ax, major) { var rng = Lib.simpleMap(ax.range, ax.r2l); var exRng = expandRange(rng); var tickMin = Math.min(exRng[0], exRng[1]); @@ -1279,7 +1279,8 @@ function arrayTicks(ax) { var ticksOut = []; for(var isMinor = 0; isMinor <= 1; isMinor++) { - if(isMinor && !ax.minor) continue; + if(!isMinor && !major) continue; + if(isMinor && (!ax.minor || major)) continue; var vals = !isMinor ? ax.tickvals : ax.minor.tickvals; var text = !isMinor ? ax.ticktext : []; From 9a6de432024296f3990ddd1250d7376fb801bf4e Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Tue, 2 Jan 2024 18:29:50 -0500 Subject: [PATCH 2/9] Add test to verify expected quantity of ticks --- src/plots/cartesian/axes.js | 1 + test/jasmine/tests/axes_test.js | 63 ++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 5b1d92a886f..f4645b8ee03 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1262,6 +1262,7 @@ function syncTicks(ax) { } function arrayTicks(ax, major) { + if (major === undefined) throw new Error("arrayTicks must specify ticktype") var rng = Lib.simpleMap(ax.range, ax.r2l); var exRng = expandRange(rng); var tickMin = Math.min(exRng[0], exRng[1]); diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index d128072a307..76b4a8e6e74 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -28,7 +28,6 @@ var supplyDefaults = require('../assets/supply_defaults'); describe('Test axes', function() { 'use strict'; - describe('swap', function() { it('should swap most attributes and fix placeholder titles', function() { var gd = { @@ -8217,3 +8216,65 @@ describe('shift tests', function() { }); }); }); +describe('test tickmode calculator', function() { + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + function generateTickConfig(){ + standardConfig = {tickmode: 'array', ticks: 'inside', ticklen: 1, showticklabels: false}; + + // Number of ticks will be random + var n = Math.floor(Math.random() * 99) + 1; + tickVals = []; + for(let i = 0; i <= n; i++) tickVals.push(i); + standardConfig['tickvals'] = tickVals; + standardConfig['ticktext'] = tickVals; + return standardConfig; + } + var ticksOff = {tickmode:"array", ticks: '', tickvals:[], ticktext:[], ticklen: 0, showticklabels: false}; + // the goal is target the arrayTicks() function, the subject of PR: https://github.com/plotly/plotly.js/pull/6829 + // we test xMajor and xMinor in on/on on/off off/on and off/off + // since we can't unit test functions that are not exported, we shim functions we don't care about instead and + // test the nearest exported functions (calcTicks) + describe('arrayTicks', function() { + for(let i = 0; i < 4; i++) { + (function(i) { + it('should return the specified correct number of major ticks and minor ticks', function() { + const BOTH = 0; + const MAJOR = 1; + const MINOR = 2; + const NEITHER = 3; + var xMajorConfig = ticksOff; + var xMinorConfig = ticksOff; + if(i == BOTH) { + xMajorConfig = generateTickConfig(); + xMinorConfig = generateTickConfig(); + } else if(i == MAJOR) { + xMajorConfig = generateTickConfig(); + } else if(i==MINOR) { + xMinorConfig = generateTickConfig(); + } else if(i == NEITHER) { + // Do nothing, all ticks off + } + xaxis = { + r2l: Lib.cleanNumber, + d2l: Lib.cleanNumber, + _separators: ',', + range: [0, 1000], + ...xMajorConfig, + minor: { + ...xMinorConfig, + }, + }; + Axes.prepMinorTicks = function() { return }; // Not part of this test + Axes.prepTicks = function() { return }; // Not part of this test + ticksOut = Axes.calcTicks(xaxis); + expect(ticksOut.length).toEqual(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); + }); + })(i); + } + }); +}); From 392b10301157a0498e40aba2a2900eee509d5ce5 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Tue, 2 Jan 2024 18:35:30 -0500 Subject: [PATCH 3/9] Add draftlog for PR #6829 --- draftlogs/6829_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/6829_fix.md diff --git a/draftlogs/6829_fix.md b/draftlogs/6829_fix.md new file mode 100644 index 00000000000..abba571afa7 --- /dev/null +++ b/draftlogs/6829_fix.md @@ -0,0 +1 @@ +- Add argument to `arrayTicks()` to render major OR minor [[#6829](https://github.com/plotly/plotly.js/pull/6829)] From 41ae65d2ca2175963ee2fac1ae34c3cc84e44028 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 3 Jan 2024 13:36:42 -0500 Subject: [PATCH 4/9] Fix linter errors --- 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 f4645b8ee03..47ad366c58c 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1262,7 +1262,7 @@ function syncTicks(ax) { } function arrayTicks(ax, major) { - if (major === undefined) throw new Error("arrayTicks must specify ticktype") + if(major === undefined) throw new Error('arrayTicks must specify ticktype'); var rng = Lib.simpleMap(ax.range, ax.r2l); var exRng = expandRange(rng); var tickMin = Math.min(exRng[0], exRng[1]); From 095ce0c4c9a29053a5117418312aa47a5fd60b8a Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 3 Jan 2024 14:08:12 -0500 Subject: [PATCH 5/9] Set tick test to use fullLayout, not mock x --- test/jasmine/tests/axes_test.js | 64 ++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index 76b4a8e6e74..2ae5ee3d904 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -8216,7 +8216,9 @@ describe('shift tests', function() { }); }); }); -describe('test tickmode calculator', function() { +fdescribe('test tickmode calculator', function() { + var gd; + beforeEach(function() { gd = createGraphDiv(); }); @@ -8229,20 +8231,31 @@ describe('test tickmode calculator', function() { // Number of ticks will be random var n = Math.floor(Math.random() * 99) + 1; tickVals = []; - for(let i = 0; i <= n; i++) tickVals.push(i); + for(let i = 0; i <= n; i++) { + tickVals.push(i); + } standardConfig['tickvals'] = tickVals; standardConfig['ticktext'] = tickVals; return standardConfig; } var ticksOff = {tickmode:"array", ticks: '', tickvals:[], ticktext:[], ticklen: 0, showticklabels: false}; - // the goal is target the arrayTicks() function, the subject of PR: https://github.com/plotly/plotly.js/pull/6829 - // we test xMajor and xMinor in on/on on/off off/on and off/off - // since we can't unit test functions that are not exported, we shim functions we don't care about instead and - // test the nearest exported functions (calcTicks) + + function _assert(expLength) { + var ax = gd._fullLayout.xaxis; + + // all positions + var positions = + ax._vals + .filter(function(d) { return d; }) + .map(function(d) { return d.x; }); + + expect(positions.length).toEqual(expLength); + } + describe('arrayTicks', function() { for(let i = 0; i < 4; i++) { (function(i) { - it('should return the specified correct number of major ticks and minor ticks', function() { + it('should return the specified correct number of major ticks and minor ticks', function(done) { const BOTH = 0; const MAJOR = 1; const MINOR = 2; @@ -8259,20 +8272,29 @@ describe('test tickmode calculator', function() { } else if(i == NEITHER) { // Do nothing, all ticks off } - xaxis = { - r2l: Lib.cleanNumber, - d2l: Lib.cleanNumber, - _separators: ',', - range: [0, 1000], - ...xMajorConfig, - minor: { - ...xMinorConfig, - }, - }; - Axes.prepMinorTicks = function() { return }; // Not part of this test - Axes.prepTicks = function() { return }; // Not part of this test - ticksOut = Axes.calcTicks(xaxis); - expect(ticksOut.length).toEqual(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); + Plotly.newPlot(gd, { + data: [{ + x: [0, 1], + y: [0, 1] + }], + layout: { + width: 400, + height: 400, + margin: { + t: 40, + b: 40, + l: 40, + r: 40 + }, + xaxis: { + range: [0, 1000], + ...xMajorConfig, + minor: xMinorConfig, + }, + } + }).then(function() { + _assert(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); + }).then(done, done.fail); }); })(i); } From 69345fcd0d01af79414d5317f0f68c4fa26b6819 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 3 Jan 2024 14:17:38 -0500 Subject: [PATCH 6/9] Eliminate unnecesary closure in parameterized test --- test/jasmine/tests/axes_test.js | 86 ++++++++++++++++----------------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index 2ae5ee3d904..ec12f165d18 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -8253,50 +8253,48 @@ fdescribe('test tickmode calculator', function() { } describe('arrayTicks', function() { - for(let i = 0; i < 4; i++) { - (function(i) { - it('should return the specified correct number of major ticks and minor ticks', function(done) { - const BOTH = 0; - const MAJOR = 1; - const MINOR = 2; - const NEITHER = 3; - var xMajorConfig = ticksOff; - var xMinorConfig = ticksOff; - if(i == BOTH) { - xMajorConfig = generateTickConfig(); - xMinorConfig = generateTickConfig(); - } else if(i == MAJOR) { - xMajorConfig = generateTickConfig(); - } else if(i==MINOR) { - xMinorConfig = generateTickConfig(); - } else if(i == NEITHER) { - // Do nothing, all ticks off + it('should return the specified correct number of major ticks and minor ticks', function(done) { + for(let i = 0; i < 4; i++) { + const BOTH = 0; + const MAJOR = 1; + const MINOR = 2; + const NEITHER = 3; + var xMajorConfig = ticksOff; + var xMinorConfig = ticksOff; + if(i == BOTH) { + xMajorConfig = generateTickConfig(); + xMinorConfig = generateTickConfig(); + } else if(i == MAJOR) { + xMajorConfig = generateTickConfig(); + } else if(i==MINOR) { + xMinorConfig = generateTickConfig(); + } else if(i == NEITHER) { + // Do nothing, all ticks off + } + Plotly.newPlot(gd, { + data: [{ + x: [0, 1], + y: [0, 1] + }], + layout: { + width: 400, + height: 400, + margin: { + t: 40, + b: 40, + l: 40, + r: 40 + }, + xaxis: { + range: [0, 1000], + ...xMajorConfig, + minor: xMinorConfig, + }, } - Plotly.newPlot(gd, { - data: [{ - x: [0, 1], - y: [0, 1] - }], - layout: { - width: 400, - height: 400, - margin: { - t: 40, - b: 40, - l: 40, - r: 40 - }, - xaxis: { - range: [0, 1000], - ...xMajorConfig, - minor: xMinorConfig, - }, - } - }).then(function() { - _assert(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); - }).then(done, done.fail); - }); - })(i); - } + }).then(function() { + _assert(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); + }).then(done, done.fail); + } + }); }); }); From 365231d4c81dd970f563e49afbcc8be191809822 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 3 Jan 2024 14:26:59 -0500 Subject: [PATCH 7/9] Use Lib.psuedoRandom(), not Math.random() --- test/jasmine/tests/axes_test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index ec12f165d18..ed1ac7c2b7a 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -8229,7 +8229,8 @@ fdescribe('test tickmode calculator', function() { standardConfig = {tickmode: 'array', ticks: 'inside', ticklen: 1, showticklabels: false}; // Number of ticks will be random - var n = Math.floor(Math.random() * 99) + 1; + Lib.seedPseudoRandom(); + var n = (Lib.pseudoRandom() * 99) + 1; tickVals = []; for(let i = 0; i <= n; i++) { tickVals.push(i); From cd3b261e8d4dfaa52ded2197d8d8c48e04d00e41 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 3 Jan 2024 16:41:41 -0500 Subject: [PATCH 8/9] Remove closure function from tickArray test --- test/jasmine/tests/axes_test.js | 96 ++++++++++++++------------------- 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index ed1ac7c2b7a..9f1dff70617 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -8216,7 +8216,7 @@ describe('shift tests', function() { }); }); }); -fdescribe('test tickmode calculator', function() { +describe('test tickmode calculator', function() { var gd; beforeEach(function() { @@ -8225,21 +8225,21 @@ fdescribe('test tickmode calculator', function() { afterEach(destroyGraphDiv); - function generateTickConfig(){ - standardConfig = {tickmode: 'array', ticks: 'inside', ticklen: 1, showticklabels: false}; - - // Number of ticks will be random - Lib.seedPseudoRandom(); - var n = (Lib.pseudoRandom() * 99) + 1; - tickVals = []; - for(let i = 0; i <= n; i++) { - tickVals.push(i); - } - standardConfig['tickvals'] = tickVals; - standardConfig['ticktext'] = tickVals; - return standardConfig; + function generateTickConfig() { + var standardConfig = {tickmode: 'array', ticks: 'inside', ticklen: 1, showticklabels: false}; + + // Number of ticks will be random + Lib.seedPseudoRandom(); + var n = (Lib.pseudoRandom() * 99) + 1; + var tickVals = []; + for(var i = 0; i <= n; i++) { + tickVals.push(i); + } + standardConfig.tickvals = tickVals; + standardConfig.ticktext = tickVals; + return standardConfig; } - var ticksOff = {tickmode:"array", ticks: '', tickvals:[], ticktext:[], ticklen: 0, showticklabels: false}; + var ticksOff = {tickmode: 'array', ticks: '', tickvals: [], ticktext: [], ticklen: 0, showticklabels: false}; function _assert(expLength) { var ax = gd._fullLayout.xaxis; @@ -8255,47 +8255,31 @@ fdescribe('test tickmode calculator', function() { describe('arrayTicks', function() { it('should return the specified correct number of major ticks and minor ticks', function(done) { - for(let i = 0; i < 4; i++) { - const BOTH = 0; - const MAJOR = 1; - const MINOR = 2; - const NEITHER = 3; - var xMajorConfig = ticksOff; - var xMinorConfig = ticksOff; - if(i == BOTH) { - xMajorConfig = generateTickConfig(); - xMinorConfig = generateTickConfig(); - } else if(i == MAJOR) { - xMajorConfig = generateTickConfig(); - } else if(i==MINOR) { - xMinorConfig = generateTickConfig(); - } else if(i == NEITHER) { - // Do nothing, all ticks off - } - Plotly.newPlot(gd, { - data: [{ - x: [0, 1], - y: [0, 1] - }], - layout: { - width: 400, - height: 400, - margin: { - t: 40, - b: 40, - l: 40, - r: 40 - }, - xaxis: { - range: [0, 1000], - ...xMajorConfig, - minor: xMinorConfig, - }, - } - }).then(function() { - _assert(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); - }).then(done, done.fail); - } + var xMajorConfig = ticksOff; + var xMinorConfig = ticksOff; + xMajorConfig = generateTickConfig(); + xMinorConfig = generateTickConfig(); + xMajorConfig.range = [0, 1000]; + xMajorConfig.minor = xMinorConfig; + Plotly.newPlot(gd, { + data: [{ + x: [0, 1], + y: [0, 1] + }], + layout: { + width: 400, + height: 400, + margin: { + t: 40, + b: 40, + l: 40, + r: 40 + }, + xaxis: xMajorConfig, + } + }).then(function() { + _assert(xMajorConfig.tickvals.length + xMinorConfig.tickvals.length); + }).then(done, done.fail); }); }); }); From 1051bb7f89bfad160238e9f9ca43614d69cf7900 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 3 Jan 2024 17:07:57 -0500 Subject: [PATCH 9/9] Add legacy mode to arrayTicks() --- src/plots/cartesian/axes.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 47ad366c58c..8368e1c5398 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -949,10 +949,10 @@ axes.calcTicks = function calcTicks(ax, opts) { if(mockAx.tickmode === 'array') { if(major) { tickVals = []; - ticksOut = arrayTicks(ax, major); + ticksOut = arrayTicks(ax, !isMinor); } else { minorTickVals = []; - minorTicks = arrayTicks(ax, false); + minorTicks = arrayTicks(ax, !isMinor); } continue; } @@ -1261,8 +1261,7 @@ function syncTicks(ax) { return ticksOut; } -function arrayTicks(ax, major) { - if(major === undefined) throw new Error('arrayTicks must specify ticktype'); +function arrayTicks(ax, majorOnly) { var rng = Lib.simpleMap(ax.range, ax.r2l); var exRng = expandRange(rng); var tickMin = Math.min(exRng[0], exRng[1]); @@ -1280,11 +1279,10 @@ function arrayTicks(ax, major) { var ticksOut = []; for(var isMinor = 0; isMinor <= 1; isMinor++) { - if(!isMinor && !major) continue; - if(isMinor && (!ax.minor || major)) continue; + if((majorOnly !== undefined) && ((majorOnly && isMinor) || (majorOnly === false && !isMinor))) continue; + if(isMinor && !ax.minor) continue; var vals = !isMinor ? ax.tickvals : ax.minor.tickvals; var text = !isMinor ? ax.ticktext : []; - if(!vals) continue;