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.

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
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
32 changes: 25 additions & 7 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var anchorUtils = require('./anchor_utils');

var SHOWISOLATETIP = true;
var DBLCLICKDELAY = interactConstants.DBLCLICKDELAY;
var BLANK_STRING_REGEX = /^[\s\r]*$/;

module.exports = function draw(gd) {
var fullLayout = gd._fullLayout;
Expand Down Expand Up @@ -392,24 +393,41 @@ function drawTexts(g, gd) {
this.text(text)
.call(textLayout);

var origText = text;

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

var fullInput = legendItem.trace._fullInput || {},
astr;
var transforms, direction;
var fullInput = legendItem.trace._fullInput || {};
var update = {};

// N.B. this block isn't super clean,
// is unfortunately untested at the moment,
// and only works for for 'ohlc' and 'candlestick',
// but should be generalized for other one-to-many transforms
if(['ohlc', 'candlestick'].indexOf(fullInput.type) !== -1) {
var transforms = legendItem.trace.transforms,
direction = transforms[transforms.length - 1].direction;
transforms = legendItem.trace.transforms;
direction = transforms[transforms.length - 1].direction;

update[direction + '.name'] = text;
} else if(Registry.hasTransform(fullInput, 'groupby')) {
var groupbyIndices = Registry.getTransformIndices(fullInput, 'groupby');
var index = groupbyIndices[groupbyIndices.length - 1];

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

astr = direction + '.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);
} else {
carr.set(legendItem.trace._group, text);
}

update = carr.constructUpdate();
} else {
update.name = text;
}
else astr = 'name';

Plotly.restyle(gd, astr, text, traceIndex);
return Plotly.restyle(gd, update, traceIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

fun 👍

});
}
else text.call(textLayout);
Expand Down
31 changes: 31 additions & 0 deletions src/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var BADNUM = numConstants.BADNUM;
var lib = module.exports = {};

lib.nestedProperty = require('./nested_property');
lib.keyedContainer = require('./keyed_container');
lib.isPlainObject = require('./is_plain_object');
lib.isArray = require('./is_array');
lib.mod = require('./mod');
Expand Down Expand Up @@ -727,3 +728,33 @@ lib.numSeparate = function(value, separators, separatethousands) {

return x1 + x2;
};

var TEMPLATE_STRING_REGEX = /%{([^\s%{}]*)}/g;
var SIMPLE_PROPERTY_REGEX = /^\w*$/;

/*
* Substitute values from an object into a string
*
* Examples:
* Lib.templateString('name: %{trace}', {trace: 'asdf'}) --> 'name: asdf'
* Lib.templateString('name: %{trace[0].name}', {trace: [{name: 'asdf'}]}) --> 'name: asdf'
*
* @param {string} input string containing %{...} template strings
* @param {obj} data object containing substitution values
*
* @return {string} templated string
*/

