Skip to content
Merged
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/7908_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix crash ("Something went wrong with axis scaling") when a colorbar's title or padding leaves it with a negative domain length [[#7908](https://github.com/plotly/plotly.js/pull/7908)], with thanks to @zeehio for the contribution!
21 changes: 11 additions & 10 deletions src/components/colorbar/draw.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.

What do you think of creating helper functions to handle this and using those on each affected line? Something like this:

const insetDomainStart = (domain, delta) => [Math.min(domain[0] + delta, domain[1]), domain[1]];
const insetDomainEnd = (domain, delta) => [domain[0], Math.max(domain[1] - delta, domain[0])];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ function makeColorBarData(gd) {
return out;
}

// Move domain[0] toward domain[1] by delta, without crossing it
const insetDomainStart = (domain, delta) => [Math.min(domain[0] + delta, domain[1]), domain[1]];

// Move domain[1] toward domain[0] by delta, without crossing it
const insetDomainEnd = (domain, delta) => [domain[0], Math.max(domain[1] - delta, domain[0])];

function drawColorBar(g, opts, gd) {
var isVertical = opts.orientation === 'v';
var len = opts.len;
Expand Down Expand Up @@ -300,13 +306,8 @@ function drawColorBar(g, opts, gd) {

// set domain after init, because we may want to
// allow it outside [0,1]
ax.domain = isVertical ? [
vFrac + ypad / gs.h,
vFrac + lenFrac - ypad / gs.h
] : [
vFrac + xpad / gs.w,
vFrac + lenFrac - xpad / gs.w
];
var padFrac = isVertical ? ypad / gs.h : xpad / gs.w;
ax.domain = insetDomainEnd(insetDomainStart([vFrac, vFrac + lenFrac], padFrac), padFrac);

ax.setScale();

Expand Down Expand Up @@ -473,10 +474,10 @@ function drawColorBar(g, opts, gd) {
titleHeight += 5;

if(titleSide === 'top') {
ax.domain[1] -= titleHeight / gs.h;
ax.domain = insetDomainEnd(ax.domain, titleHeight / gs.h);
titleTrans[1] *= -1;
} else {
ax.domain[0] += titleHeight / gs.h;
ax.domain = insetDomainStart(ax.domain, titleHeight / gs.h);
var nlines = svgTextUtils.lineCount(titleText);
titleTrans[1] += (1 - nlines) * lineSize;
}
Expand All @@ -487,7 +488,7 @@ function drawColorBar(g, opts, gd) {
} else { // horizontal colorbars
if(titleWidth) {
if(titleSide === 'right') {
ax.domain[0] += (titleWidth + titleFontSize / 2) / gs.w;
ax.domain = insetDomainStart(ax.domain, (titleWidth + titleFontSize / 2) / gs.w);
}

titleGroup.attr('transform', strTranslate(titleTrans[0], titleTrans[1]));
Expand Down
70 changes: 70 additions & 0 deletions test/jasmine/tests/colorbar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,76 @@ describe('Test colorbar:', function() {
.then(done, done.fail);
});

// see https://github.com/plotly/plotly.js/issues/6499
it('does not throw when the colorbar is too short for its own length/padding', function(done) {
Plotly.newPlot(gd, [{
type: 'scatter',
mode: 'markers',
x: [1, 2, 3],
y: [1, 2, 3],
marker: {
color: [1, 2, 3],
colorbar: {len: 0.01, thickness: 30}
}
}], {width: 300, height: 120})
.then(done, done.fail);
});

it('does not throw when a top-side colorbar title is taller than the colorbar', function(done) {
Plotly.newPlot(gd, [{
type: 'scatter',
mode: 'markers',
x: [1, 2, 3],
y: [1, 2, 3],
marker: {
color: [1, 2, 3],
colorbar: {
title: {text: 'Long title', side: 'top', font: {size: 60}},
len: 0.3,
thickness: 30
}
}
}], {width: 300, height: 120})
.then(done, done.fail);
});

it('does not throw when a bottom-side colorbar title is taller than the colorbar', function(done) {
Plotly.newPlot(gd, [{
type: 'scatter',
mode: 'markers',
x: [1, 2, 3],
y: [1, 2, 3],
marker: {
color: [1, 2, 3],
colorbar: {
title: {text: 'Long title', side: 'bottom', font: {size: 60}},
len: 0.3,
thickness: 30
}
}
}], {width: 300, height: 120})
.then(done, done.fail);
});

it('does not throw when a right-side horizontal colorbar title is wider than the colorbar', function(done) {
Plotly.newPlot(gd, [{
type: 'scatter',
mode: 'markers',
x: [1, 2, 3],
y: [1, 2, 3],
marker: {
color: [1, 2, 3],
colorbar: {
orientation: 'h',
title: {text: 'Long title', side: 'right', font: {size: 60}},
len: 0.3,
thickness: 30
}
}
}], {width: 300, height: 120})
.then(done, done.fail);
});

function assertCB(msg, present, opts) {
var expandedMarginR = opts.expandedMarginR;
var expandedMarginT = opts.expandedMarginT;
Expand Down
Loading