Skip to content

Blank editable legend item fixes #2587

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 1 commit into from
Apr 27, 2018
Merged

Blank editable legend item fixes #2587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
77 changes: 48 additions & 29 deletions src/components/legend/draw.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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