Skip to content

Implement legendrank attribute in traces #5591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jun 16, 2021
Merged

Implement legendrank attribute in traces #5591

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3cee835
implement legendrank
archmoj Apr 15, 2021
a60a382
keep track of initial order - to handle chrome unstable sort in old v…
archmoj Apr 15, 2021
5813717
remove some empty lines
archmoj Apr 16, 2021
5136cc0
add legend calc jasmine test with legendrank
archmoj Apr 16, 2021
598d60c
add image test - pie ranking looks strange
archmoj Apr 16, 2021
4ffb06c
describe why dflt 1000 is used - mentioning ranking works within each…
archmoj Apr 16, 2021
6e61bd8
refactor - less variable and use push
archmoj Apr 19, 2021
737478e
refactor - add sort step
archmoj Apr 19, 2021
274db06
refactor - bring down the sort function
archmoj Apr 19, 2021
915c67a
refactor - simplify logic
archmoj Apr 19, 2021
69c0b29
update image test
archmoj Apr 19, 2021
e1e9a74
simplify and fixup legend sort
archmoj Apr 19, 2021
f232f42
no more need for _initID now that sort and collapse are fixed
archmoj Apr 19, 2021
c00d56e
more refactor
archmoj Apr 19, 2021
69bf7eb
remove extra loop - update comments
archmoj Apr 19, 2021
d87ee12
update legendrank description
archmoj Apr 22, 2021
d35ff08
add fallback to have stable sort in Chrome < 70
archmoj Jun 15, 2021
a1f87b9
would be > are
archmoj Jun 15, 2021
32a890c
rank groups based on minimum legendranks within each group
archmoj Jun 16, 2021
a1abb19
add legendrank2 mock
archmoj Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/components/legend/get_legend_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ module.exports = function getLegendData(calcdata, opts) {
var lgroupi = 0;
var maxNameLength = 0;
var i, j;

var initID = 0;
function addOneItem(legendGroup, legendItem) {
legendItem._initID = initID++;

// each '' legend group is treated as a separate group
if(legendGroup === '' || !helpers.isGrouped(opts)) {
// TODO: check this against fullData legendgroups?
Expand Down Expand Up @@ -71,21 +73,34 @@ module.exports = function getLegendData(calcdata, opts) {
var ltraces;
var legendData;

// sort considering trace.legendrank and legend.traceorder
var dir = helpers.isReversed(opts) ? -1 : 1;
var orderFn = function(a, b) {
var A = a[0].trace;
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.


return dir * delta;
};

if(hasOneNonBlankGroup && helpers.isGrouped(opts)) {
legendData = new Array(lgroupsLength);

for(i = 0; i < lgroupsLength; i++) {
ltraces = lgroupToTraces[lgroups[i]];
legendData[i] = helpers.isReversed(opts) ? ltraces.reverse() : ltraces;
legendData[i] = ltraces.sort(orderFn);
}
} else {
// collapse all groups into one if all groups are blank
legendData = [new Array(lgroupsLength)];

legendData = [[]];
for(i = 0; i < lgroupsLength; i++) {
ltraces = lgroupToTraces[lgroups[i]][0];
legendData[0][helpers.isReversed(opts) ? lgroupsLength - i - 1 : i] = ltraces;
legendData[0].push(
lgroupToTraces[lgroups[i]][0]
);
}
legendData[0] = legendData[0].sort(orderFn);
lgroupsLength = 1;
}

Expand Down
10 changes: 10 additions & 0 deletions src/plots/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ module.exports = {
'when toggling legend items.'
].join(' ')
},
legendrank: {
valType: 'number',
dflt: 1000,
editType: 'style',
description: [
'Sets the legend rank for this trace.',
'Items with smaller ranks would be presented on top/left side while',
'with `*reversed* `legend.traceorder` they would be on bottom/right side.'
].join(' ')
},
opacity: {
valType: 'number',
min: 0,
Expand Down
1 change: 1 addition & 0 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
);

coerce('legendgroup');
coerce('legendrank');

traceOut._dfltShowLegend = true;
} else {
Expand Down
1 change: 1 addition & 0 deletions src/traces/parcats/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ module.exports = {
hoverlabel: undefined,
ids: undefined,
legendgroup: undefined,
legendrank: undefined,
opacity: undefined,
selectedpoints: undefined,
showlegend: undefined
Expand Down
24 changes: 12 additions & 12 deletions test/jasmine/tests/legend_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,22 +283,22 @@ describe('legend getLegendData', function() {

expected = [
[
[{trace: {
[{_initID: 0, trace: {
type: 'scatter',
visible: true,
legendgroup: 'group',
showlegend: true

}}],
[{trace: {
[{_initID: 2, trace: {
type: 'scatter',
visible: true,
legendgroup: 'group',
showlegend: true
}}]
],
[
[{trace: {
[{_initID: 1, trace: {
type: 'bar',
visible: 'legendonly',
legendgroup: '',
Expand Down Expand Up @@ -341,20 +341,20 @@ describe('legend getLegendData', function() {

expected = [
[
[{trace: {
[{_initID: 0, trace: {
type: 'scatter',
visible: true,
legendgroup: '',
showlegend: true

}}],
[{trace: {
[{_initID: 1, trace: {
type: 'bar',
visible: 'legendonly',
legendgroup: '',
showlegend: true
}}],
[{trace: {
[{_initID: 2, trace: {
type: 'scatter',
visible: true,
legendgroup: '',
Expand Down Expand Up @@ -427,20 +427,20 @@ describe('legend getLegendData', function() {

expected = [
[
[{trace: {
[{_initID: 2, trace: {
type: 'box',
visible: true,
legendgroup: '',
showlegend: true

}}],
[{trace: {
[{_initID: 1, trace: {
type: 'bar',
visible: 'legendonly',
legendgroup: '',
showlegend: true
}}],
[{trace: {
[{_initID: 0, trace: {
type: 'scatter',
visible: true,
legendgroup: '',
Expand Down Expand Up @@ -483,22 +483,22 @@ describe('legend getLegendData', function() {

expected = [
[
[{trace: {
[{_initID: 2, trace: {
type: 'box',
visible: true,
legendgroup: 'group',
showlegend: true

}}],
[{trace: {
[{_initID: 0, trace: {
type: 'scatter',
visible: true,
legendgroup: 'group',
showlegend: true
}}]
],
[
[{trace: {
[{_initID: 1, trace: {
type: 'bar',
visible: 'legendonly',
legendgroup: '',
Expand Down