Fixes #4772: added scope for tooltip position mode call and added docs #4784
Conversation
|
@etimberg Is this ok? I think the biggest change here is the extended entry in the documentation :D |
|
@simonbrunel |
|
I just did a quick google. It seems like the one below has node, chrome, and firefox. I haven't tested to see if it works with chart.js or not |
|
Agree about the unit test. Once we that's in this is good to merge IMO |
|
Yes! It is still on my todo. Planed it for tomorrow. The last two weeks were a bit stressful at work ;) |
|
Thanks for adding the unit test! |
test/specs/core.tooltip.tests.js
Outdated
|
|
||
| expect(this instanceof Chart.Tooltip).toBe(true); | ||
| expect(elements instanceof Array).toBe(true); | ||
| expect(eventPosition.hasOwnProperty('x') && eventPosition.hasOwnProperty('y')).toBe(true); |
There was a problem hiding this comment.
It doesn't work because if the method is not called, then expectations are not evaluated. We could use spyOn instead to be sure that the method is actually called, something like that:
var scope = null;
Chart.Tooltip.positioners.test = function() {
scope = this;
return {x: 0, y: 0};
};
spyOn(Chart.Tooltip.positioners, 'test');
// {...}
node.dispatchEvent(evt);
var fn = Chart.Tooltip.positioners.test;
expect(fn.calls.count()).toBe(1);
expect(fn.calls.first().args[0] instanceof Array).toBe(true);
expect(fn.calls.first().args[1].hasOwnProperty('x')).toBe(true);
expect(fn.calls.first().args[1].hasOwnProperty('y')).toBe(true);
expect(scope instanceof Chart.Tooltip).toBe(true);
|
@simonbrunel I resolved your requested changes. Was a bit tricky... |
|
Actually I had to look for alternatives to scope = this. Scope was undefined all the time so I was wondering if the spyOn is not storing the context somehow. |
…ded docs (chartjs#4784) * added scope for tooltip position mode call and added docs * added test for positioner * removed named func for lint * resolved pull-request comments
…ded docs (chartjs#4784) * added scope for tooltip position mode call and added docs * added test for positioner * removed named func for lint * resolved pull-request comments
See linked issue for more information #4772