-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Implement dataset.order
#6268
Implement dataset.order
#6268
Conversation
My personal opinion is that it would be clearer and more flexible to use z-index here and mix the datasets with the other layers. I understand there's some concern that we fire |
@benmccann after 74ff647 this |
Maybe I don't understand what stacking order of datasets is. I thought it was the order in which datasets were drawn similar to z-index. I don't think you could have both stacking order and z-index because they're essentially the same thing and what if you specified conflicting orders? |
Basically, new Chart(ctx, {
data: {
datasets: [
{ ... },
{ ... },
//...
}].sort(dataset => dataset.order)
}) ... which I think should be the responsibility of the user to sort the array before creating/updating the chart but too many people asked for something built-in. Though, in v3, we should convert the dataset array to an object keyed by the dataset id, in which case, // V3 datasets
new Chart(ctx, {
datasets: {
line0: { order: 1 },
line1: { order: 0 },
// ...
}
})
I tend to think that it's less confusing. Most use cases have all datasets before, after or between scale components (but not mixed). In this case,
I agree but to me it's a v3 feature (which hasn't been requested yet) |
Ok. Makes sense to me. Thanks for the additional explanation. This PR looks pretty good to me |
Rebased and addressed review comments |
When do you plan to release this feature? With version 2 there will be only with |
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except for a few minor things.
Updated based on comments |
Allow sorting datasets based on the `order` property
Resolves: #6050
Resolves: #5171
Pen (updated!)