From 663809297ddde28e13bf2d08bc5a8c909bbe0245 Mon Sep 17 00:00:00 2001 From: RoboErikG Date: Mon, 7 Oct 2019 11:38:32 -0700 Subject: [PATCH] Set correct defaults for Fields (#3179) null was being converted to 0 by Number() when it should cause the default value to be set instead. This updates FieldNumber and FieldAngle to handle nulls correctly. Also update jsdoc. Fixes #3177 --- core/field.js | 2 +- core/field_angle.js | 30 ++++++++++++----- core/field_number.js | 57 ++++++++++++++++++++------------ core/field_textinput.js | 2 +- tests/mocha/field_angle_test.js | 33 ++++++++++++++++++ tests/mocha/field_number_test.js | 18 ++++++++++ 6 files changed, 109 insertions(+), 33 deletions(-) diff --git a/core/field.js b/core/field.js index ebc8813f6..ad7477830 100644 --- a/core/field.js +++ b/core/field.js @@ -39,7 +39,7 @@ goog.require('Blockly.utils.style'); /** * Abstract class for an editable field. * @param {*} value The initial value of the field. - * @param {Function=} opt_validator A function that is called to validate + * @param {?Function=} opt_validator A function that is called to validate * changes to the field's value. Takes in a value & returns a validated * value, or null to abort the change. * @param {Object=} opt_config A map of options used to configure the field. See diff --git a/core/field_angle.js b/core/field_angle.js index e383598d8..a8f9fb869 100644 --- a/core/field_angle.js +++ b/core/field_angle.js @@ -172,20 +172,32 @@ Blockly.FieldAngle.prototype.configure_ = function(config) { if (typeof clockwise == 'boolean') { this.clockwise_ = clockwise; } - var offset = Number(config['offset']); - if (!isNaN(offset)) { - this.offset_ = offset; + + var offset = config['offset']; + if (offset != null) { + offset = Number(offset); + if (!isNaN(offset)) { + this.offset_ = offset; + } } - var wrap = Number(config['wrap']); - if (!isNaN(wrap)) { - this.wrap_ = wrap; + var wrap = config['wrap']; + if (wrap != null) { + wrap = Number(wrap); + if (!isNaN(wrap)) { + this.wrap_ = wrap; + } } - var round = Number(config['round']); - if (!isNaN(round)) { - this.round_ = round; + var round = config['round']; + if (round != null) { + round = Number(round); + if (!isNaN(round)) { + this.round_ = round; + } } }; + + /** * Create the block UI for this field. * @package diff --git a/core/field_number.js b/core/field_number.js index bbe340896..396cac7f5 100644 --- a/core/field_number.js +++ b/core/field_number.js @@ -32,10 +32,10 @@ goog.require('Blockly.utils.object'); * Class for an editable number field. * @param {string|number=} opt_value The initial value of the field. Should cast * to a number. Defaults to 0. - * @param {(string|number)=} opt_min Minimum value. - * @param {(string|number)=} opt_max Maximum value. - * @param {(string|number)=} opt_precision Precision for value. - * @param {Function=} opt_validator A function that is called to validate + * @param {?(string|number)=} opt_min Minimum value. + * @param {?(string|number)=} opt_max Maximum value. + * @param {?(string|number)=} opt_precision Precision for value. + * @param {?Function=} opt_validator A function that is called to validate * changes to the field's value. Takes in a number & returns a validated * number, or null to abort the change. * @param {Object=} opt_config A map of options used to configure the field. @@ -124,9 +124,9 @@ Blockly.FieldNumber.prototype.configure_ = function(config) { * values. That is, the user's value will rounded to the closest multiple of * precision. The least significant digit place is inferred from the precision. * Integers values can be enforces by choosing an integer precision. - * @param {number|string|undefined} min Minimum value. - * @param {number|string|undefined} max Maximum value. - * @param {number|string|undefined} precision Precision for value. + * @param {?(number|string|undefined)} min Minimum value. + * @param {?(number|string|undefined)} max Maximum value. + * @param {?(number|string|undefined)} precision Precision for value. */ Blockly.FieldNumber.prototype.setConstraints = function(min, max, precision) { this.setMinInternal_(min); @@ -137,7 +137,7 @@ Blockly.FieldNumber.prototype.setConstraints = function(min, max, precision) { /** * Sets the minimum value this field can contain. Updates the value to reflect. - * @param {number|string|undefined} min Minimum value. + * @param {?(number|string|undefined)} min Minimum value. */ Blockly.FieldNumber.prototype.setMin = function(min) { this.setMinInternal_(min); @@ -147,13 +147,17 @@ Blockly.FieldNumber.prototype.setMin = function(min) { /** * Sets the minimum value this field can contain. Called internally to avoid * value updates. - * @param {number|string|undefined} min Minimum value. + * @param {?(number|string|undefined)} min Minimum value. * @private */ Blockly.FieldNumber.prototype.setMinInternal_ = function(min) { - min = Number(min); - if (!isNaN(min)) { - this.min_ = min; + if (min == null) { + this.min_ = -Infinity; + } else { + min = Number(min); + if (!isNaN(min)) { + this.min_ = min; + } } }; @@ -168,7 +172,7 @@ Blockly.FieldNumber.prototype.getMin = function() { /** * Sets the maximum value this field can contain. Updates the value to reflect. - * @param {number|string|undefined} max Maximum value. + * @param {?(number|string|undefined)} max Maximum value. */ Blockly.FieldNumber.prototype.setMax = function(max) { this.setMaxInternal_(max); @@ -178,13 +182,17 @@ Blockly.FieldNumber.prototype.setMax = function(max) { /** * Sets the maximum value this field can contain. Called internally to avoid * value updates. - * @param {number|string|undefined} max Maximum value. + * @param {?(number|string|undefined)} max Maximum value. * @private */ Blockly.FieldNumber.prototype.setMaxInternal_ = function(max) { - max = Number(max); - if (!isNaN(max)) { - this.max_ = max; + if (max == null) { + this.max_ = Infinity; + } else { + max = Number(max); + if (!isNaN(max)) { + this.max_ = max; + } } }; @@ -200,7 +208,7 @@ Blockly.FieldNumber.prototype.getMax = function() { /** * Sets the precision of this field's value, i.e. the number to which the * value is rounded. Updates the field to reflect. - * @param {number|string|undefined} precision The number to which the + * @param {?(number|string|undefined)} precision The number to which the * field's value is rounded. */ Blockly.FieldNumber.prototype.setPrecision = function(precision) { @@ -211,14 +219,19 @@ Blockly.FieldNumber.prototype.setPrecision = function(precision) { /** * Sets the precision of this field's value. Called internally to avoid * value updates. - * @param {number|string|undefined} precision The number to which the + * @param {?(number|string|undefined)} precision The number to which the * field's value is rounded. * @private */ Blockly.FieldNumber.prototype.setPrecisionInternal_ = function(precision) { - precision = Number(precision); - if (!isNaN(precision)) { - this.precision_ = precision; + if (precision == null) { + // Number(precision) would also be 0, but set explicitly to be clear. + this.precision_ = 0; + } else { + precision = Number(precision); + if (!isNaN(precision)) { + this.precision_ = precision; + } } var precisionString = this.precision_.toString(); diff --git a/core/field_textinput.js b/core/field_textinput.js index f6312ef3e..46bcb7bc5 100644 --- a/core/field_textinput.js +++ b/core/field_textinput.js @@ -42,7 +42,7 @@ goog.require('Blockly.utils.userAgent'); * Class for an editable text field. * @param {string=} opt_value The initial value of the field. Should cast to a * string. Defaults to an empty string if null or undefined. - * @param {Function=} opt_validator A function that is called to validate + * @param {?Function=} opt_validator A function that is called to validate * changes to the field's value. Takes in a string & returns a validated * string, or null to abort the change. * @param {Object=} opt_config A map of options used to configure the field. diff --git a/tests/mocha/field_angle_test.js b/tests/mocha/field_angle_test.js index 500c35599..aeb1f8930 100644 --- a/tests/mocha/field_angle_test.js +++ b/tests/mocha/field_angle_test.js @@ -305,6 +305,17 @@ suite('Angle Fields', function() { var field = new Blockly.FieldAngle(); chai.assert.equal(field.offset_, 90); }); + test('Null', function() { + // Note: Generally constants should be set at compile time, not + // runtime (since they are constants) but for testing purposes we + // can do this. + Blockly.FieldAngle.OFFSET = 90; + var field = Blockly.FieldAngle.fromJson({ + value: 0, + offset: null + }); + chai.assert.equal(field.offset_, 90); + }); }); suite('Wrap', function() { test('JS Configuration', function() { @@ -328,6 +339,17 @@ suite('Angle Fields', function() { var field = new Blockly.FieldAngle(); chai.assert.equal(field.wrap_, 180); }); + test('Null', function() { + // Note: Generally constants should be set at compile time, not + // runtime (since they are constants) but for testing purposes we + // can do this. + Blockly.FieldAngle.WRAP = 180; + var field = Blockly.FieldAngle.fromJson({ + value: 0, + wrap: null + }); + chai.assert.equal(field.wrap_, 180); + }); }); suite('Round', function() { test('JS Configuration', function() { @@ -351,6 +373,17 @@ suite('Angle Fields', function() { var field = new Blockly.FieldAngle(); chai.assert.equal(field.round_, 30); }); + test('Null', function() { + // Note: Generally constants should be set at compile time, not + // runtime (since they are constants) but for testing purposes we + // can do this. + Blockly.FieldAngle.ROUND = 30; + var field = Blockly.FieldAngle.fromJson({ + value: 0, + round: null + }); + chai.assert.equal(field.round_, 30); + }); }); suite('Mode', function() { suite('Compass', function() { diff --git a/tests/mocha/field_number_test.js b/tests/mocha/field_number_test.js index ca2a94dc3..1a4d9f700 100644 --- a/tests/mocha/field_number_test.js +++ b/tests/mocha/field_number_test.js @@ -247,6 +247,12 @@ suite('Number Fields', function() { numberField.setValue(123.456); assertValue(numberField, 123); }); + test('null', function() { + var numberField = new Blockly.FieldNumber + .fromJson({ precision: null}); + numberField.setValue(123.456); + assertValue(numberField, 123.456); + }); }); suite('Min', function() { test('-10', function() { @@ -276,6 +282,12 @@ suite('Number Fields', function() { numberField.setValue(20); assertValue(numberField, 20); }); + test('null', function() { + var numberField = new Blockly.FieldNumber + .fromJson({ min: null}); + numberField.setValue(-20); + assertValue(numberField, -20); + }); }); suite('Max', function() { test('-10', function() { @@ -305,6 +317,12 @@ suite('Number Fields', function() { numberField.setValue(20); assertValue(numberField, 10); }); + test('null', function() { + var numberField = new Blockly.FieldNumber + .fromJson({ max: null}); + numberField.setValue(20); + assertValue(numberField, 20); + }); }); }); });