Skip to content
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.

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

add entrywidth and entrywidthmode to legend #6202

Merged
merged 7 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions src/components/legend/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@ module.exports = {
'Sets the amount of vertical space (in px) between legend groups.'
].join(' ')
},
entrywidth: {
valType: 'number',
archmoj marked this conversation as resolved.
Show resolved Hide resolved
min: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should provide a dflt for this attribute.
However using zero looks confusing.
@alexcjohnson

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually think it's nice to use zero to mean "automatic - use the text width," if we document that.

Alternatively, we could have a third value for entrywidthmode, perhaps 'auto', that means we use the text width. Then the logic would be:

  • If a nonzero entrywidth is provided, entrywidthmode defaults to 'pixel'. If not, it defaults to 'auto'.
  • If entrywidthmode is 'auto' we don't coerce entrywidth.

That feels unnecessarily cumbersome to me though. Only reason I can think of that would justify this is if we want to actually support a width of zero. Any reason someone would want that?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to use zero as the auto default the min namely for the pixel mode should be a value greater or equal 1 not zero IMHO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That would be confusing, wouldn't it? We'd document "Use 0 to size the entry based on the text width" but document a min of 1... yet if you explicitly set it to 0 it would work, because we'd throw out the 0 and replace it with the default 0.

Let's keep this simple, and consistent with colorbar.len and colorbar.thickness - min: 0 and no max regardless of entrywidthmode, the only difference being that 0 has a special meaning we'll document. If it turns out that certain values (either very small or very large) cause problems worse than "it looks bad," we can address that during the drawing process when we know more than we do at the supplyDefaults step.

editType: 'legend',
description: 'Sets the width (in px or fraction) of the legend.',
},
entrywidthmode: {
valType: 'enumerated',
values: ['fraction', 'pixels'],
dflt: 'pixels',
editType: 'legend',
description: 'Determines what entrywidth means.',
},
itemsizing: {
valType: 'enumerated',
values: ['trace', 'constant'],
Expand Down
2 changes: 2 additions & 0 deletions src/components/legend/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {
coerce('traceorder', defaultOrder);
if(helpers.isGrouped(layoutOut.legend)) coerce('tracegroupgap');

coerce('entrywidth');
coerce('entrywidthmode');
coerce('itemsizing');
coerce('itemwidth');

Expand Down
44 changes: 37 additions & 7 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,23 @@ function _draw(gd, legendObj) {
}], gd);
}

function getTraceWidth(trace, legendObj, textGap) {
var legendItem = trace[0];
var legendWidth = legendItem.width;

var traceLegendWidth = legendItem.trace.legendwidth || legendObj.entrywidth;

if(traceLegendWidth) {
if(legendObj.entrywidthmode === 'pixels') {
return traceLegendWidth + textGap;
} else {
return legendObj._maxWidth * traceLegendWidth;
}
}

return legendWidth + textGap;
}

