Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for issues #3917, #4380, #2966 - log scale with min: 0 #4913

Merged
merged 5 commits into from Nov 10, 2017
Merged

Fix for issues #3917, #4380, #2966 - log scale with min: 0 #4913

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b7421d
Fix for issues #3917, #4380, #2966
jcopperfield Nov 1, 2017
4de3227
Add: Requested code style changes for getPixelForValue and getValueFo…
jcopperfield Nov 6, 2017
165ad97
Fix: could not minify due to invalid function declaration
jcopperfield Nov 6, 2017
3519612
Fix: renamed variables to improve readability
jcopperfield Nov 6, 2017
c5e24a6
Fix: prefix private function with _ and correct jsdocs
jcopperfield Nov 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
115 changes: 73 additions & 42 deletions src/scales/scale.logarithmic.js
View file
Open in desktop
Expand Up @@ -177,64 +177,95 @@ module.exports = function(Chart) {
getPixelForTick: function(index) {
return this.getPixelForValue(this.tickValues[index]);
},
/**
* Returns the value of the first tick
*
* @Private
* @param {Number} [value] - The minimum not zero value.
* @return {Number} [firstTickValue] - The first tick value.
Copy link
Member

Choose a reason for hiding this comment

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

Should be @param {Number} value - The ... since value is not optional (http://usejsdoc.org/tags-param.html)
Should be @return {Number} The first ..., without name and separator (http://usejsdoc.org/tags-returns.html)
Can you also move @private at the end and only lowercase, and I would remove the extra empty line:

/**
 * Returns the value of the first tick.
 * @param {Number} value - The minimum not zero value.
 * @return {Number} The first tick value.
 * @private
 */
_getFirstTickValue: function(value) {

Similar to https://github.com/chartjs/Chart.js/blob/master/src/core/core.plugin.js#L92

Also we started to prefix private members by _ (for example), though it's not yet consistent.

*/
getFirstTickValue: function(value) {
var exp = Math.floor(helpers.log10(value));
var significand = Math.floor(value / Math.pow(10, exp));

return significand * Math.pow(10, exp);
},
getPixelForValue: function(value) {
var me = this;
var start = me.start;
var newVal = +me.getRightValue(value);
var opts = me.options;
var tickOpts = opts.ticks;
var innerDimension, pixel, range;
var reverse = me.options.ticks.reverse;
var log10 = helpers.log10;
var firstTickValue = me.getFirstTickValue(me.minNotZero);
var offset = 0;
var innerDimension, pixel, start, end, sign;

value = +me.getRightValue(value);
if (reverse) {
start = me.end;
end = me.start;
sign = -1;
} else {
start = me.start;
end = me.end;
sign = 1;
}
if (me.isHorizontal()) {
range = helpers.log10(me.end) - helpers.log10(start); // todo: if start === 0
if (newVal === 0) {
pixel = me.left;
} else {
innerDimension = me.width;
pixel = me.left + (innerDimension / range * (helpers.log10(newVal) - helpers.log10(start)));
}
innerDimension = me.width;
pixel = reverse ? me.right : me.left;
} else {
// Bottom - top since pixels increase downward on a screen
innerDimension = me.height;
if (start === 0 && !tickOpts.reverse) {
range = helpers.log10(me.end) - helpers.log10(me.minNotZero);
if (newVal === start) {
pixel = me.bottom;
} else if (newVal === me.minNotZero) {
pixel = me.bottom - innerDimension * 0.02;
} else {
pixel = me.bottom - innerDimension * 0.02 - (innerDimension * 0.98 / range * (helpers.log10(newVal) - helpers.log10(me.minNotZero)));
}
} else if (me.end === 0 && tickOpts.reverse) {
range = helpers.log10(me.start) - helpers.log10(me.minNotZero);
if (newVal === me.end) {
pixel = me.top;
} else if (newVal === me.minNotZero) {
pixel = me.top + innerDimension * 0.02;
} else {
pixel = me.top + innerDimension * 0.02 + (innerDimension * 0.98 / range * (helpers.log10(newVal) - helpers.log10(me.minNotZero)));
}
} else if (newVal === 0) {
pixel = tickOpts.reverse ? me.top : me.bottom;
} else {
range = helpers.log10(me.end) - helpers.log10(start);
innerDimension = me.height;
pixel = me.bottom - (innerDimension / range * (helpers.log10(newVal) - helpers.log10(start)));
sign *= -1; // invert, since the upper-left corner of the canvas is at pixel (0, 0)
pixel = reverse ? me.top : me.bottom;
}
if (value !== start) {
if (start === 0) { // include zero tick
offset = helpers.getValueOrDefault(
me.options.ticks.fontSize,
Chart.defaults.global.defaultFontSize
);
innerDimension -= offset;
start = firstTickValue;
}
if (value !== 0) {
offset += innerDimension / (log10(end) - log10(start)) * (log10(value) - log10(start));
}
pixel += sign * offset;
}
return pixel;
},
getValueForPixel: function(pixel) {
var me = this;
var range = helpers.log10(me.end) - helpers.log10(me.start);
var value, innerDimension;
var reverse = me.options.ticks.reverse;
var log10 = helpers.log10;
var firstTickValue = me.getFirstTickValue(me.minNotZero);
var innerDimension, start, end, value;

if (reverse) {
start = me.end;
end = me.start;
} else {
start = me.start;
end = me.end;
}
if (me.isHorizontal()) {
innerDimension = me.width;
value = me.start * Math.pow(10, (pixel - me.left) * range / innerDimension);
} else { // todo: if start === 0
value = reverse ? me.right - pixel : pixel - me.left;
} else {
innerDimension = me.height;
value = Math.pow(10, (me.bottom - pixel) * range / innerDimension) / me.start;
value = reverse ? pixel - me.top : me.bottom - pixel;
}
if (value !== start) {
if (start === 0) { // include zero tick
var offset = helpers.getValueOrDefault(
me.options.ticks.fontSize,
Chart.defaults.global.defaultFontSize
);
value -= offset;
innerDimension -= offset;
start = firstTickValue;
}
value *= log10(end) - log10(start);
value /= innerDimension;
value = Math.pow(10, log10(start) + value);
}
return value;
}
Expand Down
130 changes: 91 additions & 39 deletions test/specs/scale.logarithmic.tests.js
View file
Open in desktop
Expand Up @@ -735,47 +735,99 @@ describe('Logarithmic Scale tests', function() {
expect(yScale.getValueForPixel(246)).toBeCloseTo(10, 1e-4);
});

it('should get the correct pixel value for a point when 0 values are present', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [{
yAxisID: 'yScale',
data: [0.063, 4, 0, 63, 10, 0.5]
}],
labels: []
it('should get the correct pixel value for a point when 0 values are present or min: 0', function() {
var config = [
{
dataset: [{x: 0, y: 0}, {x: 10, y: 10}, {x: 1.2, y: 1.2}, {x: 25, y: 25}, {x: 78, y: 78}],
firstTick: 1, // value of the first tick
lastTick: 80
},
options: {
scales: {
yAxes: [{
id: 'yScale',
type: 'logarithmic',
ticks: {
reverse: false
}
}]
{
dataset: [{x: 0, y: 0}, {x: 10, y: 10}, {x: 6.3, y: 6.3}, {x: 25, y: 25}, {x: 78, y: 78}],
firstTick: 6,
lastTick: 80
},
{
dataset: [{x: 10, y: 10}, {x: 1.2, y: 1.2}, {x: 25, y: 25}, {x: 78, y: 78}],
scale: {ticks: {min: 0}},
firstTick: 1,
lastTick: 80
},
{
dataset: [{x: 10, y: 10}, {x: 6.3, y: 6.3}, {x: 25, y: 25}, {x: 78, y: 78}],
scale: {ticks: {min: 0}},
firstTick: 6,
lastTick: 80
},
];
Chart.helpers.each(config, function(setup) {
var xScaleConfig = {
type: 'logarithmic'
};
var yScaleConfig = {
type: 'logarithmic'
};
Chart.helpers.extend(xScaleConfig, setup.scale);
Chart.helpers.extend(yScaleConfig, setup.scale);
var chart = window.acquireChart({
type: 'line',
data: {
datasets: [{
data: setup.dataset
}],
},
options: {
scales: {
xAxes: [xScaleConfig],
yAxes: [yScaleConfig]
}
}
}
});

var chartArea = chart.chartArea;
var expectations = [
{
id: 'x-axis-0', // horizontal scale
axis: 'xAxes',
start: chartArea.left,
end: chartArea.right
},
{
id: 'y-axis-0', // vertical scale
axis: 'yAxes',
start: chartArea.bottom,
end: chartArea.top
}
];
Chart.helpers.each(expectations, function(expectation) {
var scale = chart.scales[expectation.id];
var firstTick = setup.firstTick;
var lastTick = setup.lastTick;
var fontSize = chart.options.defaultFontSize;
var start = expectation.start;
var end = expectation.end;
var sign = scale.isHorizontal() ? 1 : -1;

expect(scale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(start);
expect(scale.getPixelForValue(lastTick, 0, 0)).toBeCloseToPixel(end);
expect(scale.getPixelForValue(firstTick, 0, 0)).toBeCloseToPixel(start + sign * fontSize);

expect(scale.getValueForPixel(start)).toBeCloseTo(0);
expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick);
expect(scale.getValueForPixel(start + sign * fontSize)).toBeCloseTo(firstTick);

chart.options.scales[expectation.axis][0].ticks.reverse = true; // Reverse mode
chart.update();

expect(scale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(end);
expect(scale.getPixelForValue(lastTick, 0, 0)).toBeCloseToPixel(start);
expect(scale.getPixelForValue(firstTick, 0, 0)).toBeCloseToPixel(end - sign * fontSize);

expect(scale.getValueForPixel(end)).toBeCloseTo(0);
expect(scale.getValueForPixel(start)).toBeCloseTo(lastTick);
expect(scale.getValueForPixel(end - sign * fontSize)).toBeCloseTo(firstTick);
});
});

var yScale = chart.scales.yScale;
expect(yScale.getPixelForValue(70, 0, 0)).toBeCloseToPixel(32); // top + paddingTop
expect(yScale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(484); // bottom - paddingBottom
expect(yScale.getPixelForValue(0.063, 0, 0)).toBeCloseToPixel(475); // minNotZero 2% from range
expect(yScale.getPixelForValue(0.5, 0, 0)).toBeCloseToPixel(344);
expect(yScale.getPixelForValue(4, 0, 0)).toBeCloseToPixel(213);
expect(yScale.getPixelForValue(10, 0, 0)).toBeCloseToPixel(155);
expect(yScale.getPixelForValue(63, 0, 0)).toBeCloseToPixel(38.5);

chart.options.scales.yAxes[0].ticks.reverse = true; // Reverse mode
chart.update();

expect(yScale.getPixelForValue(70, 0, 0)).toBeCloseToPixel(484); // bottom - paddingBottom
expect(yScale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(32); // top + paddingTop
expect(yScale.getPixelForValue(0.063, 0, 0)).toBeCloseToPixel(41); // minNotZero 2% from range
expect(yScale.getPixelForValue(0.5, 0, 0)).toBeCloseToPixel(172);
expect(yScale.getPixelForValue(4, 0, 0)).toBeCloseToPixel(303);
expect(yScale.getPixelForValue(10, 0, 0)).toBeCloseToPixel(361);
expect(yScale.getPixelForValue(63, 0, 0)).toBeCloseToPixel(477);
});

});