Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/7950_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix box, violin, candlestick and ohlc traces losing their forced minimum tick spacing when a graph div is updated in place with `Plotly.react` or `Plotly.restyle` [[#7950](https://github.com/plotly/plotly.js/pull/7950)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how ohlc traces are affected by this. Can you produce a test that confirms this?

12 changes: 7 additions & 5 deletions src/plots/cartesian/set_convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function isValidCategory(v) {
* Creates/updates these conversion functions, and a few more utilities
* like cleanRange, and makeCalcdata
*
* also clears ._minDtick, ._forceTick0
* also creates ax.clearCalc, which clears ._minDtick, ._forceTick0
*/
module.exports = function setConvert(ax, fullLayout) {
fullLayout = fullLayout || {};
Expand Down Expand Up @@ -952,6 +952,12 @@ module.exports = function setConvert(ax, fullLayout) {

// should skip if not category nor multicategory
ax.clearCalc = function() {
// for bar charts and box plots: reset forced minimum tick spacing.
// this has to happen here rather than in setConvert, as the values
// are relinked onto the new fullLayout after supplyDefaults runs
delete ax._minDtick;
delete ax._forceTick0;

var group = ax._matchGroup;
if(group) {
var categories = null;
Expand Down Expand Up @@ -1024,8 +1030,4 @@ module.exports = function setConvert(ax, fullLayout) {
// even though it won't be needed by this axis
ax._separators = fullLayout.separators;
ax._numFormat = locale ? locale.numberFormat : numberFormat;

// and for bar charts and box plots: reset forced minimum tick spacing
delete ax._minDtick;
delete ax._forceTick0;
};
39 changes: 39 additions & 0 deletions test/jasmine/tests/axes_test.js

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test here covers scatter+box via react but I'd like to see some additional tests to guard against regressions:

  • a test that uses restyle to reproduce the bug, as mentioned in the changelog
  • a test that uses a box then react to an image

Original file line number Diff line number Diff line change
Expand Up @@ -8303,6 +8303,45 @@ describe('more react tests', function() {
expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.173, 2]);
}).then(done, done.fail);
});

it('should not carry over the forced minimum tick spacing of the previous figure', function(done) {
var layout = {width: 700, height: 400};

var scatterFig = {
data: [{y: [1, 2, 3]}],
layout: layout
};

// one box per integer position - each box forces a tick of its own
var boxFig = {
data: [{
type: 'box',
x: [1, 1, 2, 2, 3, 3],
y: [1, 2, 3, 4, 5, 6]
}],
layout: layout
};

function getXLabels() {
return gd._fullLayout.xaxis._vals.map(function(d) { return d.text; });
}

Plotly.newPlot(gd, boxFig)
.then(function() {
expect(getXLabels()).toEqual(['1', '2', '3']);

// scatter cancels the forcing for its own figure only
return Plotly.newPlot(gd, scatterFig);
})
.then(function() {
return Plotly.react(gd, boxFig);
})
.then(function() {
expect(gd._fullLayout.xaxis._minDtick).toBe(1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a useful assertion - it's testing internal details when we actually care about the outcome, which the next line asserts on.

Suggested change
expect(gd._fullLayout.xaxis._minDtick).toBe(1);

expect(getXLabels()).toEqual(['1', '2', '3']);
})
.then(done, done.fail);
});
});

describe('category preservation tests on gd passed to Plotly.react()', function() {
Expand Down
3 changes: 2 additions & 1 deletion test/jasmine/tests/plot_api_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,8 @@ describe('Test plot api', function () {
return Plotly.restyle(gd, { x0: 12.3 });
})
.then(function () {
checkTicks('x', ['12', '12.5'], 'switched to numeric');
// a single box forces one tick at its own position
checkTicks('x', ['12.3'], 'switched to numeric');
expect(gd._fullLayout.xaxis.type).toBe('linear');
})
.then(done, done.fail);
Expand Down
Loading