Skip to content

Commit

Permalink
Fix legend item layout issue (#5816)
Browse files Browse the repository at this point in the history
  • Loading branch information
nagix authored and simonbrunel committed Nov 12, 2018
1 parent 9140ef7 commit e730f87
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 14 deletions.
23 changes: 13 additions & 10 deletions src/plugins/plugin.legend.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ var Legend = Element.extend({

// Width of each line of legend boxes. Labels wrap onto multiple lines when there are too many to fit on one
var lineWidths = me.lineWidths = [0];
var totalHeight = me.legendItems.length ? fontSize + (labelOpts.padding) : 0;
var totalHeight = 0;

ctx.textAlign = 'left';
ctx.textBaseline = 'top';
Expand All @@ -250,9 +250,9 @@ var Legend = Element.extend({
var boxWidth = getBoxWidth(labelOpts, fontSize);
var width = boxWidth + (fontSize / 2) + ctx.measureText(legendItem.text).width;

if (lineWidths[lineWidths.length - 1] + width + labelOpts.padding >= me.width) {
totalHeight += fontSize + (labelOpts.padding);
lineWidths[lineWidths.length] = me.left;
if (i === 0 || lineWidths[lineWidths.length - 1] + width + labelOpts.padding > minSize.width) {
totalHeight += fontSize + labelOpts.padding;
lineWidths[lineWidths.length - (i > 0 ? 0 : 1)] = labelOpts.padding;
}

// Store the hitbox width and height here. Final position will be updated in `draw`
Expand Down Expand Up @@ -281,7 +281,7 @@ var Legend = Element.extend({
var itemWidth = boxWidth + (fontSize / 2) + ctx.measureText(legendItem.text).width;

// If too tall, go to new column
if (currentColHeight + itemHeight > minSize.height) {
if (i > 0 && currentColHeight + itemHeight > minSize.height - vPadding) {
totalWidth += currentColWidth + labelOpts.padding;
columnWidths.push(currentColWidth); // previous column width

Expand Down Expand Up @@ -412,7 +412,7 @@ var Legend = Element.extend({
var isHorizontal = me.isHorizontal();
if (isHorizontal) {
cursor = {
x: me.left + ((legendWidth - lineWidths[0]) / 2),
x: me.left + ((legendWidth - lineWidths[0]) / 2) + labelOpts.padding,
y: me.top + labelOpts.padding,
line: 0
};
Expand All @@ -431,13 +431,16 @@ var Legend = Element.extend({
var x = cursor.x;
var y = cursor.y;

// Use (me.left + me.minSize.width) and (me.top + me.minSize.height)
// instead of me.right and me.bottom because me.width and me.height
// may have been changed since me.minSize was calculated
if (isHorizontal) {
if (x + width >= legendWidth) {
if (i > 0 && x + width + labelOpts.padding > me.left + me.minSize.width) {
y = cursor.y += itemHeight;
cursor.line++;
x = cursor.x = me.left + ((legendWidth - lineWidths[cursor.line]) / 2);
x = cursor.x = me.left + ((legendWidth - lineWidths[cursor.line]) / 2) + labelOpts.padding;
}
} else if (y + itemHeight > me.bottom) {
} else if (i > 0 && y + itemHeight > me.top + me.minSize.height) {
x = cursor.x = x + me.columnWidths[cursor.line] + labelOpts.padding;
y = cursor.y = me.top + labelOpts.padding;
cursor.line++;
Expand All @@ -452,7 +455,7 @@ var Legend = Element.extend({
fillText(x, y, legendItem, textWidth);

if (isHorizontal) {
cursor.x += width + (labelOpts.padding);
cursor.x += width + labelOpts.padding;
} else {
cursor.y += itemHeight;
}
Expand Down
144 changes: 140 additions & 4 deletions test/specs/plugin.legend.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('Legend block tests', function() {
expect(makeChart).not.toThrow();
});

it('should draw correctly', function() {
it('should draw correctly when the legend is positioned on the top', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
Expand Down Expand Up @@ -205,9 +205,9 @@ describe('Legend block tests', function() {
expect(chart.legend.legendHitBoxes.length).toBe(3);

[
{h: 12, l: 101, t: 10, w: 93},
{h: 12, l: 205, t: 10, w: 93},
{h: 12, l: 308, t: 10, w: 93}
{h: 12, l: 107, t: 10, w: 93},
{h: 12, l: 210, t: 10, w: 93},
{h: 12, l: 312, t: 10, w: 93}
].forEach(function(expected, i) {
expect(chart.legend.legendHitBoxes[i].height).toBeCloseToPixel(expected.h);
expect(chart.legend.legendHitBoxes[i].left).toBeCloseToPixel(expected.l);
Expand Down Expand Up @@ -389,6 +389,142 @@ describe('Legend block tests', function() {
}]);*/
});

it('should draw correctly when the legend is positioned on the left', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [{
label: 'dataset1',
backgroundColor: '#f31',
borderCapStyle: 'butt',
borderDash: [2, 2],
borderDashOffset: 5.5,
data: []
}, {
label: 'dataset2',
hidden: true,
borderJoinStyle: 'miter',
data: []
}, {
label: 'dataset3',
borderWidth: 10,
borderColor: 'green',
data: []
}],
labels: []
},
options: {
legend: {
position: 'left'
}
}
});

expect(chart.legend.legendHitBoxes.length).toBe(3);

[
{h: 12, l: 10, t: 16, w: 93},
{h: 12, l: 10, t: 38, w: 93},
{h: 12, l: 10, t: 60, w: 93}
].forEach(function(expected, i) {
expect(chart.legend.legendHitBoxes[i].height).toBeCloseToPixel(expected.h);
expect(chart.legend.legendHitBoxes[i].left).toBeCloseToPixel(expected.l);
expect(chart.legend.legendHitBoxes[i].top).toBeCloseToPixel(expected.t);
expect(chart.legend.legendHitBoxes[i].width).toBeCloseToPixel(expected.w);
});
});

it('should draw correctly when the legend is positioned on the top and has multiple rows', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [1, 2, 3, 4, 5].map(function(n) {
return {
label: 'dataset' + n,
data: []
};
}),
labels: []
}
});

expect(chart.legend.left).toBeCloseToPixel(0);
expect(chart.legend.top).toBeCloseToPixel(0);
expect(chart.legend.width).toBeCloseToPixel(512);
expect(chart.legend.height).toBeCloseToPixel(54);
expect(chart.legend.legendHitBoxes.length).toBe(5);
expect(chart.legend.legendHitBoxes.length).toBe(5);

[
{h: 12, l: 56, t: 10, w: 93},
{h: 12, l: 158, t: 10, w: 93},
{h: 12, l: 261, t: 10, w: 93},
{h: 12, l: 364, t: 10, w: 93},
{h: 12, l: 210, t: 32, w: 93}
].forEach(function(expected, i) {
expect(chart.legend.legendHitBoxes[i].height).toBeCloseToPixel(expected.h);
expect(chart.legend.legendHitBoxes[i].left).toBeCloseToPixel(expected.l);
expect(chart.legend.legendHitBoxes[i].top).toBeCloseToPixel(expected.t);
expect(chart.legend.legendHitBoxes[i].width).toBeCloseToPixel(expected.w);
});
});

it('should draw correctly when the legend is positioned on the left and has multiple columns', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22].map(function(n) {
return {
label: 'dataset' + n,
data: []
};
}),
labels: []
},
options: {
legend: {
position: 'left'
}
}
});

expect(chart.legend.left).toBeCloseToPixel(0);
expect(chart.legend.top).toBeCloseToPixel(6);
expect(chart.legend.width).toBeCloseToPixel(228.7);
expect(chart.legend.height).toBeCloseToPixel(478);
expect(chart.legend.legendHitBoxes.length).toBe(22);

[
{h: 12, l: 10, t: 16, w: 93},
{h: 12, l: 10, t: 38, w: 93},
{h: 12, l: 10, t: 60, w: 93},
{h: 12, l: 10, t: 82, w: 93},
{h: 12, l: 10, t: 104, w: 93},
{h: 12, l: 10, t: 126, w: 93},
{h: 12, l: 10, t: 148, w: 93},
{h: 12, l: 10, t: 170, w: 93},
{h: 12, l: 10, t: 192, w: 93},
{h: 12, l: 10, t: 214, w: 99},
{h: 12, l: 10, t: 236, w: 99},
{h: 12, l: 10, t: 258, w: 99},
{h: 12, l: 10, t: 280, w: 99},
{h: 12, l: 10, t: 302, w: 99},
{h: 12, l: 10, t: 324, w: 99},
{h: 12, l: 10, t: 346, w: 99},
{h: 12, l: 10, t: 368, w: 99},
{h: 12, l: 10, t: 390, w: 99},
{h: 12, l: 10, t: 412, w: 99},
{h: 12, l: 10, t: 434, w: 99},
{h: 12, l: 10, t: 456, w: 99},
{h: 12, l: 119, t: 16, w: 99}
].forEach(function(expected, i) {
expect(chart.legend.legendHitBoxes[i].height).toBeCloseToPixel(expected.h);
expect(chart.legend.legendHitBoxes[i].left).toBeCloseToPixel(expected.l);
expect(chart.legend.legendHitBoxes[i].top).toBeCloseToPixel(expected.t);
expect(chart.legend.legendHitBoxes[i].width).toBeCloseToPixel(expected.w);
});
});

describe('config update', function() {
it ('should update the options', function() {
var chart = acquireChart({
Expand Down

0 comments on commit e730f87

Please sign in to comment.