CodingPirates/blockly
1
0
Fork 0
mirror of https://github.com/google/blockly.git synced 2026-01-07 00:50:27 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix: check potential variables for flyout variable fields (#8873)

Browse Source 
* fix: check potential variables for flyout variable fields

* fix: format

* chore: move comment
This commit is contained in:
Maribeth Moffatt
2025-04-07 17:29:00 -07:00
committed by GitHub
parent 76b02de654
commit 89194b2ead
3 changed files with 72 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
core/field_variable.ts
View File

@@ -71,7 +71,8 @@ export class FieldVariable extends FieldDropdown {
* field's value. Takes in a variable ID & returns a validated variable
* ID, or null to abort the change.
* @param variableTypes A list of the types of variables to include in the
* dropdown. Will only be used if config is not provided.
* dropdown. Pass `null` to include all types that exist on the
* workspace. Will only be used if config is not provided.
* @param defaultType The type of variable to create if this field's value
* is not explicitly set. Defaults to ''. Will only be used if config
* is not provided.
@@ -83,7 +84,7 @@ export class FieldVariable extends FieldDropdown {
constructor(
varName: string | null | typeof Field.SKIP_SETUP,
validator?: FieldVariableValidator,
variableTypes?: string[],
variableTypes?: string[] | null,
defaultType?: string,
config?: FieldVariableConfig,
) {
@@ -423,25 +424,27 @@ export class FieldVariable extends FieldDropdown {
* Return a list of variable types to include in the dropdown.
*
* @returns Array of variable types.
* @throws {Error} if variableTypes is an empty array.
*/
private getVariableTypes(): string[] {
let variableTypes = this.variableTypes;
if (variableTypes === null) {
// If variableTypes is null, return all variable types.
if (this.sourceBlock_ && !this.sourceBlock_.isDeadOrDying()) {
return this.sourceBlock_.workspace.getVariableMap().getTypes();
}
if (this.variableTypes) return this.variableTypes;
if (!this.sourceBlock_ || this.sourceBlock_.isDeadOrDying()) {
// We should include all types in the block's workspace,
// but the block is dead so just give up.
return [''];
}
variableTypes = variableTypes || [''];
if (variableTypes.length === 0) {
// Throw an error if variableTypes is an empty list.
const name = this.getText();
throw Error(
"'variableTypes' of field variable " + name + ' was an empty list',
);
// If variableTypes is null, return all variable types in the workspace.
let allTypes = this.sourceBlock_.workspace.getVariableMap().getTypes();
if (this.sourceBlock_.isInFlyout) {
// If this block is in a flyout, we also need to check the potential variables
const potentialMap =
this.sourceBlock_.workspace.getPotentialVariableMap();
if (!potentialMap) return allTypes;
allTypes = Array.from(new Set([...allTypes, ...potentialMap.getTypes()]));
}
return variableTypes;
return allTypes;
}
/**
@@ -455,11 +458,15 @@ export class FieldVariable extends FieldDropdown {
* value is not explicitly set. Defaults to ''.
*/
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.
// Set the allowable variable types. Null means all types on the workspace.
const name = this.getText();
if (Array.isArray(variableTypes)) {
if (variableTypes.length === 0) {
// Throw an error if variableTypes is an empty list.
throw Error(
`'variableTypes' of field variable ${name} was an empty list. If you want to include all variable types, pass 'null' instead.`,
);
}
// Make sure the default type is valid.
let isInArray = false;
for (let i = 0; i < variableTypes.length; i++) {
@@ -477,8 +484,7 @@ export class FieldVariable extends FieldDropdown {
}
} else if (variableTypes !== null) {
throw Error(
"'variableTypes' was not an array in the definition of " +
'a FieldVariable',
`'variableTypes' was not an array or null in the definition of FieldVariable ${name}`,
);
}
// Only update the field once all checks pass.

1
core/workspace.ts
View File

@@ -854,7 +854,6 @@ export class Workspace implements IASTNodeLocation {
* These exist in the flyout but not in the workspace.
*
* @returns The potential variable map.
* @internal
*/
getPotentialVariableMap(): IVariableMap<
IVariableModel<IVariableState>

57
tests/mocha/field_variable_test.js
View File

@@ -309,6 +309,24 @@ suite('Variable Fields', function () {
assert.deepEqual(field.variableTypes, ['Type1']);
assert.equal(field.defaultType, 'Type1');
});
test('Empty list of types', function () {
assert.throws(function () {
const fieldVariable = new Blockly.FieldVariable(
'name1',
undefined,
[],
);
});
});
test('invalid value for list of types', function () {
assert.throws(function () {
const fieldVariable = new Blockly.FieldVariable(
'name1',
undefined,
'not an array',
);
});
});
test('JSON Definition', function () {
const field = Blockly.FieldVariable.fromJson({
variable: 'test',
@@ -353,13 +371,6 @@ suite('Variable Fields', function () {
this.workspace.createVariable('name1', 'type1');
this.workspace.createVariable('name2', 'type2');
});
test('variableTypes is undefined', 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();
assert.deepEqual(resultTypes, ['']);
});
test('variableTypes is explicit', function () {
// Expect that since variableTypes is defined, it will be the return
// value, regardless of what types are available on the workspace.
@@ -377,6 +388,17 @@ suite('Variable Fields', function () {
'Default type was wrong',
);
});
test('variableTypes is undefined', function () {
// Expect all variable types in the workspace to be returned, same
// as if variableTypes is null.
const fieldVariable = new Blockly.FieldVariable('name1');
const mockBlock = createTestBlock();
mockBlock.workspace = this.workspace;
fieldVariable.setSourceBlock(mockBlock);
const resultTypes = fieldVariable.getVariableTypes();
assert.deepEqual(resultTypes, ['type1', 'type2']);
});
test('variableTypes is null', function () {
// Expect all variable types to be returned.
// The field does not need to be initialized to do this--it just needs
@@ -390,16 +412,23 @@ suite('Variable Fields', function () {
const resultTypes = fieldVariable.getVariableTypes();
assert.deepEqual(resultTypes, ['type1', 'type2']);
});
test('variableTypes is the empty list', function () {
const fieldVariable = new Blockly.FieldVariable('name1');
test('variableTypes is null and variable is in the flyout', function () {
// Expect all variable types in the workspace and
// flyout workspace to be returned.
const fieldVariable = new Blockly.FieldVariable('name1', undefined, null);
const mockBlock = createTestBlock();
mockBlock.workspace = this.workspace;
fieldVariable.setSourceBlock(mockBlock);
fieldVariable.variableTypes = [];
assert.throws(function () {
fieldVariable.getVariableTypes();
});
// Pretend this is a flyout workspace with potential variables
mockBlock.isInFlyout = true;
mockBlock.workspace.createPotentialVariableMap();
mockBlock.workspace
.getPotentialVariableMap()
.createVariable('name3', 'type3');
fieldVariable.setSourceBlock(mockBlock);
const resultTypes = fieldVariable.getVariableTypes();
assert.deepEqual(resultTypes, ['type1', 'type2', 'type3']);
});
});
suite('Default types', function () {