Add switch to specify major/minor in axes.arrayTicks()
#6829
Conversation
|
Thanks very much for the PR. |
test/jasmine/tests/axes_test.js
Outdated
| function generateTickConfig(){ | ||
| standardConfig = {tickmode: 'array', ticks: 'inside', ticklen: 1, showticklabels: false}; | ||
|
|
||
| // Number of ticks will be random |
There was a problem hiding this comment.
Let's use Lib.pseudoRandom as well as Lib.seedPseudoRandom as it applied by other jasmine tests.
test/jasmine/tests/axes_test.js
Outdated
| // test the nearest exported functions (calcTicks) | ||
| describe('arrayTicks', function() { | ||
| for(let i = 0; i < 4; i++) { | ||
| (function(i) { |
There was a problem hiding this comment.
Please declare this function outside the loop and use ES5 to pass the syntax test.
src/plots/cartesian/axes.js
Outdated
| @@ -1261,7 +1261,8 @@ function syncTicks(ax) { | |||
| return ticksOut; | |||
| } | |||
|
|
|||
| function arrayTicks(ax) { | |||
| function arrayTicks(ax, major) { | |||
| if (major === undefined) throw new Error("arrayTicks must specify ticktype") | |||
There was a problem hiding this comment.
Do we really need this line?
After you added it in 9a6de43, some tests failed.
There was a problem hiding this comment.
That's good actually!
I put that line in there to force failure if other calls relied on default behavior.
Jasmine is wonky on my local box so I didn't catch those failed tests before push.
There was a problem hiding this comment.
The arrayTicks will now behave in a legacy manner if the caller isn't aware to use the major/minor switch.
test/jasmine/tests/axes_test.js
Outdated
| r2l: Lib.cleanNumber, | ||
| d2l: Lib.cleanNumber, | ||
| _separators: ',', | ||
| range: [0, 1000], |
There was a problem hiding this comment.
Instead of mocking the xaxis, can't we simply call Plotly.newPlot(...) and then inspect the gd._fullLayout.xaxis?
|
So, this should be about done. If I use Alex's strategy for implementing That said, I'm not too thrilled with Jasmine nor ES5, so I have started a small refactor fork: https://github.com/geopozo/plotly.js/tree/master?tab=readme-ov-file#refactor |
|
Thanks for all the effort on this pull. Thanks again. |
Long explanation in: #6828: but basically
calcTicks()tries and thinks its differentiating between major and minor ticks when doing some adjustments, but it is not actually. It ends up creating aticksOutthat contains each tick duplicated once instead.Actual changes simple. If this function is used outside of
axes.js, should probably make default behavior same as original behavior.I will do the draft log tomorrow.
Fixing this hidden error would solve all problems in my other draft pull request and allow
proportionaltickmode for minor ticks.