lib.templateString = function(string, obj) {
// Not all that useful, but cache nestedProperty instantiation
// just in case it speeds things up *slightly*:
var getterCache = {};

return string.replace(TEMPLATE_STRING_REGEX, function(dummy, key) {
if(SIMPLE_PROPERTY_REGEX.test(key)) {
return obj[key] || '';
}
getterCache[key] = getterCache[key] || lib.nestedProperty(obj, key).get;
return getterCache[key]() || '';
});
};
153 changes: 153 additions & 0 deletions src/lib/keyed_container.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/**
* Copyright 2012-2017, Plotly, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

var nestedProperty = require('./nested_property');

var SIMPLE_PROPERTY_REGEX = /^\w*$/;

// bitmask for deciding what's updated:
var NONE = 0;
var NAME = 1;
var VALUE = 2;
var BOTH = 3;
var UNSET = 4;

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 = {};

if(path && path.length) { arr = nestedProperty(baseObj, path).get();
} else {
arr = baseObj;
}

path = path || '';
arr = arr || [];

// Construct an index:
var indexLookup = {};
for(i = 0; i < arr.length; i++) {
indexLookup[arr[i][keyName]] = i;
}

var isSimpleValueProp = SIMPLE_PROPERTY_REGEX.test(valueName);

var obj = {
// NB: this does not actually modify the baseObj
set: function(name, value) {
var changeType = value === null ? UNSET : NONE;

var idx = indexLookup[name];
if(idx === undefined) {
changeType = changeType | BOTH;
idx = arr.length;
indexLookup[name] = idx;
} else if(value !== (isSimpleValueProp ? arr[idx][valueName] : nestedProperty(arr[idx], valueName).get())) {
changeType = changeType | VALUE;
}

var newValue = arr[idx] = arr[idx] || {};
newValue[keyName] = name;

if(isSimpleValueProp) {
newValue[valueName] = value;
} else {
nestedProperty(newValue, valueName).set(value);
}

// If it's not an unset, force that bit to be unset. This is all related to the fact
// that undefined and null are a bit specially implemented in nestedProperties.
if(value !== null) {
changeType = changeType & ~UNSET;
Copy link
Contributor

@etpinard etpinard Aug 15, 2017

Choose a reason for hiding this comment

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

What does & ~4 do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm tracking what needs to be updated with a bit mask (which, before adding a bit to track whether it's unset in a way that nestedProperty needs to handle specially, originally made 10% more sense since everything else was strictly an OR operation). & ~4 unsets precisely the 4 bit.

}

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

return obj;
},
get: function(name) {
var idx = indexLookup[name];

if(idx === undefined) {
return undefined;
} else if(isSimpleValueProp) {
return arr[idx][valueName];
} else {
return nestedProperty(arr[idx], valueName).get();
}
},
rename: function(name, newName) {
var idx = indexLookup[name];

if(idx === undefined) return obj;
changeTypes[idx] = changeTypes[idx] | NAME;

indexLookup[newName] = idx;
delete indexLookup[name];

arr[idx][keyName] = newName;

return obj;
},
remove: function(name) {
var idx = indexLookup[name];

if(idx === undefined) return obj;

var object = arr[idx];
if(Object.keys(object).length > 2) {
// This object contains more than just the key/value, so unset
// the value without modifying the entry otherwise:
changeTypes[idx] = changeTypes[idx] | VALUE;
return obj.set(name, null);
}

for(i = idx; i < arr.length; i++) {
changeTypes[i] = changeTypes[i] | BOTH;
}
for(i = idx; i < arr.length; i++) {
indexLookup[arr[i][keyName]]--;
}
arr.splice(idx, 1);
delete(indexLookup[name]);

return obj;
},
constructUpdate: function() {
var astr, idx;
var update = {};
var changed = Object.keys(changeTypes);
for(var i = 0; i < changed.length; i++) {
idx = changed[i];
astr = path + '[' + idx + ']';
if(arr[idx]) {
if(changeTypes[idx] & NAME) {
update[astr + '.' + keyName] = arr[idx][keyName];
}
if(changeTypes[idx] & VALUE) {
if(isSimpleValueProp) {
update[astr + '.' + valueName] = (changeTypes[idx] & UNSET) ? null : arr[idx][valueName];
} else {
update[astr + '.' + valueName] = (changeTypes[idx] & UNSET) ? null : nestedProperty(arr[idx], valueName).get();
}
}
} else {
update[astr] = null;
}
}

return update;
}
};

return obj;
};
4 changes: 4 additions & 0 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,10 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {
var expandedTrace = expandedTraces[j];
var fullExpandedTrace = plots.supplyTraceDefaults(expandedTrace, cnt, fullLayout, i);

// relink private (i.e. underscore) keys expanded trace to full expanded trace so
// that transform supply-default methods can set _ keys for future use.
relinkPrivateKeys(fullExpandedTrace, expandedTrace);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment here e.g.:

relink private (i.e. underscore) keys expanded trace to full expanded trace so that transform supply-default methods can set _ keys for future use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


// mutate uid here using parent uid and expanded index
// to promote consistency between update calls
expandedTrace.uid = fullExpandedTrace.uid = fullTrace.uid + j;
Expand Down
42 changes: 42 additions & 0 deletions src/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,48 @@ exports.traceIs = function(traceType, category) {
return !!_module.categories[category];
};

/**
* Determine if this trace has a transform of the given type and return
* array of matching indices.
*
* @param {object} data
* a trace object (member of data or fullData)
* @param {string} type
* type of trace to test
* @return {array}
* array of matching indices. If none found, returns []
*/
exports.getTransformIndices = function(data, type) {
var indices = [];
var transforms = data.transforms || [];
for(var i = 0; i < transforms.length; i++) {
if(transforms[i].type === type) {
indices.push(i);
}
}
return indices;
};

/**
* Determine if this trace has a transform of the given type
*
* @param {object} data
* a trace object (member of data or fullData)
* @param {string} type
* type of trace to test
* @return {boolean}
*/
exports.hasTransform = function(data, type) {
var transforms = data.transforms || [];
for(var i = 0; i < transforms.length; i++) {
if(transforms[i].type === type) {
return true;
}
}
return false;

};

/**
* Retrieve component module method. Falls back on noop if either the
* module or the method is missing, so the result can always be safely called
Expand Down
Loading