diff --git a/src/lib/index.js b/src/lib/index.js index 37d492dbf23..56f61d66668 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -589,18 +589,36 @@ lib.objectFromPath = function(path, value) { /** * Iterate through an object in-place, converting dotted properties to objects. * - * @example - * lib.expandObjectPaths({'nested.test.path': 'value'}); - * // returns { nested: { test: {path: 'value'}}} + * Examples: + * + * lib.expandObjectPaths({'nested.test.path': 'value'}); + * => { nested: { test: {path: 'value'}}} + * + * It also handles array notation, e.g.: + * + * lib.expandObjectPaths({'foo[1].bar': 'value'}); + * => { foo: [null, {bar: value}] } + * + * It handles merges the results when two properties are specified in parallel: + * + * lib.expandObjectPaths({'foo[1].bar': 10, 'foo[0].bar': 20}); + * => { foo: [{bar: 10}, {bar: 20}] } + * + * It does NOT, however, merge mulitple mutliply-nested arrays:: + * + * lib.expandObjectPaths({'marker[1].range[1]': 5, 'marker[1].range[0]': 4}) + * => { marker: [null, {range: 4}] } */ -// Store this to avoid recompiling regex on every prop since this may happen many -// many times for animations. -// TODO: Premature optimization? Remove? -var dottedPropertyRegex = /^([^\.]*)\../; +// Store this to avoid recompiling regex on *every* prop since this may happen many +// many times for animations. Could maybe be inside the function. Not sure about +// scoping vs. recompilation tradeoff, but at least it's not just inlining it into +// the inner loop. +var dottedPropertyRegex = /^([^\[\.]+)\.(.+)?/; +var indexedPropertyRegex = /^([^\.]+)\[([0-9]+)\](\.)?(.+)?/; lib.expandObjectPaths = function(data) { - var match, key, prop, datum; + var match, key, prop, datum, idx, dest, trailingPath; if(typeof data === 'object' && !Array.isArray(data)) { for(key in data) { if(data.hasOwnProperty(key)) { @@ -611,12 +629,47 @@ lib.expandObjectPaths = function(data) { delete data[key]; data[prop] = lib.extendDeepNoArrays(data[prop] || {}, lib.objectFromPath(key, lib.expandObjectPaths(datum))[prop]); + } else if((match = key.match(indexedPropertyRegex))) { + datum = data[key]; + + prop = match[1]; + idx = parseInt(match[2]); + + delete data[key]; + + data[prop] = data[prop] || []; + + if(match[3] === '.') { + // This is the case where theere are subsequent properties into which + // we must recurse, e.g. transforms[0].value + trailingPath = match[4]; + dest = data[prop][idx] = data[prop][idx] || {}; + + // NB: Extend deep no arrays prevents this from working on multiple + // nested properties in the same object, e.g. + // + // { + // foo[0].bar[1].range + // foo[0].bar[0].range + // } + // + // In this case, the extendDeepNoArrays will overwrite one array with + // the other, so that both properties *will not* be present in the + // result. Fixing this would require a more intelligent tracking + // of changes and merging than extendDeepNoArrays currently accomplishes. + lib.extendDeepNoArrays(dest, lib.objectFromPath(trailingPath, lib.expandObjectPaths(datum))); + } else { + // This is the case where this property is the end of the line, + // e.g. xaxis.range[0] + data[prop][idx] = lib.expandObjectPaths(datum); + } } else { data[key] = lib.expandObjectPaths(data[key]); } } } } + return data; }; diff --git a/src/plots/plots.js b/src/plots/plots.js index c504faa0c37..2610eff994d 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1299,7 +1299,7 @@ plots.modifyFrames = function(gd, operations) { */ plots.computeFrame = function(gd, frameName) { var frameLookup = gd._transitionData._frameHash; - var i, traceIndices, traceIndex, expandedObj, destIndex, copy; + var i, traceIndices, traceIndex, destIndex; var framePtr = frameLookup[frameName]; @@ -1326,9 +1326,7 @@ plots.computeFrame = function(gd, frameName) { // Merge, starting with the last and ending with the desired frame: while((framePtr = frameStack.pop())) { if(framePtr.layout) { - copy = Lib.extendDeepNoArrays({}, framePtr.layout); - expandedObj = Lib.expandObjectPaths(copy); - result.layout = Lib.extendDeepNoArrays(result.layout || {}, expandedObj); + result.layout = plots.extendLayout(result.layout, framePtr.layout); } if(framePtr.data) { @@ -1363,9 +1361,7 @@ plots.computeFrame = function(gd, frameName) { result.traces[destIndex] = traceIndex; } - copy = Lib.extendDeepNoArrays({}, framePtr.data[i]); - expandedObj = Lib.expandObjectPaths(copy); - result.data[destIndex] = Lib.extendDeepNoArrays(result.data[destIndex] || {}, expandedObj); + result.data[destIndex] = plots.extendTrace(result.data[destIndex], framePtr.data[i]); } } } @@ -1373,6 +1369,91 @@ plots.computeFrame = function(gd, frameName) { return result; }; +/** + * Extend an object, treating container arrays very differently by extracting + * their contents and merging them separately. + * + * This exists so that we can extendDeepNoArrays and avoid stepping into data + * arrays without knowledge of the plot schema, but so that we may also manually + * recurse into known container arrays, such as transforms. + * + * See extendTrace and extendLayout below for usage. + */ +plots.extendObjectWithContainers = function(dest, src, containerPaths) { + var containerProp, containerVal, i, j, srcProp, destProp, srcContainer, destContainer; + var copy = Lib.extendDeepNoArrays({}, src || {}); + var expandedObj = Lib.expandObjectPaths(copy); + var containerObj = {}; + + // Step through and extract any container properties. Otherwise extendDeepNoArrays + // will clobber any existing properties with an empty array and then supplyDefaults + // will reset everything to defaults. + if(containerPaths && containerPaths.length) { + for(i = 0; i < containerPaths.length; i++) { + containerProp = Lib.nestedProperty(expandedObj, containerPaths[i]); + containerVal = containerProp.get(); + containerProp.set(null); + Lib.nestedProperty(containerObj, containerPaths[i]).set(containerVal); + } + } + + dest = Lib.extendDeepNoArrays(dest || {}, expandedObj); + + if(containerPaths && containerPaths.length) { + for(i = 0; i < containerPaths.length; i++) { + srcProp = Lib.nestedProperty(containerObj, containerPaths[i]); + srcContainer = srcProp.get(); + + if(!srcContainer) continue; + + destProp = Lib.nestedProperty(dest, containerPaths[i]); + + destContainer = destProp.get(); + if(!Array.isArray(destContainer)) { + destContainer = []; + destProp.set(destContainer); + } + + for(j = 0; j < srcContainer.length; j++) { + destContainer[j] = plots.extendObjectWithContainers(destContainer[j], srcContainer[j]); + } + } + } + + return dest; +}; + +/* + * Extend a trace definition. This method: + * + * 1. directly transfers any array references + * 2. manually recurses into container arrays like transforms + * + * The result is the original object reference with the new contents merged in. + */ +plots.extendTrace = function(destTrace, srcTrace) { + return plots.extendObjectWithContainers(destTrace, srcTrace, ['transforms']); +}; + +/* + * Extend a layout definition. This method: + * + * 1. directly transfers any array references (not critically important for + * layout since there aren't really data arrays) + * 2. manually recurses into container arrays like annotations + * + * The result is the original object reference with the new contents merged in. + */ +plots.extendLayout = function(destLayout, srcLayout) { + return plots.extendObjectWithContainers(destLayout, srcLayout, [ + 'annotations', + 'shapes', + 'images', + 'sliders', + 'updatemenus' + ]); +}; + /** * Transition to a set of new data and layout properties * @@ -1411,16 +1492,7 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) transitionedTraces.push(traceIdx); - // This is a multi-step process. First clone w/o arrays so that - // we're not modifying the original: - var update = Lib.extendDeepNoArrays({}, data[i]); - - // Then expand object paths since we don't obey object-overwrite - // semantics here: - update = Lib.expandObjectPaths(update); - - // Finally apply the update (without copying arrays, of course): - Lib.extendDeepNoArrays(gd.data[traceIndices[i]], update); + gd.data[traceIndices[i]] = plots.extendTrace(gd.data[traceIndices[i]], data[i]); } // Follow the same procedure. Clone it so we don't mangle the input, then diff --git a/test/jasmine/tests/animate_test.js b/test/jasmine/tests/animate_test.js index e9f0a5cfb15..092cb31ca07 100644 --- a/test/jasmine/tests/animate_test.js +++ b/test/jasmine/tests/animate_test.js @@ -84,7 +84,7 @@ describe('Test animate API', function() { }); Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { - Plotly.addFrames(gd, mockCopy.frames); + return Plotly.addFrames(gd, mockCopy.frames); }).then(done); }); diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 2ed3fccf402..1cdf8b20f98 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -537,6 +537,90 @@ describe('Test lib.js:', function() { expect(input).toBe(expanded); expect(origArray).toBe(newArray); }); + + it('expands bracketed array notation', function() { + var input = {'marker[1]': {color: 'red'}}; + var expected = {marker: [undefined, {color: 'red'}]}; + expect(Lib.expandObjectPaths(input)).toEqual(expected); + }); + + it('expands nested arrays', function() { + var input = {'marker[1].range[1]': 5}; + var expected = {marker: [undefined, {range: [undefined, 5]}]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + + it('expands bracketed array with more nested attributes', function() { + var input = {'marker[1]': {'color.alpha': 2}}; + var expected = {marker: [undefined, {color: {alpha: 2}}]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + + it('expands bracketed array notation without further nesting', function() { + var input = {'marker[1]': 8}; + var expected = {marker: [undefined, 8]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + + it('expands bracketed array notation with further nesting', function() { + var input = {'marker[1].size': 8}; + var expected = {marker: [undefined, {size: 8}]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + + it('expands bracketed array notation with further nesting', function() { + var input = {'marker[1].size.magnitude': 8}; + var expected = {marker: [undefined, {size: {magnitude: 8}}]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + + it('combines changes with single array nesting', function() { + var input = {'marker[1].foo': 5, 'marker[0].foo': 4}; + var expected = {marker: [{foo: 4}, {foo: 5}]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + + // TODO: This test is unimplemented since it's a currently-unused corner case. + // Getting the test to pass requires some extension (pun?) to extendDeepNoArrays + // that's intelligent enough to only selectively merge *some* arrays, in particular + // not data arrays but yes on arrays that were previously expanded. This is a bit + // tricky to get to work just right and currently doesn't have any known use since + // container arrays are not multiply nested. + // + // Additional notes on what works or what doesn't work. This case does *not* work + // because the two nested arrays that would result from the expansion need to be + // deep merged. + // + // Lib.expandObjectPaths({'marker.range[0]': 5, 'marker.range[1]': 2}) + // + // // => {marker: {range: [null, 2]}} + // + // This case *does* work becuase the array merging does not require a deep extend: + // + // Lib.expandObjectPaths({'range[0]': 5, 'range[1]': 2} + // + // // => {range: [5, 2]} + // + // Finally note that this case works fine becuase there's no merge necessary: + // + // Lib.expandObjectPaths({'marker.range[1]': 2}) + // + // // => {marker: {range: [null, 2]}} + // + /* + it('combines changes', function() { + var input = {'marker[1].range[1]': 5, 'marker[1].range[0]': 4}; + var expected = {marker: [undefined, {range: [4, 5]}]}; + var computed = Lib.expandObjectPaths(input); + expect(computed).toEqual(expected); + }); + */ }); describe('coerce', function() { diff --git a/test/jasmine/tests/transition_test.js b/test/jasmine/tests/transition_test.js index 63deb99a688..ff54580b747 100644 --- a/test/jasmine/tests/transition_test.js +++ b/test/jasmine/tests/transition_test.js @@ -65,6 +65,41 @@ function runTests(transitionDuration) { }).catch(fail).then(done); }); + it('transitions a transform', function(done) { + Plotly.restyle(gd, { + 'transforms[0]': { + enabled: true, + type: 'filter', + operation: '<', + filtersrc: 'x', + value: 10 + } + }, [0]).then(function() { + expect(gd._fullData[0].transforms).toEqual([{ + enabled: true, + type: 'filter', + operation: '<', + filtersrc: 'x', + value: 10 + }]); + + return Plots.transition(gd, [{ + 'transforms[0].operation': '>' + }], null, [0], + {redraw: true, duration: transitionDuration}, + {duration: transitionDuration, easing: 'cubic-in-out'} + ); + }).then(function() { + expect(gd._fullData[0].transforms).toEqual([{ + enabled: true, + type: 'filter', + operation: '>', + filtersrc: 'x', + value: 10 + }]); + }).catch(fail).then(done); + }); + // This doesn't really test anything that the above tests don't cover, but it combines // the behavior and attempts to ensure chaining and events happen in the correct order. it('transitions may be chained', function(done) {