Skip to content

Commit de22310

Browse files
alexcjohnsonalexcjohnson
authored
Merge pull request #2587 from plotly/blank-legend-fix
Blank editable legend item fixes
2 parents 74d7e00 + ba787a0 commit de22310

File tree

4 files changed

+101
-46
lines changed

4 files changed

+101
-46
lines changed

‎src/components/legend/draw.js

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ module.exports = function draw(gd) {
4141

4242
if(!gd._legendMouseDownTime) gd._legendMouseDownTime = 0;
4343

44-
var opts = fullLayout.legend,
45-
legendData = fullLayout.showlegend && getLegendData(gd.calcdata, opts),
46-
hiddenSlices = fullLayout.hiddenlabels || [];
44+
var opts = fullLayout.legend;
45+
var legendData = fullLayout.showlegend && getLegendData(gd.calcdata, opts);
46+
var hiddenSlices = fullLayout.hiddenlabels || [];
4747

4848
if(!fullLayout.showlegend || !legendData.length) {
4949
fullLayout._infolayer.selectAll('.legend').remove();
@@ -53,6 +53,17 @@ module.exports = function draw(gd) {
5353
return;
5454
}
5555

56+
var maxLength = 0;
57+
for(var i = 0; i < legendData.length; i++) {
58+
for(var j = 0; j < legendData[i].length; j++) {
59+
var item = legendData[i][j][0];
60+
var trace = item.trace;
61+
var isPie = Registry.traceIs(trace, 'pie');
62+
var name = isPie ? item.label : trace.name;
63+
maxLength = Math.max(maxLength, name && name.length || 0);
64+
}
65+
}
66+
5667
var firstRender = false;
5768
var legend = Lib.ensureSingle(fullLayout._infolayer, 'g', 'legend', function(s) {
5869
s.attr('pointer-events', 'all');
@@ -108,7 +119,7 @@ module.exports = function draw(gd) {
108119
})
109120
.each(function() {
110121
d3.select(this)
111-
.call(drawTexts, gd)
122+
.call(drawTexts, gd, maxLength)
112123
.call(setupTraceToggle, gd);
113124
});
114125

@@ -352,38 +363,35 @@ module.exports = function draw(gd) {
352363
}
353364
};
354365

355-
function drawTexts(g, gd) {
356-
var legendItem = g.data()[0][0],
357-
fullLayout = gd._fullLayout,
358-
trace = legendItem.trace,
359-
isPie = Registry.traceIs(trace, 'pie'),
360-
traceIndex = trace.index,
361-
name = isPie ? legendItem.label : trace.name;
366+
function drawTexts(g, gd, maxLength) {
367+
var legendItem = g.data()[0][0];
368+
var fullLayout = gd._fullLayout;
369+
var trace = legendItem.trace;
370+
var isPie = Registry.traceIs(trace, 'pie');
371+
var traceIndex = trace.index;
372+
var name = isPie ? legendItem.label : trace.name;
373+
var isEditable = gd._context.edits.legendText && !isPie;
362374

363-
var text = Lib.ensureSingle(g, 'text', 'legendtext');
375+
var textEl = Lib.ensureSingle(g, 'text', 'legendtext');
364376

365-
text.attr('text-anchor', 'start')
377+
textEl.attr('text-anchor', 'start')
366378
.classed('user-select-none', true)
367379
.call(Drawing.font, fullLayout.legend.font)
368-
.text(name);
380+
.text(isEditable ? ensureLength(name, maxLength) : name);
369381

370382
function textLayout(s) {
371383
svgTextUtils.convertToTspans(s, gd, function() {
372384
computeTextDimensions(g, gd);
373385
});
374386
}
375387

376-
if(gd._context.edits.legendText && !isPie) {
377-
text.call(svgTextUtils.makeEditable, {gd: gd})
388+
if(isEditable) {
389+
textEl.call(svgTextUtils.makeEditable, {gd: gd, text: name})
378390
.call(textLayout)
379-
.on('edit', function(text) {
380-
this.text(text)
391+
.on('edit', function(newName) {
392+
this.text(ensureLength(newName, maxLength))
381393
.call(textLayout);
382394

383-
var origText = text;
384-
385-
if(!this.text()) text = ' \u0020\u0020 ';
386-
387395
var fullInput = legendItem.trace._fullInput || {};
388396
var update = {};
389397

@@ -393,24 +401,35 @@ function drawTexts(g, gd) {
393401

394402
var kcont = Lib.keyedContainer(fullInput, 'transforms[' + index + '].styles', 'target', 'value.name');
395403

396-
if(origText === '') {
397-
kcont.remove(legendItem.trace._group);
398-
} else {
399-
kcont.set(legendItem.trace._group, text);
400-
}
404+
kcont.set(legendItem.trace._group, newName);
401405

402406
update = kcont.constructUpdate();
403407
} else {
404-
update.name = text;
408+
update.name = newName;
405409
}
406410

407411
return Registry.call('restyle', gd, update, traceIndex);
408412
});
409413
} else {
410-
textLayout(text);
414+
textLayout(textEl);
411415
}
412416
}
413417

418+
/*
419+
* Make sure we have a reasonably clickable region.
420+
* If this string is missing or very short, pad it with spaces out to at least
421+
* 4 characters, up to the max length of other labels, on the assumption that
422+
* most characters are wider than spaces so a string of spaces will usually be
423+
* no wider than the real labels.
424+
*/
425+
function ensureLength(str, maxLength) {
426+
var targetLength = Math.max(4, maxLength);
427+
if(str && str.trim().length >= targetLength / 2) return str;
428+
str = str || '';
429+
for(var i = targetLength - str.length; i > 0; i--) str += ' ';
430+
return str;
431+
}
432+
414433
function setupTraceToggle(g, gd) {
415434
var newMouseDownTime,
416435
numClicks = 1;

‎src/lib/svg_text_utils.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,9 @@ exports.makeEditable = function(context, options) {
607607
var cStyle = context.node().style;
608608
var fontSize = parseFloat(cStyle.fontSize || 12);
609609

610+
var initialText = options.text;
611+
if(initialText === undefined) initialText = context.attr('data-unformatted');
612+
610613
div.classed('plugin-editable editable', true)
611614
.style({
612615
position: 'absolute',
@@ -621,7 +624,7 @@ exports.makeEditable = function(context, options) {
621624
'box-sizing': 'border-box'
622625
})
623626
.attr({contenteditable: true})
624-
.text(options.text || context.attr('data-unformatted'))
627+
.text(initialText)
625628
.call(alignHTMLWith(context, container, options))
626629
.on('blur', function() {
627630
gd._editing = false;

‎src/transforms/groupby.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ function transformOne(trace, state) {
214214
suppliedName = groupNameObj.get(groupName);
215215
}
216216

217-
if(suppliedName) {
217+
if(suppliedName || suppliedName === '') {
218218
newTrace.name = suppliedName;
219219
} else {
220220
newTrace.name = Lib.templateString(opts.nameformat, {

‎test/jasmine/tests/legend_test.js

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -920,11 +920,11 @@ describe('legend interaction', function() {
920920
x: [1, 2, 3],
921921
y: [5, 4, 3]
922922
}, {
923-
x: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14],
924-
y: [1, 3, 2, 4, 3, 5, 4, 6, 5, 7, 6, 8, 7, 9],
923+
x: [1, 2, 3, 4, 5, 6, 7, 8],
924+
y: [1, 3, 2, 4, 3, 5, 4, 6],
925925
transforms: [{
926926
type: 'groupby',
927-
groups: [1, 2, 1, 2, 3, 4, 3, 4, 5, 6, 5, 6, 7, 8]
927+
groups: [1, 2, 1, 2, 3, 4, 3, 4]
928928
}]
929929
}],
930930
config: {editable: true}
@@ -947,35 +947,68 @@ describe('legend interaction', function() {
947947
}).then(delay(20));
948948
}
949949

950+
function assertLabels(expected) {
951+
var labels = [];
952+
d3.selectAll('text.legendtext').each(function() {
953+
labels.push(this.textContent);
954+
});
955+
expect(labels).toEqual(expected);
956+
}
957+
950958
it('sets and unsets trace group names', function(done) {
959+
assertLabels(['trace 0', '1 (trace 1)', '2 (trace 1)', '3 (trace 1)', '4 (trace 1)']);
951960
// Set the name of the first trace:
952961
_setValue(0, 'foo').then(function() {
953962
expect(gd.data[0].name).toEqual('foo');
954-
}).then(function() {
963+
// labels shorter than half the longest get padded with spaces to match the longest length
964+
assertLabels(['foo ', '1 (trace 1)', '2 (trace 1)', '3 (trace 1)', '4 (trace 1)']);
965+
955966
// Set the name of the third legend item:
956-
return _setValue(3, 'bar');
967+
return _setValue(3, 'barbar');
957968
}).then(function() {
958969
expect(gd.data[1].transforms[0].styles).toEqual([
959-
{value: {name: 'bar'}, target: 3}
970+
{value: {name: 'barbar'}, target: 3}
960971
]);
972+
assertLabels(['foo ', '1 (trace 1)', '2 (trace 1)', 'barbar', '4 (trace 1)']);
973+
974+
return _setValue(2, 'asdf');
961975
}).then(function() {
962-
return _setValue(4, 'asdf');
976+
expect(gd.data[1].transforms[0].styles).toEqual([
977+
{value: {name: 'barbar'}, target: 3},
978+
{value: {name: 'asdf'}, target: 2}
979+
]);
980+
assertLabels(['foo ', '1 (trace 1)', 'asdf ', 'barbar', '4 (trace 1)']);
981+
982+
// Clear the group names:
983+
return _setValue(3, '');
963984
}).then(function() {
985+
assertLabels(['foo ', '1 (trace 1)', 'asdf ', ' ', '4 (trace 1)']);
986+
return _setValue(2, '');
987+
}).then(function() {
988+
// Verify the group names have been cleared:
964989
expect(gd.data[1].transforms[0].styles).toEqual([
965-
{value: {name: 'bar'}, target: 3},
966-
{value: {name: 'asdf'}, target: 4}
990+
{target: 3, value: {name: ''}},
991+
{target: 2, value: {name: ''}}
967992
]);
993+
assertLabels(['foo ', '1 (trace 1)', ' ', ' ', '4 (trace 1)']);
994+
995+
return _setValue(0, '');
968996
}).then(function() {
969-
// Unset the group names:
970-
return _setValue(3, '');
997+
expect(gd.data[0].name).toEqual('');
998+
assertLabels([' ', '1 (trace 1)', ' ', ' ', '4 (trace 1)']);
999+
1000+
return _setValue(0, 'boo~~~');
9711001
}).then(function() {
972-
return _setValue(4, '');
1002+
expect(gd.data[0].name).toEqual('boo~~~');
1003+
assertLabels(['boo~~~', '1 (trace 1)', ' ', ' ', '4 (trace 1)']);
1004+
1005+
return _setValue(2, 'hoo');
9731006
}).then(function() {
974-
// Verify the group names have been cleaned up:
9751007
expect(gd.data[1].transforms[0].styles).toEqual([
976-
{target: 3, value: {}},
977-
{target: 4, value: {}}
1008+
{target: 3, value: {name: ''}},
1009+
{target: 2, value: {name: 'hoo'}}
9781010
]);
1011+
assertLabels(['boo~~~', '1 (trace 1)', 'hoo ', ' ', '4 (trace 1)']);
9791012
}).catch(fail).then(done);
9801013
});
9811014
});

0 commit comments

Comments
 (0)