From ba787a05218daaf74312cd57df43fb4f5ef622e0 Mon Sep 17 00:00:00 2001
From: alexcjohnson <alex@plot.ly>
Date: Fri, 27 Apr 2018 14:01:50 -0400
Subject: [PATCH] cleaner handling of empty legend items, and groupby
consistent with regular
---
src/components/legend/draw.js | 77 +++++++++++++++++++------------
src/lib/svg_text_utils.js | 5 +-
src/transforms/groupby.js | 2 +-
test/jasmine/tests/legend_test.js | 63 +++++++++++++++++++------
4 files changed, 101 insertions(+), 46 deletions(-)
diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js
index fbedb04763f..891e2387c4b 100644
--- a/src/components/legend/draw.js
+++ b/src/components/legend/draw.js
@@ -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();
@@ -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');
@@ -108,7 +119,7 @@ module.exports = function draw(gd) {
})
.each(function() {
d3.select(this)
- .call(drawTexts, gd)
+ .call(drawTexts, gd, maxLength)
.call(setupTraceToggle, gd);
});
@@ -352,20 +363,21 @@ 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() {
@@ -373,17 +385,13 @@ function drawTexts(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 = {};
@@ -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;
diff --git a/src/lib/svg_text_utils.js b/src/lib/svg_text_utils.js
index add53dcd3ed..6f08f91371b 100644
--- a/src/lib/svg_text_utils.js
+++ b/src/lib/svg_text_utils.js
@@ -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',
@@ -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;
diff --git a/src/transforms/groupby.js b/src/transforms/groupby.js
index 53d253d4d2e..74ef3a12f8a 100644
--- a/src/transforms/groupby.js
+++ b/src/transforms/groupby.js
@@ -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, {
diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js
index eecef5461e9..940b5c940a2 100644
--- a/test/jasmine/tests/legend_test.js
+++ b/test/jasmine/tests/legend_test.js
@@ -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}
@@ -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);
});
});