Draw labels on top of lines. Fixes #94. #161
Conversation
src/annotation.js
Outdated
| element.transition(easingDecimal).draw(); | ||
| }); | ||
| elements.forEach(function(element) { | ||
| element.configure(); |
There was a problem hiding this comment.
do we need to call configure a second time? same with transition
There was a problem hiding this comment.
🤔 Good question! I don't know?
There was a problem hiding this comment.
Ok. Can you test if it works without the duplicate calls here? Let's remove if possible
There was a problem hiding this comment.
Confirmed, draw() doesn't mutate the state of model as used in configure. Updated in 44a5432
|
Chart.js 2.9.0 will have a concept of z-index: chartjs/Chart.js#6262. It would be nice to make this utilize the same concept |
|
Given the way callbacks are currently being handled, I'm not sure that refactoring the entire draw loop is in scope for this issue - you'd have to delegate all drawing back to the chart controller. |
|
@kurkle you might be interested in this one given your work in the main repo |
|
Hold off on this please. I've just noticed a side-effect where axis values aren't being redrawn when datasets are toggle on or off. |
…ue with axes not being recalculated.
|
Ok, phew, fixed that. |
|
This is a step towards layers, looks quite good. Should work without the 2nd transition. |
Code taken from chartjs#161
|
Hi, I except the same enhancement. Is it still in progress ? |
| element.transition(easingDecimal).draw(); | ||
| }); | ||
| elements.forEach(function(element) { | ||
| element.transition(easingDecimal).drawLabel(); |
There was a problem hiding this comment.
Should not need to transition again?
I've split out the label from the line drawing, and added a second loop for this phase of drawing.