Skip to content

Commit

Permalink
Merge pull request #6664 from plotly/per-legend-traceorder-dflt
Browse files Browse the repository at this point in the history
Fix per legend `traceorder` defaults and legend groups in multi legends
  • Loading branch information
archmoj committed Jul 5, 2023
2 parents c3397fc + f0ebd1f commit b249c14
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 5 deletions.
1 change: 1 addition & 0 deletions draftlogs/6664_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix per legend traceorder defaults and legend groups when having multiple legends [[#6664](https://github.com/plotly/plotly.js/pull/6664)]
14 changes: 9 additions & 5 deletions src/components/legend/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,12 @@ function groupDefaults(legendId, layoutIn, layoutOut, fullData) {
var legendReallyHasATrace = false;
var defaultOrder = 'normal';

for(var i = 0; i < fullData.length; i++) {
trace = fullData[i];
var allLegendItems = fullData.filter(function(d) {
return legendId === (d.legend || 'legend');
});

for(var i = 0; i < allLegendItems.length; i++) {
trace = allLegendItems[i];

if(!trace.visible) continue;

Expand Down Expand Up @@ -87,10 +91,10 @@ function groupDefaults(legendId, layoutIn, layoutOut, fullData) {

var showLegend = Lib.coerce(layoutIn, layoutOut,
basePlotLayoutAttributes, 'showlegend',
legendReallyHasATrace && legendTraceCount > 1);
legendReallyHasATrace && (legendTraceCount > (legendId === 'legend' ? 1 : 0)));

// delete legend
if(showLegend === false) layoutOut.legend = undefined;
if(showLegend === false) layoutOut[legendId] = undefined;

if(showLegend === false && !containerIn.uirevision) return;

Expand Down Expand Up @@ -166,7 +170,7 @@ function groupDefaults(legendId, layoutIn, layoutOut, fullData) {
}, 'y');

coerce('traceorder', defaultOrder);
if(helpers.isGrouped(layoutOut.legend)) coerce('tracegroupgap');
if(helpers.isGrouped(layoutOut[legendId])) coerce('tracegroupgap');

coerce('entrywidth');
coerce('entrywidthmode');
Expand Down
81 changes: 81 additions & 0 deletions test/jasmine/tests/legend_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,31 @@ describe('legend defaults', function() {
expect(layoutOut.legend.traceorder).toEqual('reversed');
});

it('should default traceorder to reversed for stack bar charts | multi-legend case', function() {
fullData = allShown([
{type: 'scatter'},
{legend: 'legend2', type: 'bar', visible: 'legendonly'},
{legend: 'legend2', type: 'bar', visible: 'legendonly'},
{legend: 'legend2', type: 'scatter'},
{legend: 'legend3', type: 'scatter'}
]);

layoutOut.legend2 = {};
layoutOut.legend3 = {};

supplyLayoutDefaults(layoutIn, layoutOut, fullData);
expect(layoutOut.legend.traceorder).toEqual('normal');
expect(layoutOut.legend2.traceorder).toEqual('normal');
expect(layoutOut.legend3.traceorder).toEqual('normal');

layoutOut.barmode = 'stack';

supplyLayoutDefaults(layoutIn, layoutOut, fullData);
expect(layoutOut.legend.traceorder).toEqual('normal');
expect(layoutOut.legend2.traceorder).toEqual('reversed');
expect(layoutOut.legend3.traceorder).toEqual('normal');
});

it('should default traceorder to reversed for filled tonext scatter charts', function() {
fullData = allShown([
{type: 'scatter'},
Expand All @@ -148,6 +173,30 @@ describe('legend defaults', function() {
expect(layoutOut.legend.traceorder).toEqual('grouped+reversed');
});

it('should default traceorder to grouped when a group is present | multi-legend case', function() {
fullData = allShown([
{type: 'scatter'},
{legend: 'legend2', type: 'scatter', legendgroup: 'group'},
{legend: 'legend2', type: 'scatter'},
{legend: 'legend3', type: 'scatter'}
]);

layoutOut.legend2 = {};
layoutOut.legend3 = {};

supplyLayoutDefaults(layoutIn, layoutOut, fullData);
expect(layoutOut.legend.traceorder).toEqual('normal');
expect(layoutOut.legend2.traceorder).toEqual('grouped');
expect(layoutOut.legend3.traceorder).toEqual('normal');

fullData[1].fill = 'tonextx';

supplyLayoutDefaults(layoutIn, layoutOut, fullData);
expect(layoutOut.legend.traceorder).toEqual('normal');
expect(layoutOut.legend2.traceorder).toEqual('reversed+grouped');
expect(layoutOut.legend3.traceorder).toEqual('normal');
});

it('does not consider invisible traces for traceorder default', function() {
fullData = allShown([
{type: 'bar', visible: false},
Expand All @@ -169,6 +218,38 @@ describe('legend defaults', function() {
expect(layoutOut.legend.traceorder).toEqual('normal');
});

it('does not consider invisible traces for traceorder default | multi-legend case', function() {
fullData = allShown([
{type: 'scatter'},
{legend: 'legend2', type: 'bar', visible: false},
{legend: 'legend2', type: 'bar', visible: false},
{legend: 'legend2', type: 'scatter'},
{legend: 'legend3', type: 'scatter'},
]);

layoutOut.legend2 = {};
layoutOut.legend3 = {};

layoutOut.barmode = 'stack';

supplyLayoutDefaults(layoutIn, layoutOut, fullData);
expect(layoutOut.legend.traceorder).toEqual('normal');
expect(layoutOut.legend2.traceorder).toEqual('normal');
expect(layoutOut.legend3.traceorder).toEqual('normal');

fullData = allShown([
{type: 'scatter'},
{legend: 'legend2', type: 'scatter', legendgroup: 'group', visible: false},
{legend: 'legend2', type: 'scatter'},
{legend: 'legend3', type: 'scatter'}
]);

supplyLayoutDefaults(layoutIn, layoutOut, fullData);
expect(layoutOut.legend.traceorder).toEqual('normal');
expect(layoutOut.legend2.traceorder).toEqual('normal');
expect(layoutOut.legend3.traceorder).toEqual('normal');
});

it('should default orientation to vertical', function() {
supplyLayoutDefaults(layoutIn, layoutOut, []);
expect(layoutOut.legend.orientation).toEqual('v');
Expand Down

0 comments on commit b249c14

Please sign in to comment.