clean up editTypes/impliedEdits and formalize & document their schema definition

alexcjohnson · alexcjohnson · commit 96cc57fe5a7c · 2017-09-14T21:42:01.000-04:00
diff --git a/src/lib/coerce.js b/src/lib/coerce.js
@@ -18,7 +18,7 @@ var colorscaleNames = Object.keys(require('../components/colorscale/scales'));
 var nestedProperty = require('./nested_property');
 var counterRegex = require('./regex').counter;
 
-exports.valObjects = {
+exports.valObjectMeta = {
     data_array: {
         // You can use *dflt=[] to force said array to exist though.
         description: [
@@ -283,7 +283,7 @@ exports.valObjects = {
  * Ensures that container[attribute] has a valid value.
  *
  * attributes[attribute] is an object with possible keys:
- * - valType: data_array, enumerated, boolean, ... as in valObjects
+ * - valType: data_array, enumerated, boolean, ... as in valObjectMeta
  * - values: (enumerated only) array of allowed vals
  * - min, max: (number, integer only) inclusive bounds on allowed vals
  *      either or both may be omitted
@@ -310,7 +310,7 @@ exports.coerce = function(containerIn, containerOut, attributes, attribute, dflt
         return v;
     }
 
-    exports.valObjects[opts.valType].coerceFunction(v, propOut, dflt, opts);
+    exports.valObjectMeta[opts.valType].coerceFunction(v, propOut, dflt, opts);
 
     return propOut.get();
 };
@@ -377,12 +377,12 @@ exports.coerceHoverinfo = function(traceIn, traceOut, layoutOut) {
 };
 
 exports.validate = function(value, opts) {
-    var valObject = exports.valObjects[opts.valType];
+    var valObjectDef = exports.valObjectMeta[opts.valType];
 
     if(opts.arrayOk && Array.isArray(value)) return true;
 
-    if(valObject.validateFunction) {
-        return valObject.validateFunction(value, opts);
+    if(valObjectDef.validateFunction) {
+        return valObjectDef.validateFunction(value, opts);
     }
 
     var failed = {},
@@ -391,6 +391,6 @@ exports.validate = function(value, opts) {
 
     // 'failed' just something mutable that won't be === anything else
 
-    valObject.coerceFunction(value, propMock, failed, opts);
+    valObjectDef.coerceFunction(value, propMock, failed, opts);
     return out !== failed;
 };
diff --git a/src/lib/index.js b/src/lib/index.js
@@ -29,7 +29,7 @@ lib.relinkPrivateKeys = require('./relink_private');
 lib.ensureArray = require('./ensure_array');
 
 var coerceModule = require('./coerce');
-lib.valObjects = coerceModule.valObjects;
+lib.valObjectMeta = coerceModule.valObjectMeta;
 lib.coerce = coerceModule.coerce;
 lib.coerce2 = coerceModule.coerce2;
 lib.coerceFont = coerceModule.coerceFont;
diff --git a/src/plot_api/edit_types.js b/src/plot_api/edit_types.js
@@ -12,42 +12,69 @@ var Lib = require('../lib');
 var extendFlat = Lib.extendFlat;
 var isPlainObject = Lib.isPlainObject;
 
+var traceOpts = {
+    valType: 'flaglist',
+    extras: ['none'],
+    flags: ['calc', 'calcIfAutorange', 'plot', 'style', 'colorbars'],
+    description: [
+        'trace attributes should include an `editType` string matching this flaglist.',
+        '*calc* is the most extensive: a full `Plotly.plot` starting by clearing `gd.calcdata`',
+        'to force it to be regenerated',
+        '*calcIfAutorange* does a full `Plotly.plot`, but only clears and redoes `gd.calcdata`',
+        'if there is at least one autoranged axis.',
+        '*plot* calls `Plotly.plot` but without first clearing `gd.calcdata`.',
+        '*style* only calls `module.style` for all trace modules and redraws the legend.',
+        '*colorbars* only redraws colorbars.'
+    ].join(' ')
+};
+
+var layoutOpts = {
+    valType: 'flaglist',
+    extras: ['none'],
+    flags: [
+        'calc', 'calcIfAutorange', 'plot', 'legend', 'ticks',
+        'layoutstyle', 'modebar', 'camera', 'arraydraw'
+    ],
+    description: [
+        'layout attributes should include an `editType` string matching this flaglist.',
+        '*calc* is the most extensive: a full `Plotly.plot` starting by clearing `gd.calcdata`',
+        'to force it to be regenerated',
+        '*calcIfAutorange* does a full `Plotly.plot`, but only clears and redoes `gd.calcdata`',
+        'if there is at least one autoranged axis.',
+        '*plot* calls `Plotly.plot` but without first clearing `gd.calcdata`.',
+        '*legend* only redraws the legend.',
+        '*ticks* only redraws axis ticks, labels, and gridlines.',
+        '*layoutstyle* reapplies global and SVG cartesian axis styles.',
+        '*modebar* just updates the modebar.',
+        '*camera* just updates the camera settings for gl3d scenes.',
+        '*arraydraw* allows component arrays to invoke the redraw routines just for the',
+        'component(s) that changed.'
+    ].join(' ')
+};
+
+// flags for inside restyle/relayout include a few extras
+// that shouldn't be used in attributes, to deal with certain
+// combinations and conditionals efficiently
+var traceEditTypeFlags = traceOpts.flags.slice()
+    .concat(['clearCalc', 'fullReplot']);
+
+var layoutEditTypeFlags = layoutOpts.flags.slice()
+    .concat('layoutReplot');
+
 module.exports = {
+    traces: traceOpts,
+    layout: layoutOpts,
     /*
      * default (all false) edit flags for restyle (traces)
      * creates a new object each call, so the caller can mutate freely
      */
-    traces: function() {
-        return {
-            calc: false,
-            calcIfAutorange: false,
-            plot: false,
-            style: false,
-            colorbars: false,
-            autorangeOn: false,
-            clearCalc: false,
-            fullReplot: false
-        };
-    },
+    traceFlags: function() { return falseObj(traceEditTypeFlags); },
 
     /*
      * default (all false) edit flags for relayout
      * creates a new object each call, so the caller can mutate freely
      */
-    layout: function() {
-        return {
-            legend: false,
-            ticks: false,
-            layoutstyle: false,
-            plot: false,
-            calc: false,
-            calcIfAutorange: false,
-            modebar: false,
-            camera: false,
-            arraydraw: false,
-            layoutReplot: false
-        };
-    },
+    layoutFlags: function() { return falseObj(layoutEditTypeFlags); },
 
     /*
      * update `flags` with the `editType` values found in `attr`
@@ -65,6 +92,12 @@ module.exports = {
     overrideAll: overrideAll
 };
 
+function falseObj(keys) {
+    var out = {};
+    for(var i = 0; i < keys.length; i++) out[keys[i]] = false;
+    return out;
+}
+
 /**
  * For attributes that are largely copied from elsewhere into a plot type that doesn't
  * support partial redraws - overrides the editType field of all attributes in the object
diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js
@@ -1331,7 +1331,7 @@ function _restyle(gd, aobj, _traces) {
     var traces = helpers.coerceTraceIndices(gd, _traces);
 
     // initialize flags
-    var flags = editTypes.traces();
+    var flags = editTypes.traceFlags();
 
     // copies of the change (and previous values of anything affected)
     // for the undo / redo queue
@@ -1585,9 +1585,14 @@ function _restyle(gd, aobj, _traces) {
     }
 
     // do we need to force a recalc?
-    Plotly.Axes.list(gd).forEach(function(ax) {
-        if(ax.autorange) flags.autorangeOn = true;
-    });
+    var autorangeOn = false;
+    var axList = Plotly.Axes.list(gd);
+    for(i = 0; i < axList.length; i++) {
+        if(axList[i].autorange) {
+            autorangeOn = true;
+            break;
+        }
+    }
 
     // check axes we've flagged for possible deletion
     // flagAxForDelete is a hash so we can make sure we only get each axis once
@@ -1610,7 +1615,7 @@ function _restyle(gd, aobj, _traces) {
     }
 
     // combine a few flags together;
-    if(flags.calc || (flags.calcIfAutorange && flags.autorangeOn)) {
+    if(flags.calc || (flags.calcIfAutorange && autorangeOn)) {
         flags.clearCalc = true;
     }
     if(flags.calc || flags.plot || flags.calcIfAutorange) {
@@ -1735,7 +1740,7 @@ function _relayout(gd, aobj) {
     }
 
     // initialize flags
-    var flags = editTypes.layout();
+    var flags = editTypes.layoutFlags();
 
     // copies of the change (and previous values of anything affected)
     // for the undo / redo queue
diff --git a/src/plot_api/plot_schema.js b/src/plot_api/plot_schema.js
@@ -63,11 +63,29 @@ exports.get = function() {
 
     return {
         defs: {
-            valObjects: Lib.valObjects,
+            valObjects: Lib.valObjectMeta,
             metaKeys: UNDERSCORE_ATTRS.concat(['description', 'role']),
-            editTypes: {
-                traces: editTypes.traces(),
-                layout: editTypes.layout()
+            editType: {
+                traces: editTypes.traces,
+                layout: editTypes.layout
+            },
+            impliedEdits: {
+                description: [
+                    'Sometimes when an attribute is changed, other attributes',
+                    'must be altered as well in order to achieve the intended',
+                    'result. For example, when `range` is specified, it is',
+                    'important to set `autorange` to `false` or the new `range`',
+                    'value would be lost in the redraw. `impliedEdits` is the',
+                    'mechanism to do this: `impliedEdits: {autorange: false}`.',
+                    'Each key is a relative paths to the attribute string to',
+                    'change, using *^* to ascend into the parent container,',
+                    'for example `range[0]` has `impliedEdits: {*^autorange*: false}`.',
+                    'A value of `undefined` means that the attribute will not be',
+                    'changed, but its previous value should be recorded in case',
+                    'we want to reverse this change later. For example, `autorange`',
+                    'has `impliedEdits: {*range[0]*: undefined, *range[1]*:undefined}',
+                    'because the range will likely be changed by redraw.'
+                ].join(' ')
             }
         },
 
diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js
@@ -2154,7 +2154,7 @@ describe('plot_api helpers', function() {
 
 describe('plot_api edit_types', function() {
     it('initializes flags with all false', function() {
-        ['traces', 'layout'].forEach(function(container) {
+        ['traceFlags', 'layoutFlags'].forEach(function(container) {
             var initFlags = editTypes[container]();
             Object.keys(initFlags).forEach(function(key) {
                 expect(initFlags[key]).toBe(false, container + '.' + key);
@@ -2163,35 +2163,35 @@ describe('plot_api edit_types', function() {
     });
 
     it('makes no changes if editType is not included', function() {
-        var flags = {docalc: false, dostyle: true};
+        var flags = {calc: false, style: true};
 
         editTypes.update(flags, {
             valType: 'boolean',
             dflt: true,
             role: 'style'
         });
 
-        expect(flags).toEqual({docalc: false, dostyle: true});
+        expect(flags).toEqual({calc: false, style: true});
 
         editTypes.update(flags, {
             family: {valType: 'string', dflt: 'Comic sans'},
             size: {valType: 'number', dflt: 96},
             color: {valType: 'color', dflt: 'red'}
         });
 
-        expect(flags).toEqual({docalc: false, dostyle: true});
+        expect(flags).toEqual({calc: false, style: true});
     });
 
     it('gets updates from the outer object and ignores nested items', function() {
-        var flags = {docalc: false, dolegend: true};
+        var flags = {calc: false, legend: true};
 
         editTypes.update(flags, {
-            editType: 'docalc+dostyle',
+            editType: 'calc+style',
             valType: 'number',
             dflt: 1,
             role: 'style'
         });
 
-        expect(flags).toEqual({docalc: true, dolegend: true, dostyle: true});
+        expect(flags).toEqual({calc: true, legend: true, style: true});
     });
 });
diff --git a/test/jasmine/tests/plotschema_test.js b/test/jasmine/tests/plotschema_test.js
@@ -242,17 +242,6 @@ describe('plot schema', function() {
     });
 
     it('has valid `editType` in all attributes and containers', function() {
-        var traceEditTypeOpts = {
-            valType: 'flaglist',
-            extras: ['none'],
-            flags: Object.keys(editTypes.traces)
-        };
-        var layoutEditTypeOpts = {
-            valType: 'flaglist',
-            extras: ['none'],
-            flags: Object.keys(editTypes.layout)
-        };
-
         function shouldHaveEditType(attr, attrName) {
             // ensure any object (container or regular val object) has editType
             // array containers have extra nesting where editType would be redundant
@@ -262,21 +251,21 @@ describe('plot schema', function() {
 
         assertTraceSchema(function(attr, attrName, attrs, level, attrString) {
             if(shouldHaveEditType(attr, attrName)) {
-                expect(Lib.validate(attr.editType, traceEditTypeOpts))
+                expect(Lib.validate(attr.editType, editTypes.traces))
                     .toBe(true, attrString + ': ' + JSON.stringify(attr.editType));
             }
         });
 
         assertTransformSchema(function(attr, attrName, attrs, level, attrString) {
             if(shouldHaveEditType(attr, attrName)) {
-                expect(Lib.validate(attr.editType, traceEditTypeOpts))
+                expect(Lib.validate(attr.editType, editTypes.traces))
                     .toBe(true, attrString + ': ' + JSON.stringify(attr.editType));
             }
         });
 
         assertLayoutSchema(function(attr, attrName, attrs, level, attrString) {
             if(shouldHaveEditType(attr, attrName)) {
-                expect(Lib.validate(attr.editType, layoutEditTypeOpts))
+                expect(Lib.validate(attr.editType, editTypes.layout))
                     .toBe(true, attrString + ': ' + JSON.stringify(attr.editType));
             }
         });