function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) {
var trace = legendItem.data()[0][0].trace;
var evtData = {
Expand Down Expand Up @@ -636,6 +653,7 @@ function computeLegendDimensions(gd, groups, traces, legendObj) {
var isAbovePlotArea = legendObj.y > 1 || (legendObj.y === 1 && yanchor === 'bottom');

var traceGroupGap = legendObj.tracegroupgap;
var legendGroupWidths = {};

// - if below/above plot area, give it the maximum potential margin-push value
// - otherwise, extend the height of the plot area
Expand Down Expand Up @@ -688,7 +706,7 @@ function computeLegendDimensions(gd, groups, traces, legendObj) {
var maxItemWidth = 0;
var combinedItemWidth = 0;
traces.each(function(d) {
var w = d[0].width + textGap;
var w = getTraceWidth(d, legendObj, textGap);
maxItemWidth = Math.max(maxItemWidth, w);
combinedItemWidth += w;
});
Expand All @@ -704,15 +722,16 @@ function computeLegendDimensions(gd, groups, traces, legendObj) {
var maxWidthInGroup = 0;
var offsetY = 0;
d3.select(this).selectAll('g.traces').each(function(d) {
var w = d[0].width;
var w = getTraceWidth(d, legendObj, textGap);
var h = d[0].height;

Drawing.setTranslate(this,
titleSize[0],
titleSize[1] + bw + itemGap + h / 2 + offsetY
);
offsetY += h;
maxWidthInGroup = Math.max(maxWidthInGroup, textGap + w);
maxWidthInGroup = Math.max(maxWidthInGroup, w);
legendGroupWidths[d[0].trace.legendgroup] = maxWidthInGroup;
});

var next = maxWidthInGroup + itemGap;
Expand Down Expand Up @@ -750,8 +769,12 @@ function computeLegendDimensions(gd, groups, traces, legendObj) {
var rowWidth = 0;
traces.each(function(d) {
var h = d[0].height;
var w = textGap + d[0].width;
var next = (oneRowLegend ? w : maxItemWidth) + itemGap;
var w = getTraceWidth(d, legendObj, textGap, isGrouped);
var next = (oneRowLegend ? w : maxItemWidth);

if(legendObj.entrywidthmode !== 'fraction') {
next += itemGap;
}

if((next + bw + offsetX - itemGap) >= legendObj._maxWidth) {
maxRowWidth = Math.max(maxRowWidth, rowWidth);
Expand Down Expand Up @@ -802,8 +825,15 @@ function computeLegendDimensions(gd, groups, traces, legendObj) {
traces.each(function(d) {
var traceToggle = d3.select(this).select('.legendtoggle');
var h = d[0].height;
var w = isEditable ? textGap : (toggleRectWidth || (textGap + d[0].width));
if(!isVertical) w += itemGap / 2;
var legendgroup = d[0].trace.legendgroup;
var traceWidth = getTraceWidth(d, legendObj, textGap);
if(isGrouped && legendgroup !== '') {
traceWidth = legendGroupWidths[legendgroup];
}
var w = isEditable ? textGap : (toggleRectWidth || traceWidth);
if(!isVertical && legendObj.entrywidthmode !== 'fraction') {
w += itemGap / 2;
}
Drawing.setRect(traceToggle, 0, -h / 2, w, h);
});
}
Expand Down
6 changes: 6 additions & 0 deletions src/plots/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ module.exports = {
'and ranks greater than 1000 to go after all unranked items.'
].join(' ')
},
legendwidth: {
valType: 'number',
min: 0,
editType: 'style',
description: 'Sets the width (in px or fraction) of the legend for this trace.',
},
opacity: {
valType: 'number',
min: 0,
Expand Down
1 change: 1 addition & 0 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
'showlegend'
);

coerce('legendwidth');
coerce('legendgroup');
coerce('legendgrouptitle.text');
coerce('legendrank');
Expand Down
85 changes: 85 additions & 0 deletions test/jasmine/tests/legend_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var DBLCLICKDELAY = require('@src/plot_api/plot_config').dfltConfig.doubleClickD
var Legend = require('@src/components/legend');
var getLegendData = require('@src/components/legend/get_legend_data');
var helpers = require('@src/components/legend/helpers');
var constants = require('@src/components/legend/constants');

var d3Select = require('../../strict-d3').select;
var d3SelectAll = require('../../strict-d3').selectAll;
Expand Down Expand Up @@ -2345,3 +2346,87 @@ describe('legend with custom doubleClickDelay', function() {
.then(done, done.fail);
}, 3 * jasmine.DEFAULT_TIMEOUT_INTERVAL);
});

describe('legend with custom legendwidth', function() {
var gd;

var data = [
{x: [1, 2, 1], y: [1, 2, 1], name: 'legend text 1'},
{x: [2, 1, 2], y: [2, 1, 2], name: 'legend text 12'},
{x: [2, 3, 4], y: [2, 3, 4], name: 'legend text 123'}
];

var layout = {
legend: {
orientation: 'h'
}
};

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

function assertLegendTextWidth(variants) {
var nodes = d3SelectAll('rect.legendtoggle');
var index = 0;
nodes.each(function() {
var node = d3Select(this);
var w = +node.attr('width');
if(variants[index]) expect(w).toEqual(variants[index]);
index += 1;
});
}

it('should change width when trace has legendwidth', function(done) {
var extendedData = Lib.extendDeep([], data);
extendedData.forEach(function(trace, index) {
trace.legendwidth = (index + 1) * 50;
});

var textGap = 30 + constants.itemGap * 2 + constants.itemGap / 2;

Plotly.newPlot(gd, {data: extendedData, layout: layout}).then(function() {
assertLegendTextWidth([50 + textGap, 100 + textGap, 150 + textGap]);
}).then(done);
});

it('should change width when legend has entrywidth', function(done) {
var extendedLayout = Lib.extendDeep([], layout);
var width = 50;
extendedLayout.legend.entrywidth = width;

var textGap = 30 + constants.itemGap * 2 + constants.itemGap / 2;

Plotly.newPlot(gd, {data: data, layout: extendedLayout}).then(function() {
assertLegendTextWidth([width + textGap, width + textGap, width + textGap]);
}).then(done);
});

it('should change group width when trace has legendwidth', function(done) {
var extendedLayout = Lib.extendDeep([], layout);
extendedLayout.legend.traceorder = 'grouped';

var extendedData = Lib.extendDeep([], data);
extendedData[0].legendwidth = 100;
extendedData[0].legendgroup = 'test';
extendedData[1].legendgroup = 'test';

var textGap = 30 + constants.itemGap * 2 + constants.itemGap / 2;

Plotly.newPlot(gd, {data: extendedData, layout: extendedLayout}).then(function() {
assertLegendTextWidth([100 + textGap, 100 + textGap, undefined]);
}).then(done);
});

it('should change width when legend has entrywidth and entrywidthmode is fraction', function(done) {
var extendedLayout = Lib.extendDeep([], layout);
extendedLayout.legend.entrywidthmode = 'fraction';
extendedLayout.legend.entrywidth = 0.3;

Plotly.newPlot(gd, {data: data, layout: extendedLayout}).then(function() {
assertLegendTextWidth([162, 162, 162]);
}).then(done);
});
});
Loading