Enable line annotation using two scales

[kurkle]

Changes:

• position: 'start', 'end' or 'center'
• alternative xScaleID, yScaleID, xMin, yMin, xMax, yMax options for line annotation
• label hit testing by rotating the point (similar to datalabels plugin)
• simpler label position adjustment by calculating the parameter t for pointInLine

[stockiNail]

@kurkle thank you very much!
I'm gonna have a look tomorrow.

[stockiNail]

same with this pr: (note the text direction, that might be the next feedback :))

I have already done :D. See my previous note in the review close to the code.

[stockiNail]

In this case, removing the clipping would give a good result, but it does not help if there is no room outside the chart area.
I'm wondering if it would be better to consider this when setting the scale ranges?

Pr #282 solved this issue recalculating the label position in order to stay inside the chart area.

I know that it's not so simple (that's why I added chart reference into element in that PR).

Removing the cliparea could be configurable in order to address the issue #258 as well, something like that:

label: {
	clipEnabled: false, // <-- defaults to true
	position: 'start',
	rotation: -45,
	backgroundColor: 'red',
	content: 'This is a long test label',
	enabled: true
},

In this way you can delegate the user to have enough space for the label.

To change the scales limits (as is done now for the values) could be a solution but this could change the chart going to a picture that probably you don't like with additional space in chart area that you want to avoid.
Honestly, I don't like also the current implementation which is changing the min/max of scales when the values are out of scale, therefore we can maintain as is.
If this will be the solution, we must calculate the rotation and then the necessary space for the label before the element drawing is invoked, I guess.

[stockiNail]

Other feedback (not so important). All samples are still using the previous position option values (left, right, top and bottom).
If it can help, I can submit a PR to your branch, if you don't have time to do it.

[kurkle]

Other feedback (not so important). All samples are still using the previous position option values (left, right, top and bottom).
If it can help, I can submit a PR to your branch, if you don't have time to do it.

Found only one sample using that. And it turs out it does not work correctly with this PR:

pr: (start)

master: (top)

[stockiNail]

@kurkle It seems that return Math.max(x, y) (in calculateTAdjust method) is wrong because is returning the maximun value instead it should return x (if line is horizontal) and y (if line is vertical)

[stockiNail]

Not sure if it could be correct but the return could be

return w > h ? x : y;

if width of line if greater than height means that the line is horizontal

[kurkle]

Ok, another try.

Adjust the label position in 2 phases, first along the line, up to 25% from the desired end, then with slightly modified current method.

[kurkle]

Filename	Size	Change
dist/chartjs-plugin-annotation.esm.js	6.58 kB	+9 B (0%)
dist/chartjs-plugin-annotation.js	6.7 kB	+13 B (0%)
dist/chartjs-plugin-annotation.min.js	4.47 kB	-152 B (-3%)

[etimberg]

Didn't notice anything wrong. is the change to start, center, end considered breaking?

[stockiNail]

@etimberg in my opinion, yes. You don't have errors because if the position is not start or end it will use center, without any issue.
Nevertheless who is using now left, right, top or bottom will have the label in the center, therefore I think it should be considered a breaking change.
Am I wrong?

[kurkle]

@etimberg thats the reason I labeled this breaking.

[stockiNail]

@kurkle apologize for delay of my feedback but I had other stuff to do it.
But finally I had time to test it and it sounds working correctly!
Thank you very much!