Skip to content

Commit

Permalink
Addressing 2nd round of review feedback:
Browse files Browse the repository at this point in the history
- adding colour and removing borders from image tests for legend blocks.
- corrected/adjusted all images in image tests
- changed default align for vertical positioned legends to 'center'
- refactored alignment offset function
- removed superfluous 'line' variable
- made consistent the check for moving to a new line/column between vertical/horizontal fit algorithm
  • Loading branch information
Dave Kichler committed Mar 20, 2019
1 parent 4081897 commit 01f2876
Show file tree
Hide file tree
Showing 22 changed files with 41 additions and 25 deletions.
4 changes: 2 additions & 2 deletions docs/configuration/legend.md
Expand Up @@ -26,8 +26,8 @@ Position of the legend. Options are:

## Align
Alignment of the legend. Options are:
* `'start'` (default for left/right positioned legends)
* `'center'` (default for top/bottom positioned legends)
* `'start'`
* `'center'`
* `'end'`

Defaults to `'center'` for unrecognized values.
Expand Down
25 changes: 11 additions & 14 deletions src/plugins/plugin.legend.js
Expand Up @@ -12,6 +12,7 @@ defaults._set('global', {
legend: {
display: true,
position: 'top',
align: 'center',
fullWidth: true,
reverse: false,
weight: 1000,
Expand Down Expand Up @@ -105,10 +106,6 @@ var Legend = Element.extend({
var me = this;
helpers.extend(me, config);

if (me.options && !me.align) {
// to maintain backward compatibility with existing default
me.options.align = me.isHorizontal() ? 'center' : 'start';
}
// Contains hit boxes for each dataset (in dataset order)
me.legendHitBoxes = [];

Expand Down Expand Up @@ -258,7 +255,7 @@ var Legend = Element.extend({
var boxWidth = getBoxWidth(labelOpts, fontSize);
var width = boxWidth + (fontSize / 2) + ctx.measureText(legendItem.text).width;

if (i === 0 || lineWidths[lineWidths.length - 1] + width + labelOpts.padding > minSize.width) {
if (i === 0 || lineWidths[lineWidths.length - 1] + width + labelOpts.padding > minSize.width - labelOpts.padding) {
totalHeight += fontSize + labelOpts.padding;
lineWidths[lineWidths.length - (i > 0 ? 0 : 1)] = 0;
}
Expand Down Expand Up @@ -418,29 +415,29 @@ var Legend = Element.extend({
};

var alignmentOffset = function(dimension, blockSize) {
if (opts.align === 'start') {
switch (opts.align) {
case 'start':
return labelOpts.padding;
} else if (opts.align === 'end') {
case 'end':
return dimension - blockSize;
default: // center
return (dimension - blockSize + labelOpts.padding) / 2;
}
// default to center
return (dimension - blockSize + labelOpts.padding) / 2;
};

// Horizontal
var isHorizontal = me.isHorizontal();
var line = 0;
if (isHorizontal) {
cursor = {
x: me.left + alignmentOffset(legendWidth, lineWidths[line]),
x: me.left + alignmentOffset(legendWidth, lineWidths[0]),
y: me.top + labelOpts.padding,
line: line
line: 0
};
} else {
cursor = {
x: me.left + labelOpts.padding,
y: me.top + alignmentOffset(legendHeight, columnHeights[line]),
line: line
y: me.top + alignmentOffset(legendHeight, columnHeights[0]),
line: 0
};
}

Expand Down
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30]
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": [""],
"datasets": [{
"data": [10]
"data": [10],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50]
"data": [10, 20, 30, 40, 50],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30]
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30]
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 20, 10]
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 20, 10],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": [""],
"datasets": [{
"data": [10]
"data": [10],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10]
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Expand Up @@ -4,7 +4,9 @@
"data": {
"labels": ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""],
"datasets": [{
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10]
"data": [10, 20, 30, 40, 50, 60, 70, 10, 20, 30, 40, 50, 60, 70, 10],
"backgroundColor": "#00ff00",
"borderWidth": 0
}]
},
"options": {
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions test/specs/plugin.legend.tests.js
Expand Up @@ -6,6 +6,7 @@ describe('Legend block tests', function() {
expect(Chart.defaults.global.legend).toEqual({
display: true,
position: 'top',
align: 'center',
fullWidth: true, // marks that this box should take the full width of the canvas (pushing down other boxes)
reverse: false,
weight: 1000,
Expand Down

0 comments on commit 01f2876

Please sign in to comment.