Skip to content

Commit

Permalink
Merge pull request #2587 from plotly/blank-legend-fix
Browse files Browse the repository at this point in the history 
Blank editable legend item fixes
  • Loading branch information
@alexcjohnson
alexcjohnson authored Apr 27, 2018
2 parents 74d7e00 + ba787a0 commit de22310
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 46 deletions.
77 changes: 48 additions & 29 deletions src/components/legend/draw.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ module.exports = function draw(gd) {

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

var opts = fullLayout.legend,
legendData = fullLayout.showlegend && getLegendData(gd.calcdata, opts),
hiddenSlices = fullLayout.hiddenlabels || [];
var opts = fullLayout.legend;
var legendData = fullLayout.showlegend && getLegendData(gd.calcdata, opts);
var hiddenSlices = fullLayout.hiddenlabels || [];

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

var maxLength = 0;
for(var i = 0; i < legendData.length; i++) {
for(var j = 0; j < legendData[i].length; j++) {
var item = legendData[i][j][0];
var trace = item.trace;
var isPie = Registry.traceIs(trace, 'pie');
var name = isPie ? item.label : trace.name;
maxLength = Math.max(maxLength, name && name.length || 0);
}
}

var firstRender = false;
var legend = Lib.ensureSingle(fullLayout._infolayer, 'g', 'legend', function(s) {
s.attr('pointer-events', 'all');
Expand Down Expand Up @@ -108,7 +119,7 @@ module.exports = function draw(gd) {
})
.each(function() {
d3.select(this)
.call(drawTexts, gd)
.call(drawTexts, gd, maxLength)
.call(setupTraceToggle, gd);
});

Expand Down Expand Up @@ -352,38 +363,35 @@ module.exports = function draw(gd) {
}
};

function drawTexts(g, gd) {
var legendItem = g.data()[0][0],
fullLayout = gd._fullLayout,
trace = legendItem.trace,
isPie = Registry.traceIs(trace, 'pie'),
traceIndex = trace.index,
name = isPie ? legendItem.label : trace.name;
function drawTexts(g, gd, maxLength) {
var legendItem = g.data()[0][0];
var fullLayout = gd._fullLayout;
var trace = legendItem.trace;
var isPie = Registry.traceIs(trace, 'pie');
var traceIndex = trace.index;
var name = isPie ? legendItem.label : trace.name;
var isEditable = gd._context.edits.legendText && !isPie;

var text = Lib.ensureSingle(g, 'text', 'legendtext');
var textEl = Lib.ensureSingle(g, 'text', 'legendtext');

text.attr('text-anchor', 'start')
textEl.attr('text-anchor', 'start')
.classed('user-select-none', true)
.call(Drawing.font, fullLayout.legend.font)
.text(name);
.text(isEditable ? ensureLength(name, maxLength) : name);

function textLayout(s) {
svgTextUtils.convertToTspans(s, gd, function() {
computeTextDimensions(g, gd);
});
}

if(gd._context.edits.legendText && !isPie) {
text.call(svgTextUtils.makeEditable, {gd: gd})
if(isEditable) {
textEl.call(svgTextUtils.makeEditable, {gd: gd, text: name})
.call(textLayout)
.on('edit', function(text) {
this.text(text)
.on('edit', function(newName) {
this.text(ensureLength(newName, maxLength))
.call(textLayout);

var origText = text;

if(!this.text()) text = ' \u0020\u0020 ';

var fullInput = legendItem.trace._fullInput || {};
var update = {};

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

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

if(origText === '') {
kcont.remove(legendItem.trace._group);
} else {
kcont.set(legendItem.trace._group, text);
}
kcont.set(legendItem.trace._group, newName);

update = kcont.constructUpdate();
} else {
update.name = text;
update.name = newName;
}

return Registry.call('restyle', gd, update, traceIndex);
});
} else {
textLayout(text);
textLayout(textEl);
}
}

/*
* Make sure we have a reasonably clickable region.
* If this string is missing or very short, pad it with spaces out to at least
* 4 characters, up to the max length of other labels, on the assumption that
* most characters are wider than spaces so a string of spaces will usually be
* no wider than the real labels.
*/
function ensureLength(str, maxLength) {
var targetLength = Math.max(4, maxLength);
if(str && str.trim().length >= targetLength / 2) return str;
str = str || '';
for(var i = targetLength - str.length; i > 0; i--) str += ' ';
return str;
}

function setupTraceToggle(g, gd) {
var newMouseDownTime,
numClicks = 1;
Expand Down
5 changes: 4 additions & 1 deletion src/lib/svg_text_utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,9 @@ exports.makeEditable = function(context, options) {
var cStyle = context.node().style;
var fontSize = parseFloat(cStyle.fontSize || 12);

var initialText = options.text;
if(initialText === undefined) initialText = context.attr('data-unformatted');

div.classed('plugin-editable editable', true)
.style({
position: 'absolute',
Expand All @@ -621,7 +624,7 @@ exports.makeEditable = function(context, options) {
'box-sizing': 'border-box'
})
.attr({contenteditable: true})
.text(options.text || context.attr('data-unformatted'))
.text(initialText)
.call(alignHTMLWith(context, container, options))
.on('blur', function() {
gd._editing = false;
Expand Down
2 changes: 1 addition & 1 deletion src/transforms/groupby.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function transformOne(trace, state) {
suppliedName = groupNameObj.get(groupName);
}

if(suppliedName) {
if(suppliedName || suppliedName === '') {
newTrace.name = suppliedName;
} else {
newTrace.name = Lib.templateString(opts.nameformat, {
Expand Down
63 changes: 48 additions & 15 deletions test/jasmine/tests/legend_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -920,11 +920,11 @@ describe('legend interaction', function() {
x: [1, 2, 3],
y: [5, 4, 3]
}, {
x: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14],
y: [1, 3, 2, 4, 3, 5, 4, 6, 5, 7, 6, 8, 7, 9],
x: [1, 2, 3, 4, 5, 6, 7, 8],
y: [1, 3, 2, 4, 3, 5, 4, 6],
transforms: [{
type: 'groupby',
groups: [1, 2, 1, 2, 3, 4, 3, 4, 5, 6, 5, 6, 7, 8]
groups: [1, 2, 1, 2, 3, 4, 3, 4]
}]
}],
config: {editable: true}
Expand All @@ -947,35 +947,68 @@ describe('legend interaction', function() {
}).then(delay(20));
}

function assertLabels(expected) {
var labels = [];
d3.selectAll('text.legendtext').each(function() {
labels.push(this.textContent);
});
expect(labels).toEqual(expected);
}

it('sets and unsets trace group names', function(done) {
assertLabels(['trace 0', '1 (trace 1)', '2 (trace 1)', '3 (trace 1)', '4 (trace 1)']);
// Set the name of the first trace:
_setValue(0, 'foo').then(function() {
expect(gd.data[0].name).toEqual('foo');
}).then(function() {
// labels shorter than half the longest get padded with spaces to match the longest length
assertLabels(['foo ', '1 (trace 1)', '2 (trace 1)', '3 (trace 1)', '4 (trace 1)']);

// Set the name of the third legend item:
return _setValue(3, 'bar');
return _setValue(3, 'barbar');
}).then(function() {
expect(gd.data[1].transforms[0].styles).toEqual([
{value: {name: 'bar'}, target: 3}
{value: {name: 'barbar'}, target: 3}
]);
assertLabels(['foo ', '1 (trace 1)', '2 (trace 1)', 'barbar', '4 (trace 1)']);

return _setValue(2, 'asdf');
}).then(function() {
return _setValue(4, 'asdf');
expect(gd.data[1].transforms[0].styles).toEqual([
{value: {name: 'barbar'}, target: 3},
{value: {name: 'asdf'}, target: 2}
]);
assertLabels(['foo ', '1 (trace 1)', 'asdf ', 'barbar', '4 (trace 1)']);

// Clear the group names:
return _setValue(3, '');
}).then(function() {
assertLabels(['foo ', '1 (trace 1)', 'asdf ', ' ', '4 (trace 1)']);
return _setValue(2, '');
}).then(function() {
// Verify the group names have been cleared:
expect(gd.data[1].transforms[0].styles).toEqual([
{value: {name: 'bar'}, target: 3},
{value: {name: 'asdf'}, target: 4}
{target: 3, value: {name: ''}},
{target: 2, value: {name: ''}}
]);
assertLabels(['foo ', '1 (trace 1)', ' ', ' ', '4 (trace 1)']);

return _setValue(0, '');
}).then(function() {
// Unset the group names:
return _setValue(3, '');
expect(gd.data[0].name).toEqual('');
assertLabels([' ', '1 (trace 1)', ' ', ' ', '4 (trace 1)']);

return _setValue(0, 'boo~~~');
}).then(function() {
return _setValue(4, '');
expect(gd.data[0].name).toEqual('boo~~~');
assertLabels(['boo~~~', '1 (trace 1)', ' ', ' ', '4 (trace 1)']);

return _setValue(2, 'hoo');
}).then(function() {
// Verify the group names have been cleaned up:
expect(gd.data[1].transforms[0].styles).toEqual([
{target: 3, value: {}},
{target: 4, value: {}}
{target: 3, value: {name: ''}},
{target: 2, value: {name: 'hoo'}}
]);
assertLabels(['boo~~~', '1 (trace 1)', 'hoo ', ' ', '4 (trace 1)']);
}).catch(fail).then(done);
});
});
Expand Down

0 comments on commit de22310

Please sign in to comment.