8000 Add ability to rename grouped traces by rreusser · Pull Request #1919 · plotly/plotly.js · GitHub
[go: up one dir, main page]
Skip to content

Add ability to rename grouped traces #1919

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 19 commits into from
Aug 15, 2017
Merged

Add ability to rename grouped traces #1919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
aef38e5
Add ability to rename grouped traces
rreusser Jul 27, 2017
d886c9e
Fix l egend naming
rreusser Jul 27, 2017
e6f5e13
Rename namepattern --> nameformat
rreusser Jul 27, 2017
7a2316d
Remove unused mock
rreusser Jul 27, 2017
4e6a0aa
Add custom prop names to keyedContainer
rreusser Jul 28, 2017
952e4ea
Fix groupby groupnames supplyDefaults
rreusser Jul 28, 2017
8f37fc7
Make groupby nameformat dependent on number of traces
rreusser Jul 28, 2017
dd035ef
Add registry methods for transforms
rreusser Aug 1, 2017
7298a0c
Cut out unnecessary redraw in legend update
rreusser Aug 1, 2017
64d2bcd
Remove unneeded inputTraceCount parameter
rreusser Aug 1, 2017
57dfe4a
Add templateString method to perform substitutions
rreusser Aug 1, 2017
c0089ea
Replace ad hoc solution with relinkPrivateKeys
rreusser Aug 1, 2017
42d76e0
Add test for editing legend
rreusser Aug 1, 2017
efcef43
Add comment for relinkPrivateKeys + transforms
rreusser Aug 2, 2017
daac59d
Enhance keyedContainer to accept nested props
rreusser Aug 8, 2017
6aefb41
Fix group -> target in groupby styles
rreusser Aug 8, 2017
be0e693
Extend keyedContainer to handle nested keys
rreusser Aug 11, 2017
4943e3a
Fix keyedContainer behavior to unset entries without removing
rreusser Aug 15, 2017
4b04131
Only blank string unsets legend items
rreusser Aug 15, 2017
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
Prev Previous commit
Next Next commit
Add custom prop names to keyedContainer
  • Loading branch information
@rreusser
rreusser committed Jul 28, 2017
commit 4e6a0aae25824e7613ec1a55193e00207e2d4a16
2 changes: 1 addition & 1 deletion src/components/legend/draw.js
8000
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ function drawTexts(g, gd) {
}
}

var carr = Lib.keyedContainer(fullInput, 'transforms[' + i + '].groupnames');
var carr = Lib.keyedContainer(fullInput, 'transforms[' + i + '].groupnames', 'group', 'name');

if(BLANK_STRING_REGEX.test(origText)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, there is a use case for a blank name - maybe less so for groupby than for regular traces, but sometimes you want the title for one trace to stand in for a few traces below it. You could argue then that this should just be testing for an actual empty string '' but a space ' ' should be allowed as the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, I could just check === '' instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

carr.remove(legendItem.trace._group);
Expand Down
24 changes: 15 additions & 9 deletions src/lib/keyed_container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ var NAME = 1;
var VALUE = 2;
var BOTH = 3;

module.exports = function keyedContainer(baseObj, path) {
module.exports = function keyedContainer(baseObj, path, keyName, valueName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. I like it.

@rreusser can you of other situations where this could be useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. It's also used in group styles, for one, though that case might be slightly more complicated. Also frames, though again, slightly more complicated. It's more a matter of realizing that unless we allow custom keys in the schema, abstracting this pattern seems like the only sane way forward.


keyName = keyName || 'name';
valueName = valueName || 'value';
var i, arr;
var changeTypes = {};

Expand All @@ -31,7 +34,7 @@ module.exports = function keyedContainer(baseObj, path) {
// Construct an index:
var indexLookup = {};
for(i = 0; i < arr.length; i++) {
indexLookup[arr[i].name] = i;
indexLookup[arr[i][keyName]] = i;
}

var obj = {
Expand All @@ -43,18 +46,21 @@ module.exports = function keyedContainer(baseObj, path) {
changeType = BOTH;
idx = arr.length;
indexLookup[name] = idx;
} else if(value !== arr[idx].value) {
} else if(value !== arr[idx][valueName]) {
changeType = VALUE;
}
arr[idx] = {name: name, value: value};
var newValue = {};
newValue[keyName] = name;
newValue[valueName] = value;
arr[idx] = newValue;

changeTypes[idx] = changeTypes[idx] | changeType;

return obj;
},
get: function(name) {
var idx = indexLookup[name];
return idx === undefined ? undefined : arr[idx].value;
return idx === undefined ? undefined : arr[idx][valueName];
},
rename: function(name, newName) {
var idx = indexLookup[name];
Expand All @@ -65,7 +71,7 @@ module.exports = function keyedContainer(baseObj, path) {
indexLookup[newName] = idx;
delete indexLookup[name];

arr[idx].name = newName;
arr[idx][keyName] = newName;

return obj;
},
Expand All @@ -76,7 +82,7 @@ module.exports = function keyedContainer(baseObj, path) {
changeTypes[i] = changeTypes[i] | BOTH;
}
for(i = idx; i < arr.length; i++) {
indexLookup[arr[i].name]--;
indexLookup[arr[i][keyName]]--;
}
arr.splice(idx, 1);
delete(indexLookup[name]);
Expand All @@ -92,10 +98,10 @@ module.exports = function keyedContainer(baseObj, path) {
astr = path + '[' + idx + ']';
if(arr[idx]) {
if(changeTypes[idx] & NAME) {
update[astr + '.name'] = arr[idx].name;
update[astr + '.' + keyName] = arr[idx][keyName];
}
if(changeTypes[idx] & VALUE) {
update[astr + '.value'] = arr[idx].value;
update[astr + '.' + valueName] = arr[idx][valueName];
}
} else {
update[astr] = null;
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 @@ -180,7 +180,7 @@ function transformOne(trace, state) {
}

if(opts.groupnames) {
groupNameObj = Lib.keyedContainer(opts, 'groupnames');
groupNameObj = Lib.keyedContainer(opts, 'groupnames', 'group', 'name');
}

// An index to map group name --> expanded trace index
Expand Down
29 changes: 28 additions & 1 deletion test/jasmine/tests/lib_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,7 @@ describe('Test lib.js:', function() {
});
});

describe('constrcting updates', function() {
describe('constructing updates', function() {
it('constructs updates for addition and modification', function() {
carr.set('foo', 'bar');
carr.set('name1', 'value3');
Expand Down Expand Up @@ -1700,6 +1700,33 @@ describe('Test lib.js:', function() {
});
});
});

describe('with custom named properties', function() {
it('performs standard operations', function() {
var container = {styles: [
{foo: 'name1', bar: 'value1'},
{foo: 'name2', bar: 'value2'}
]};

var carr = Lib.keyedContainer(container, 'styles', 'foo', 'bar');

carr.set('name3', 'value3');
carr.remove('name2');
carr.rename('name1', 'name2');

expect(container).toEqual({styles: [
{foo: 'name2', bar: 'value1'},
{foo: 'name3', bar: 'value3'}
]});

expect(carr.constructUpdate()).toEqual({
'styles[0].foo': 'name2',
'styles[1].foo': 'name3',
'styles[1].bar': 'value3',
'styles[2]': null
});
});
});
});
});

Expand Down
0