Implement legendrank attribute in traces #5591
Conversation
| var B = b[0].trace; | ||
| var delta = A.legendrank - B.legendrank; | ||
| if(!delta) delta = A.index - B.index; | ||
| if(!delta) delta = a[0]._initID - b[0]._initID; |
There was a problem hiding this comment.
I'm assuming _initID is important for pie traces? Anything else? OK, so the code below implies (and I just tested to confirm) that legend.traceorder flips the order of pie entries in the legend, not just the order individual traces get added to the legend.
Based on that, I'd say the correct behavior of traceorder once we add legendrank is to simply flip the whole thing. That's not what I thought we concluded on the call yesterday but it's what you've implemented and with the pie complication I think it's the simplest thing to explain :)
There was a problem hiding this comment.
I'm fine with this.
There was a problem hiding this comment.
I'm assuming
_initIDis important for pie traces? Anything else? OK, so the code below implies (and I just tested to confirm) thatlegend.traceorderflips the order of pie entries in the legend, not just the order individual traces get added to the legend.
I noticed some older version of Chrome (including our image test) sort the items differently.
So _initID was added in a60a382 to address that issue.
As you mentioned pie-like traces appear to behave differently.
In 598d60c
I was expecting that by using a higher value of legendrankon the first pie trace the slice legends come next.
However it is the opposite and it is rather confusing.
|
Looks good! Probably wants a mock (or just adding legendranks to some existing mock) and the description addition I commented on, then it'll be ready to go! |
|
This PR was blocked by the old baseline system using old Chrome. |
|
@nicolaskruchten please review and possibly give me a 🌟 to merge this feature. |
src/plots/attributes.js
Outdated
| editType: 'style', | ||
| description: [ | ||
| 'Sets the legend rank for this trace.', | ||
| 'Items and groups with smaller ranks would be presented on top/left side while', |
There was a problem hiding this comment.
replace "would be" with "are" please, throughout :)
|
As far as I can tell, the specific case outlined in #5501 is not well-handled... B's legendrank of 1 should pull its entire group into the first position:
|
|
Here's the Pen: https://codepen.io/nicolaskruchten/pen/wvJROor |
|
@nicolaskruchten good catch. It is now fixed by 32a890c and tested by a1abb19. |
|
Behaviour looks good! |
Addressing #5501.
@plotly/plotly_js