From fbfe63824b9476f0454fc9268a9f6a4a86427ced Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Thu, 25 Oct 2018 13:45:03 -0400 Subject: [PATCH 01/11] prune global-level trace attributes that are already defined in a trace --- src/plot_api/plot_schema.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/plot_api/plot_schema.js b/src/plot_api/plot_schema.js index 6f3b5e36494..5be5ff042b0 100644 --- a/src/plot_api/plot_schema.js +++ b/src/plot_api/plot_schema.js @@ -460,11 +460,22 @@ function getTraceAttributes(type) { // make 'type' the first attribute in the object attributes.type = null; + + var copyBaseAttributes = extendDeepAll({}, baseAttributes), + copyModuleAttributes = extendDeepAll({}, _module.attributes); + + // prune global-level trace attributes that are already defined in a trace + exports.crawl(copyModuleAttributes, function(attr, attrName, attrs, level, fullAttrString) { + delete copyBaseAttributes[fullAttrString]; + // Prune undefined attributes + if(attr === undefined) delete copyModuleAttributes[fullAttrString]; + }); + // base attributes (same for all trace types) - extendDeepAll(attributes, baseAttributes); + extendDeepAll(attributes, copyBaseAttributes); // module attributes - extendDeepAll(attributes, _module.attributes); + extendDeepAll(attributes, copyModuleAttributes); // subplot attributes if(basePlotModule.attributes) { From f168ddc7cf054811d69d69028ad242bd2c593194 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Thu, 25 Oct 2018 15:25:20 -0400 Subject: [PATCH 02/11] fix lint --- src/plot_api/plot_schema.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plot_api/plot_schema.js b/src/plot_api/plot_schema.js index 5be5ff042b0..b3f873248a7 100644 --- a/src/plot_api/plot_schema.js +++ b/src/plot_api/plot_schema.js @@ -461,8 +461,8 @@ function getTraceAttributes(type) { attributes.type = null; - var copyBaseAttributes = extendDeepAll({}, baseAttributes), - copyModuleAttributes = extendDeepAll({}, _module.attributes); + var copyBaseAttributes = extendDeepAll({}, baseAttributes); + var copyModuleAttributes = extendDeepAll({}, _module.attributes); // prune global-level trace attributes that are already defined in a trace exports.crawl(copyModuleAttributes, function(attr, attrName, attrs, level, fullAttrString) { From 79995174e03cda5bc1bd76a16591a3dea3de43fc Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Thu, 25 Oct 2018 15:50:54 -0400 Subject: [PATCH 03/11] properly delete nested attributes, update doc `crawl` --- src/plot_api/plot_schema.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plot_api/plot_schema.js b/src/plot_api/plot_schema.js index b3f873248a7..c33538ee25e 100644 --- a/src/plot_api/plot_schema.js +++ b/src/plot_api/plot_schema.js @@ -112,6 +112,7 @@ exports.get = function() { * @param {String} attrName name string * @param {object[]} attrs all the attributes * @param {Number} level the recursion level, 0 at the root + * @param {String} fullAttrString full attribute name (ie 'marker.line') * @param {Number} [specifiedLevel] * The level in the tree, in order to let the callback function detect descend or backtrack, * typically unsupplied (implied 0), just used by the self-recursive call. @@ -466,9 +467,9 @@ function getTraceAttributes(type) { // prune global-level trace attributes that are already defined in a trace exports.crawl(copyModuleAttributes, function(attr, attrName, attrs, level, fullAttrString) { - delete copyBaseAttributes[fullAttrString]; + Lib.nestedProperty(copyBaseAttributes, fullAttrString).set(undefined); // Prune undefined attributes - if(attr === undefined) delete copyModuleAttributes[fullAttrString]; + if(attr === undefined) Lib.nestedProperty(copyModuleAttributes, fullAttrString).set(undefined); }); // base attributes (same for all trace types) From 7f60f6ee7a43d291e248c663ae517ff7a82c29d0 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Fri, 26 Oct 2018 11:53:53 -0400 Subject: [PATCH 04/11] test pruning of global-level trace attributes on sankey --- test/jasmine/bundle_tests/plotschema_test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/jasmine/bundle_tests/plotschema_test.js b/test/jasmine/bundle_tests/plotschema_test.js index d9cc5d00f99..825c537ecc3 100644 --- a/test/jasmine/bundle_tests/plotschema_test.js +++ b/test/jasmine/bundle_tests/plotschema_test.js @@ -362,6 +362,11 @@ describe('plot schema', function() { expect(typeof splomAttrs.yaxes.items.regex).toBe('string'); expect(splomAttrs.yaxes.items.regex).toBe('/^y([2-9]|[1-9][0-9]+)?$/'); }); + + it('should prune unsupported global-level trace attributes', function() { + expect(Plotly.PlotSchema.get().traces.sankey.attributes.hoverinfo.flags.length).toBe(0); + }); + }); describe('getTraceValObject', function() { From 930e4c89ca84c09924d425066d8e31b9bd373d4f Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Mon, 29 Oct 2018 14:36:26 -0400 Subject: [PATCH 05/11] do not coerce hoverlabel if it's undefined at trace level --- src/plots/plots.js | 10 ++++++---- src/traces/parcoords/attributes.js | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index fb27601f276..c8f53089bb2 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1171,10 +1171,12 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac traceOut._dfltShowLegend = false; } - Registry.getComponentMethod( - 'fx', - 'supplyDefaults' - )(traceIn, traceOut, defaultColor, layout); + if(_module && _module.attributes.hoverlabel !== undefined) { + Registry.getComponentMethod( + 'fx', + 'supplyDefaults' + )(traceIn, traceOut, defaultColor, layout); + } // TODO add per-base-plot-module trace defaults step diff --git a/src/traces/parcoords/attributes.js b/src/traces/parcoords/attributes.js index 9c5188eb16f..40990ed7e52 100644 --- a/src/traces/parcoords/attributes.js +++ b/src/traces/parcoords/attributes.js @@ -20,6 +20,8 @@ var templatedArray = require('../../plot_api/plot_template').templatedArray; module.exports = { domain: domainAttrs({name: 'parcoords', trace: true, editType: 'calc'}), + hoverlabel: undefined, + labelfont: fontAttrs({ editType: 'calc', description: 'Sets the font for the `dimension` labels.' From b7679a6fc12cfa75fc0abc62ef087cac88348f1e Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Mon, 29 Oct 2018 15:38:39 -0400 Subject: [PATCH 06/11] make sure the hoverlabel key exists but has an undefined value --- src/plots/plots.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index c8f53089bb2..3383fc55cd4 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1171,7 +1171,9 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac traceOut._dfltShowLegend = false; } - if(_module && _module.attributes.hoverlabel !== undefined) { + if(_module && ('hoverlabel' in _module.attributes) && _module.attributes.hoverlabel === undefined) { + // Do not coerce hoverlabel + } else { Registry.getComponentMethod( 'fx', 'supplyDefaults' From d501bb0bb5d223044a4839ba070fd72e34966d01 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Mon, 29 Oct 2018 16:08:51 -0400 Subject: [PATCH 07/11] prune 'customdata' and 'ids' if undefined at trace-level attribute --- src/plots/plots.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 3383fc55cd4..764f043b95b 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1125,6 +1125,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac // we want even invisible traces to make their would-be subplots visible // so coerce the subplot id(s) now no matter what var _module = plots.getModule(traceOut); + traceOut._module = _module; if(_module) { var basePlotModule = _module.basePlotModule; @@ -1158,9 +1159,21 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac } } + function coerceUnlessPruned(attr, dflt, cb) { + if(_module && (attr in _module.attributes) && _module.attributes[attr] === undefined) { + // Pruned + } else { + if(cb && typeof cb === 'function') { + cb(); + } else { + coerce(attr, dflt); + } + } + } + if(visible) { - coerce('customdata'); - coerce('ids'); + coerceUnlessPruned('customdata'); + coerceUnlessPruned('ids'); if(Registry.traceIs(traceOut, 'showLegend')) { traceOut._dfltShowLegend = true; @@ -1171,14 +1184,12 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac traceOut._dfltShowLegend = false; } - if(_module && ('hoverlabel' in _module.attributes) && _module.attributes.hoverlabel === undefined) { - // Do not coerce hoverlabel - } else { + coerceUnlessPruned('hoverlabel','',function() { Registry.getComponentMethod( 'fx', 'supplyDefaults' )(traceIn, traceOut, defaultColor, layout); - } + }); // TODO add per-base-plot-module trace defaults step From cd0230e871917031675db4617d94855c51c8896f Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Mon, 29 Oct 2018 16:13:31 -0400 Subject: [PATCH 08/11] fix lint --- src/plots/plots.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 764f043b95b..23682134c83 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1184,7 +1184,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac traceOut._dfltShowLegend = false; } - coerceUnlessPruned('hoverlabel','',function() { + coerceUnlessPruned('hoverlabel', '', function() { Registry.getComponentMethod( 'fx', 'supplyDefaults' From da03ceb4b41284f05af33a139d9424a79378d6ea Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 30 Oct 2018 14:01:33 -0400 Subject: [PATCH 09/11] test that hoverlabel doesn't get coerced for parcoords --- test/jasmine/tests/parcoords_test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index 0e1a74406bc..60efe9eb4ea 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -79,6 +79,14 @@ describe('parcoords initialization tests', function() { expect(gd._fullData[0].tickfont).toEqual(expected); expect(gd._fullData[0].rangefont).toEqual(expected); }); + + it('should not coerce hoverlabel', function() { + var gd = Lib.extendDeep({}, mock1); + + supplyAllDefaults(gd); + + expect(gd._fullData[0].hoverlabel).toBeUndefined(); + }); }); describe('parcoords defaults', function() { From 397870b7f78b9325c5796942ced531286fc862f9 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 30 Oct 2018 15:18:14 -0400 Subject: [PATCH 10/11] prune 'transforms' attribute from traces that don't support it --- src/traces/carpet/attributes.js | 1 + src/traces/cone/attributes.js | 2 ++ src/traces/contourcarpet/attributes.js | 3 ++- src/traces/heatmap/attributes.js | 1 + src/traces/mesh3d/attributes.js | 1 + src/traces/pointcloud/attributes.js | 3 ++- src/traces/sankey/attributes.js | 3 ++- src/traces/streamtube/attributes.js | 2 ++ src/traces/surface/attributes.js | 1 + src/traces/table/attributes.js | 3 ++- 10 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/traces/carpet/attributes.js b/src/traces/carpet/attributes.js index ae38a63482d..7584a222028 100644 --- a/src/traces/carpet/attributes.js +++ b/src/traces/carpet/attributes.js @@ -127,4 +127,5 @@ module.exports = { 'Individual pieces can override this.' ].join(' ') }, + transforms: undefined }; diff --git a/src/traces/cone/attributes.js b/src/traces/cone/attributes.js index 799410ab7a8..3e97f066e00 100644 --- a/src/traces/cone/attributes.js +++ b/src/traces/cone/attributes.js @@ -180,4 +180,6 @@ attrs.hoverinfo = extendFlat({}, baseAttrs.hoverinfo, { dflt: 'x+y+z+norm+text+name' }); +attrs.transforms = undefined; + module.exports = attrs; diff --git a/src/traces/contourcarpet/attributes.js b/src/traces/contourcarpet/attributes.js index af0e76b7d29..74cfd6a91ea 100644 --- a/src/traces/contourcarpet/attributes.js +++ b/src/traces/contourcarpet/attributes.js @@ -90,7 +90,8 @@ module.exports = extendFlat({ ].join(' ') }), editType: 'plot' - } + }, + transforms: undefined }, colorscaleAttrs('', { diff --git a/src/traces/heatmap/attributes.js b/src/traces/heatmap/attributes.js index 21d9339a692..242ad3daa59 100644 --- a/src/traces/heatmap/attributes.js +++ b/src/traces/heatmap/attributes.js @@ -111,6 +111,7 @@ module.exports = extendFlat({ 'https://github.com/d3/d3-format/blob/master/README.md#locale_format' ].join(' ') }, + transforms: undefined }, colorscaleAttrs('', { cLetter: 'z', diff --git a/src/traces/mesh3d/attributes.js b/src/traces/mesh3d/attributes.js index 5f844b46d82..5a4439c3a20 100644 --- a/src/traces/mesh3d/attributes.js +++ b/src/traces/mesh3d/attributes.js @@ -165,6 +165,7 @@ module.exports = extendFlat({ 'Overrides *color* and *vertexcolor*.' ].join(' ') }, + transforms: undefined }, colorscaleAttrs('', { diff --git a/src/traces/pointcloud/attributes.js b/src/traces/pointcloud/attributes.js index df1e43900ba..2e8d844775d 100644 --- a/src/traces/pointcloud/attributes.js +++ b/src/traces/pointcloud/attributes.js @@ -141,5 +141,6 @@ module.exports = { editType: 'calc' }, editType: 'calc' - } + }, + transforms: undefined }; diff --git a/src/traces/sankey/attributes.js b/src/traces/sankey/attributes.js index 85f35ac118b..ee4d1cd5530 100644 --- a/src/traces/sankey/attributes.js +++ b/src/traces/sankey/attributes.js @@ -17,7 +17,7 @@ var domainAttrs = require('../../plots/domain').attributes; var extendFlat = require('../../lib/extend').extendFlat; var overrideAll = require('../../plot_api/edit_types').overrideAll; -module.exports = overrideAll({ +var attrs = module.exports = overrideAll({ hoverinfo: extendFlat({}, plotAttrs.hoverinfo, { flags: [], arrayOk: false, @@ -219,3 +219,4 @@ module.exports = overrideAll({ description: 'The links of the Sankey plot.' } }, 'calc', 'nested'); +attrs.transforms = undefined; diff --git a/src/traces/streamtube/attributes.js b/src/traces/streamtube/attributes.js index 0656c180d4a..3583aa1f51a 100644 --- a/src/traces/streamtube/attributes.js +++ b/src/traces/streamtube/attributes.js @@ -152,4 +152,6 @@ attrs.hoverinfo = extendFlat({}, baseAttrs.hoverinfo, { dflt: 'x+y+z+norm+text+name' }); +attrs.transforms = undefined; + module.exports = attrs; diff --git a/src/traces/surface/attributes.js b/src/traces/surface/attributes.js index c4b2f9fa7e9..fdffe43329c 100644 --- a/src/traces/surface/attributes.js +++ b/src/traces/surface/attributes.js @@ -256,3 +256,4 @@ colorscaleAttrs('', { }), 'calc', 'nested'); attrs.x.editType = attrs.y.editType = attrs.z.editType = 'calc+clearAxisTypes'; +attrs.transforms = undefined; diff --git a/src/traces/table/attributes.js b/src/traces/table/attributes.js index feccc72501b..3107a6660f9 100644 --- a/src/traces/table/attributes.js +++ b/src/traces/table/attributes.js @@ -14,7 +14,7 @@ var overrideAll = require('../../plot_api/edit_types').overrideAll; var fontAttrs = require('../../plots/font_attributes'); var domainAttrs = require('../../plots/domain').attributes; -module.exports = overrideAll({ +var attrs = module.exports = overrideAll({ domain: domainAttrs({name: 'table', trace: true}), columnwidth: { @@ -198,3 +198,4 @@ module.exports = overrideAll({ font: extendFlat({}, fontAttrs({arrayOk: true})) } }, 'calc', 'from-root'); +attrs.transforms = undefined; From 55881c65a24c695d75eef21ad7534d20d1411956 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 30 Oct 2018 17:29:58 -0400 Subject: [PATCH 11/11] do not prune `customdata` and `ids` --- src/plots/plots.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 23682134c83..a6d222da003 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1172,8 +1172,8 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac } if(visible) { - coerceUnlessPruned('customdata'); - coerceUnlessPruned('ids'); + coerce('customdata'); + coerce('ids'); if(Registry.traceIs(traceOut, 'showLegend')) { traceOut._dfltShowLegend = true;