From 08d57853be9f414e3fa9dcbb55aca20d4a6c3fe2 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 27 Apr 2023 10:05:53 -0700 Subject: [PATCH] chore: remove private underscores in field_variable.ts (#7022) * chore: remove private underscores in field_variable.ts * chore: remove stale TODO --- core/field_variable.ts | 57 +++++++++++++++--------------- tests/mocha/field_variable_test.js | 22 ++++++------ 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/core/field_variable.ts b/core/field_variable.ts index 5bde39de1..f1c6e8dda 100644 --- a/core/field_variable.ts +++ b/core/field_variable.ts @@ -37,7 +37,7 @@ export class FieldVariable extends FieldDropdown { defaultVariableName: string; /** The type of the default variable for this field. */ - private defaultType_ = ''; + private defaultType = ''; /** * All of the types of variables that will be available in this field's @@ -47,7 +47,7 @@ export class FieldVariable extends FieldDropdown { protected override size_: Size; /** The variable model associated with this field. */ - private variable_: VariableModel|null = null; + private variable: VariableModel|null = null; /** * Serializable fields are saved by the serializer, non-serializable fields @@ -101,7 +101,7 @@ export class FieldVariable extends FieldDropdown { if (config) { this.configure_(config); } else { - this.setTypes_(variableTypes, defaultType); + this.setTypes(variableTypes, defaultType); } if (validator) { this.setValidator(validator); @@ -115,7 +115,7 @@ export class FieldVariable extends FieldDropdown { */ protected override configure_(config: FieldVariableConfig) { super.configure_(config); - this.setTypes_(config.variableTypes, config.defaultType); + this.setTypes(config.variableTypes, config.defaultType); } /** @@ -130,11 +130,11 @@ export class FieldVariable extends FieldDropdown { if (!block) { throw new UnattachedFieldError(); } - if (this.variable_) { + if (this.variable) { return; // Initialization already happened. } const variable = Variables.getOrCreateVariablePackage( - block.workspace, null, this.defaultVariableName, this.defaultType_); + block.workspace, null, this.defaultVariableName, this.defaultType); // Don't call setValue because we don't want to cause a rerender. this.doValueUpdate_(variable.getId()); } @@ -195,10 +195,10 @@ export class FieldVariable extends FieldDropdown { // Make sure the variable is initialized. this.initModel(); - fieldElement.id = this.variable_!.getId(); - fieldElement.textContent = this.variable_!.name; - if (this.variable_!.type) { - fieldElement.setAttribute('variabletype', this.variable_!.type); + fieldElement.id = this.variable!.getId(); + fieldElement.textContent = this.variable!.name; + if (this.variable!.type) { + fieldElement.setAttribute('variabletype', this.variable!.type); } return fieldElement; } @@ -219,10 +219,10 @@ export class FieldVariable extends FieldDropdown { } // Make sure the variable is initialized. this.initModel(); - const state = {'id': this.variable_!.getId()}; + const state = {'id': this.variable!.getId()}; if (doFullSerialization) { - (state as AnyDuringMigration)['name'] = this.variable_!.name; - (state as AnyDuringMigration)['type'] = this.variable_!.type; + (state as AnyDuringMigration)['name'] = this.variable!.name; + (state as AnyDuringMigration)['type'] = this.variable!.type; } return state; } @@ -266,7 +266,7 @@ export class FieldVariable extends FieldDropdown { * @returns Current variable's ID. */ override getValue(): string|null { - return this.variable_ ? this.variable_.getId() : null; + return this.variable ? this.variable.getId() : null; } /** @@ -276,7 +276,7 @@ export class FieldVariable extends FieldDropdown { * is selected. */ override getText(): string { - return this.variable_ ? this.variable_.name : ''; + return this.variable ? this.variable.name : ''; } /** @@ -288,7 +288,7 @@ export class FieldVariable extends FieldDropdown { * @internal */ getVariable(): VariableModel|null { - return this.variable_; + return this.variable; } /** @@ -303,7 +303,7 @@ export class FieldVariable extends FieldDropdown { // Validators shouldn't operate on the initial setValue call. // Normally this is achieved by calling setValidator after setValue, but // this is not a possibility with variable fields. - if (this.variable_) { + if (this.variable) { return this.validator_; } return null; @@ -334,7 +334,7 @@ export class FieldVariable extends FieldDropdown { } // Type Checks. const type = variable.type; - if (!this.typeIsAllowed_(type)) { + if (!this.typeIsAllowed(type)) { console.warn( 'Variable type doesn\'t match this field! Type was ' + type); return null; @@ -355,7 +355,7 @@ export class FieldVariable extends FieldDropdown { if (!block) { throw new UnattachedFieldError(); } - this.variable_ = Variables.getVariable(block.workspace, newId as string); + this.variable = Variables.getVariable(block.workspace, newId as string); super.doValueUpdate_(newId); } @@ -365,8 +365,8 @@ export class FieldVariable extends FieldDropdown { * @param type The type to check. * @returns True if the type is in the list of allowed types. */ - private typeIsAllowed_(type: string): boolean { - const typeList = this.getVariableTypes_(); + private typeIsAllowed(type: string): boolean { + const typeList = this.getVariableTypes(); if (!typeList) { return true; // If it's null, all types are valid. } @@ -384,8 +384,7 @@ export class FieldVariable extends FieldDropdown { * @returns Array of variable types. * @throws {Error} if variableTypes is an empty array. */ - private getVariableTypes_(): string[] { - // TODO (#1513): Try to avoid calling this every time the field is edited. + private getVariableTypes(): string[] { let variableTypes = this.variableTypes; if (variableTypes === null) { // If variableTypes is null, return all variable types. @@ -413,7 +412,7 @@ export class FieldVariable extends FieldDropdown { * @param defaultType The type of the variable to create if this field's * value is not explicitly set. Defaults to ''. */ - private setTypes_(variableTypes: string[]|null = null, defaultType = '') { + private setTypes(variableTypes: string[]|null = null, defaultType = '') { // If you expected that the default type would be the same as the only entry // in the variable types array, tell the Blockly team by commenting on // #1499. @@ -438,7 +437,7 @@ export class FieldVariable extends FieldDropdown { 'a FieldVariable'); } // Only update the field once all checks pass. - this.defaultType_ = defaultType; + this.defaultType = defaultType; this.variableTypes = variableTypes; } @@ -468,11 +467,11 @@ export class FieldVariable extends FieldDropdown { if (id === internalConstants.RENAME_VARIABLE_ID) { // Rename variable. Variables.renameVariable( - this.sourceBlock_.workspace, this.variable_ as VariableModel); + this.sourceBlock_.workspace, this.variable as VariableModel); return; } else if (id === internalConstants.DELETE_VARIABLE_ID) { // Delete variable. - this.sourceBlock_.workspace.deleteVariableById(this.variable_!.getId()); + this.sourceBlock_.workspace.deleteVariableById(this.variable!.getId()); return; } } @@ -516,7 +515,7 @@ export class FieldVariable extends FieldDropdown { * @returns Array of variable names/id tuples. */ static dropdownCreate(this: FieldVariable): MenuOption[] { - if (!this.variable_) { + if (!this.variable) { throw Error( 'Tried to call dropdownCreate on a variable field with no' + ' variable selected.'); @@ -524,7 +523,7 @@ export class FieldVariable extends FieldDropdown { const name = this.getText(); let variableModelList: VariableModel[] = []; if (this.sourceBlock_ && !this.sourceBlock_.isDeadOrDying()) { - const variableTypes = this.getVariableTypes_(); + const variableTypes = this.getVariableTypes(); // Get a copy of the list, so that adding rename and new variable options // doesn't modify the workspace's list. for (let i = 0; i < variableTypes.length; i++) { diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js index 478fb1a77..a6458b5ba 100644 --- a/tests/mocha/field_variable_test.js +++ b/tests/mocha/field_variable_test.js @@ -251,7 +251,7 @@ suite('Variable Fields', function() { const field = new Blockly.FieldVariable( 'test', undefined, ['Type1'], 'Type1'); chai.assert.deepEqual(field.variableTypes, ['Type1']); - chai.assert.equal(field.defaultType_, 'Type1'); + chai.assert.equal(field.defaultType, 'Type1'); }); test('JSON Definition', function() { const field = Blockly.FieldVariable.fromJson({ @@ -260,7 +260,7 @@ suite('Variable Fields', function() { defaultType: 'Type1', }); chai.assert.deepEqual(field.variableTypes, ['Type1']); - chai.assert.equal(field.defaultType_, 'Type1'); + chai.assert.equal(field.defaultType, 'Type1'); }); test('JS Configuration - Simple', function() { const field = new Blockly.FieldVariable( @@ -269,7 +269,7 @@ suite('Variable Fields', function() { defaultType: 'Type1', }); chai.assert.deepEqual(field.variableTypes, ['Type1']); - chai.assert.equal(field.defaultType_, 'Type1'); + chai.assert.equal(field.defaultType, 'Type1'); }); test('JS Configuration - Ignore', function() { const field = new Blockly.FieldVariable( @@ -278,7 +278,7 @@ suite('Variable Fields', function() { defaultType: 'Type1', }); chai.assert.deepEqual(field.variableTypes, ['Type1']); - chai.assert.equal(field.defaultType_, 'Type1'); + chai.assert.equal(field.defaultType, 'Type1'); }); }); }); @@ -291,7 +291,7 @@ suite('Variable Fields', function() { // Expect that since variableTypes is undefined, only type empty string // will be returned (regardless of what types are available on the workspace). const fieldVariable = new Blockly.FieldVariable('name1'); - const resultTypes = fieldVariable.getVariableTypes_(); + const resultTypes = fieldVariable.getVariableTypes(); chai.assert.deepEqual(resultTypes, ['']); }); test('variableTypes is explicit', function() { @@ -299,9 +299,9 @@ suite('Variable Fields', function() { // value, regardless of what types are available on the workspace. const fieldVariable = new Blockly.FieldVariable( 'name1', null, ['type1', 'type2'], 'type1'); - const resultTypes = fieldVariable.getVariableTypes_(); + const resultTypes = fieldVariable.getVariableTypes(); chai.assert.deepEqual(resultTypes, ['type1', 'type2']); - chai.assert.equal(fieldVariable.defaultType_, 'type1', + chai.assert.equal(fieldVariable.defaultType, 'type1', 'Default type was wrong'); }); test('variableTypes is null', function() { @@ -314,7 +314,7 @@ suite('Variable Fields', function() { fieldVariable.setSourceBlock(mockBlock); fieldVariable.variableTypes = null; - const resultTypes = fieldVariable.getVariableTypes_(); + const resultTypes = fieldVariable.getVariableTypes(); // The empty string is always one of the options. chai.assert.deepEqual(resultTypes, ['type1', 'type2', '']); }); @@ -326,19 +326,19 @@ suite('Variable Fields', function() { fieldVariable.variableTypes = []; chai.assert.throws(function() { - fieldVariable.getVariableTypes_(); + fieldVariable.getVariableTypes(); }); }); }); suite('Default types', function() { test('Default type exists', function() { const fieldVariable = new Blockly.FieldVariable(null, null, ['b'], 'b'); - chai.assert.equal(fieldVariable.defaultType_, 'b', + chai.assert.equal(fieldVariable.defaultType, 'b', 'The variable field\'s default type should be "b"'); }); test('No default type', function() { const fieldVariable = new Blockly.FieldVariable(null); - chai.assert.equal(fieldVariable.defaultType_, '', 'The variable field\'s default type should be the empty string'); + chai.assert.equal(fieldVariable.defaultType, '', 'The variable field\'s default type should be the empty string'); chai.assert.isNull(fieldVariable.variableTypes, 'The variable field\'s allowed types should be null'); });