From b536a58229aa22bd7a66771c16f0995086b80683 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Fri, 22 Mar 2024 22:02:07 -0700 Subject: [PATCH 1/9] feat: Invalid Blocks --- blocks/loops.ts | 14 ++-- blocks/procedures.ts | 29 ++++---- core/block.ts | 73 +++++++++++++++++++-- core/block_svg.ts | 51 ++++++++++---- core/contextmenu_items.ts | 2 +- core/events/events_block_change.ts | 7 +- core/events/utils.ts | 11 ++-- core/generator.ts | 2 +- core/gesture.ts | 2 +- core/icons/mutator_icon.ts | 3 +- core/rendered_connection.ts | 6 +- core/renderers/common/path_object.ts | 8 ++- core/renderers/geras/path_object.ts | 4 +- core/serialization/blocks.ts | 9 +++ core/xml.ts | 17 +++++ demos/blockfactory/blocks.js | 4 +- demos/blockfactory/factory_utils.js | 8 +-- msg/json/en.json | 3 +- msg/json/qqq.json | 1 + msg/messages.js | 6 ++ tests/browser/test/basic_playground_test.js | 6 +- tests/mocha/block_test.js | 18 ++--- tests/mocha/blocks/loops_test.js | 55 ++++++++++++++++ tests/mocha/blocks/procedures_test.js | 59 +++++++++++++++-- tests/mocha/event_test.js | 30 ++++----- tests/mocha/generator_test.js | 52 +++++++++++++-- tests/mocha/index.html | 1 + 27 files changed, 382 insertions(+), 99 deletions(-) create mode 100644 tests/mocha/blocks/loops_test.js diff --git a/blocks/loops.ts b/blocks/loops.ts index 02d9d34be72..395446a431c 100644 --- a/blocks/loops.ts +++ b/blocks/loops.ts @@ -365,18 +365,20 @@ const CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { // Don't change state if: // * It's at the start of a drag. // * It's not a move event. - if (!ws.isDragging || ws.isDragging() || e.type !== Events.BLOCK_MOVE) { + if ( + !ws.isDragging || + ws.isDragging() || + (e.type !== Events.BLOCK_MOVE && e.type !== Events.BLOCK_CREATE) + ) { return; } - const enabled = !!this.getSurroundLoop(); - this.setWarningText( - enabled ? null : Msg['CONTROLS_FLOW_STATEMENTS_WARNING'], - ); + const valid = !!this.getSurroundLoop(); + this.setWarningText(valid ? null : Msg['CONTROLS_FLOW_STATEMENTS_WARNING']); if (!this.isInFlyout) { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setEnabled(enabled); + this.setInvalidReason(!valid, 'unparented control flow'); Events.setGroup(group); } }, diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 1457971d898..e3e334db33e 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -753,7 +753,6 @@ interface CallMixin extends CallMixinType { defType_: string; quarkIds_: string[] | null; quarkConnections_: {[id: string]: Connection}; - previousEnabledState_: boolean; } type CallMixinType = typeof PROCEDURE_CALL_COMMON; @@ -1105,7 +1104,8 @@ const PROCEDURE_CALL_COMMON = { } } else if ( event.type === Events.BLOCK_CHANGE && - (event as BlockChange).element === 'disabled' + ((event as BlockChange).element === 'disabled' || + (event as BlockChange).element === 'invalid') ) { const blockChangeEvent = event as BlockChange; const name = this.getProcedureCall(); @@ -1123,12 +1123,13 @@ const PROCEDURE_CALL_COMMON = { ); } Events.setGroup(event.group); - if (blockChangeEvent.newValue) { - this.previousEnabledState_ = this.isEnabled(); - this.setEnabled(false); - } else { - this.setEnabled(this.previousEnabledState_); - } + const invalid = !def.isEnabled() || !def.isValid(); + this.setInvalidReason(invalid, 'disabled procedure definition'); + this.setWarningText( + invalid + ? Msg['PROCEDURES_CALL_DISABLED_DEF_WARNING'].replace('%1', name) + : null, + ); Events.setGroup(oldGroup); } } @@ -1180,7 +1181,6 @@ blocks['procedures_callnoreturn'] = { this.argumentVarModels_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; - this.previousEnabledState_ = true; }, defType_: 'procedures_defnoreturn', @@ -1201,7 +1201,6 @@ blocks['procedures_callreturn'] = { this.argumentVarModels_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; - this.previousEnabledState_ = true; }, defType_: 'procedures_defreturn', @@ -1278,21 +1277,21 @@ const PROCEDURES_IFRETURN = { if ( ((this.workspace as WorkspaceSvg).isDragging && (this.workspace as WorkspaceSvg).isDragging()) || - e.type !== Events.BLOCK_MOVE + (e.type !== Events.BLOCK_MOVE && e.type !== Events.BLOCK_CREATE) ) { return; // Don't change state at the start of a drag. } - let legal = false; + let valid = false; // Is the block nested in a procedure? let block = this; // eslint-disable-line @typescript-eslint/no-this-alias do { if (this.FUNCTION_TYPES.includes(block.type)) { - legal = true; + valid = true; break; } block = block.getSurroundParent()!; } while (block); - if (legal) { + if (valid) { // If needed, toggle whether this block has a return value. if (block.type === 'procedures_defnoreturn' && this.hasReturnValue_) { this.removeInput('VALUE'); @@ -1318,7 +1317,7 @@ const PROCEDURES_IFRETURN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setEnabled(legal); + this.setInvalidReason(!valid, 'unparented ifreturn'); Events.setGroup(group); } }, diff --git a/core/block.ts b/core/block.ts index a321ad1417b..d9699fdf261 100644 --- a/core/block.ts +++ b/core/block.ts @@ -168,6 +168,7 @@ export class Block implements IASTNodeLocation, IDeletable { inputsInline?: boolean; icons: IIcon[] = []; private disabled = false; + private invalidReasons = new Set(); tooltip: Tooltip.TipInfo = ''; contextMenu = true; @@ -1391,7 +1392,7 @@ export class Block implements IASTNodeLocation, IDeletable { } /** - * Get whether this block is enabled or not. + * Get whether this block is enabled or not. This should reflect whether the user intentionally disabled a block. * * @returns True if enabled. */ @@ -1400,7 +1401,7 @@ export class Block implements IASTNodeLocation, IDeletable { } /** - * Set whether the block is enabled or not. + * Set whether the block is enabled or not. The user can toggle whether a block is disabled from a context menu option. * * @param enabled True if enabled. */ @@ -1426,10 +1427,10 @@ export class Block implements IASTNodeLocation, IDeletable { * * @returns True if disabled. */ - getInheritedDisabled(): boolean { + getInheritedDisabledOrInvalid(): boolean { let ancestor = this.getSurroundParent(); while (ancestor) { - if (ancestor.disabled) { + if (ancestor.disabled || !ancestor.isValid()) { return true; } ancestor = ancestor.getSurroundParent(); @@ -1438,6 +1439,70 @@ export class Block implements IASTNodeLocation, IDeletable { return false; } + /** + * Get whether this block is valid or not. A block is considered valid if + * there aren't any reasons why it would be invalid. + * + * @returns True if valid. + */ + isValid(): boolean { + return this.invalidReasons.size === 0; + } + + /** + * Add or remove a reason why the block might be invalid. If a block has + * any reasons to be invalid, then the block itself will be considered + * invalid. A block could be invalid for multiple independent reasons + * simultaneously. Automatic processes in the Blockly library and in plugins + * can call this to cooperatively determine whether the block is invalid. + * + * @param invalid If true, then the block should be considered invalid for + * at least the provided reason, otherwise the block is no longer invalid + * for that reason. + * @param reason A language-neutral identifier for a reason why the block + * could be invalid. Call this method again with the same identifier to + * update whether the block is currently invalid for this reason. + */ + setInvalidReason(invalid: boolean, reason: string): void { + if (this.invalidReasons.has(reason) !== invalid) { + if (invalid) { + this.invalidReasons.add(reason); + } else { + this.invalidReasons.delete(reason); + } + eventUtils.fire( + new (eventUtils.get(eventUtils.BLOCK_CHANGE))( + this, + 'invalid', + null, + /* oldValue= */ {[reason]: !invalid}, + /* newValue= */ {[reason]: invalid}, + ), + ); + } + } + + /** + * Get whether the block is currently invalid for the provided reason. + * + * @param reason A language-neutral identifier for a reason why the block + * could be invalid. + * @returns Whether the block is invalid for the provided reason. + */ + hasInvalidReason(reason: string): boolean { + return this.invalidReasons.has(reason); + } + + /** + * Get a set of reasons why the block is currently invalid, if any. If the + * block is valid, this set will be empty. + * + * @returns The set of reasons why the block is invalid, if any. + */ + getInvalidReasons(): ReadonlySet { + return this.invalidReasons; + } + /** * Get whether the block is collapsed or not. * diff --git a/core/block_svg.ts b/core/block_svg.ts index f276d1e8ea0..482a331eea1 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -121,7 +121,7 @@ export class BlockSvg /** Is this block a BlockSVG? */ override readonly rendered = true; - private visuallyDisabled = false; + private visuallyDisabledOrInvalid = false; /** * Is this block currently rendering? Used to stop recursive render calls @@ -530,7 +530,7 @@ export class BlockSvg } if (!collapsed) { - this.updateDisabled(); + this.updateDisabledOrInvalid(); this.removeInput(collapsedInputName); return; } @@ -877,18 +877,21 @@ export class BlockSvg * * @internal */ - updateDisabled() { - const disabled = !this.isEnabled() || this.getInheritedDisabled(); + updateDisabledOrInvalid() { + const disabledOrInvalid = + !this.isEnabled() || + !this.isValid() || + this.getInheritedDisabledOrInvalid(); - if (this.visuallyDisabled === disabled) { - this.getNextBlock()?.updateDisabled(); + if (this.visuallyDisabledOrInvalid === disabledOrInvalid) { + this.getNextBlock()?.updateDisabledOrInvalid(); return; } this.applyColour(); - this.visuallyDisabled = disabled; + this.visuallyDisabledOrInvalid = disabledOrInvalid; for (const child of this.getChildren(false)) { - child.updateDisabled(); + child.updateDisabledOrInvalid(); } } @@ -1021,8 +1024,32 @@ export class BlockSvg override setEnabled(enabled: boolean) { if (this.isEnabled() !== enabled) { super.setEnabled(enabled); - if (!this.getInheritedDisabled()) { - this.updateDisabled(); + if (!this.getInheritedDisabledOrInvalid()) { + this.updateDisabledOrInvalid(); + } + } + } + + /** + * Add or remove a reason why the block might be invalid. If a block has + * any reasons to be invalid, then the block itself will be considered + * invalid. A block could be invalid for multiple independent reasons + * simultaneously. Automatic processes in the Blockly library and in plugins + * can call this to cooperatively determine whether the block is invalid. + * + * @param invalid If true, then the block should be considered invalid for + * at least the provided reason, otherwise the block is no longer invalid + * for that reason. + * @param reason A language-neutral identifier for a reason why the block + * could be invalid. Call this method again with the same identifier to + * update whether the block is currently invalid for this reason. + */ + override setInvalidReason(invalid: boolean, reason: string): void { + const wasValid = this.isValid(); + super.setInvalidReason(invalid, reason); + if (this.isValid() !== wasValid) { + if (!this.getInheritedDisabledOrInvalid()) { + this.updateDisabledOrInvalid(); } } } @@ -1505,8 +1532,8 @@ export class BlockSvg this.updateCollapsed_(); } - if (!this.isEnabled()) { - this.updateDisabled(); + if (!this.isEnabled() || !this.isValid()) { + this.updateDisabledOrInvalid(); } this.workspace.getRenderer().render(this); diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index a540a1341ce..fd9d62d1fd6 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -466,7 +466,7 @@ export function registerDisable() { block!.workspace.options.disable && block!.isEditable() ) { - if (block!.getInheritedDisabled()) { + if (block!.getInheritedDisabledOrInvalid() || !block!.isValid()) { return 'disabled'; } return 'enabled'; diff --git a/core/events/events_block_change.ts b/core/events/events_block_change.ts index 3ef5c10850f..cf8198dc842 100644 --- a/core/events/events_block_change.ts +++ b/core/events/events_block_change.ts @@ -31,7 +31,7 @@ export class BlockChange extends BlockBase { override type = eventUtils.BLOCK_CHANGE; /** * The element that changed; one of 'field', 'comment', 'collapsed', - * 'disabled', 'inline', or 'mutation' + * 'disabled', 'invalid', 'inline', or 'mutation' */ element?: string; @@ -170,6 +170,11 @@ export class BlockChange extends BlockBase { case 'disabled': block.setEnabled(!value); break; + case 'invalid': + for (const entry of Object.entries(value as Object)) { + block.setInvalidReason(!!entry[1], entry[0]); + } + break; case 'inline': block.setInputsInline(!!value); break; diff --git a/core/events/utils.ts b/core/events/utils.ts index 469f105824c..15f217df219 100644 --- a/core/events/utils.ts +++ b/core/events/utils.ts @@ -513,14 +513,13 @@ export function get( } /** - * Enable/disable a block depending on whether it is properly connected. + * Set if a block is invalid depending on whether it is properly connected. * Use this on applications where all blocks should be connected to a top block. - * Recommend setting the 'disable' option to 'false' in the config so that - * users don't try to re-enable disabled orphan blocks. * * @param event Custom data for event. */ export function disableOrphans(event: Abstract) { + const invalidReason = 'orphaned block'; if (event.type === MOVE || event.type === CREATE) { const blockEvent = event as BlockMove | BlockCreate; if (!blockEvent.workspaceId) { @@ -539,17 +538,17 @@ export function disableOrphans(event: Abstract) { try { recordUndo = false; const parent = block.getParent(); - if (parent && parent.isEnabled()) { + if (parent && !parent.hasInvalidReason(invalidReason)) { const children = block.getDescendants(false); for (let i = 0, child; (child = children[i]); i++) { - child.setEnabled(true); + child.setInvalidReason(false, invalidReason); } } else if ( (block.outputConnection || block.previousConnection) && !eventWorkspace.isDragging() ) { do { - block.setEnabled(false); + block.setInvalidReason(true, invalidReason); block = block.getNextBlock(); } while (block); } diff --git a/core/generator.ts b/core/generator.ts index a24a0469725..5481ee9fee5 100644 --- a/core/generator.ts +++ b/core/generator.ts @@ -243,7 +243,7 @@ export class CodeGenerator { if (!block) { return ''; } - if (!block.isEnabled()) { + if (!block.isEnabled() || !block.isValid()) { // Skip past this block if it is disabled. return opt_thisOnly ? '' : this.blockToCode(block.getNextBlock()); } diff --git a/core/gesture.ts b/core/gesture.ts index 2aed48d79e4..a8890be64e4 100644 --- a/core/gesture.ts +++ b/core/gesture.ts @@ -964,7 +964,7 @@ export class Gesture { 'Cannot do a block click because the target block is ' + 'undefined', ); } - if (this.targetBlock.isEnabled()) { + if (this.targetBlock.isEnabled() && this.targetBlock.isValid()) { if (!eventUtils.getGroup()) { eventUtils.setGroup(true); } diff --git a/core/icons/mutator_icon.ts b/core/icons/mutator_icon.ts index 7fb3fcf3b81..9ccd97fc07f 100644 --- a/core/icons/mutator_icon.ts +++ b/core/icons/mutator_icon.ts @@ -309,7 +309,8 @@ export class MutatorIcon extends Icon implements IHasBubble { e.isUiEvent || e.type === eventUtils.CREATE || (e.type === eventUtils.CHANGE && - (e as BlockChange).element === 'disabled') + (e as BlockChange).element === 'disabled') || + (e as BlockChange).element === 'invalid' ); } diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 5aa6f7f7588..fd9d2d47001 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -453,7 +453,7 @@ export class RenderedConnection extends Connection { super.disconnectInternal(setParent); parent.queueRender(); - child.updateDisabled(); + child.updateDisabledOrInvalid(); child.queueRender(); // Reset visibility, since the child is now a top block. child.getSvgRoot().style.display = 'block'; @@ -502,8 +502,8 @@ export class RenderedConnection extends Connection { const parentBlock = this.getSourceBlock(); const childBlock = renderedChildConnection.getSourceBlock(); - parentBlock.updateDisabled(); - childBlock.updateDisabled(); + parentBlock.updateDisabledOrInvalid(); + childBlock.updateDisabledOrInvalid(); childBlock.queueRender(); // The input the child block is connected to (if any). diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index d5c0850a1b8..0aec0603440 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -132,7 +132,11 @@ export class PathObject implements IPathObject { this.svgPath.setAttribute('fill', this.style.colourPrimary); this.updateShadow_(block.isShadow()); - this.updateDisabled_(!block.isEnabled() || block.getInheritedDisabled()); + this.updateDisabledOrInvalid_( + !block.isEnabled() || + !block.isValid() || + block.getInheritedDisabledOrInvalid(), + ); } /** @@ -196,7 +200,7 @@ export class PathObject implements IPathObject { * * @param disabled True if disabled. */ - protected updateDisabled_(disabled: boolean) { + protected updateDisabledOrInvalid_(disabled: boolean) { this.setClass_('blocklyDisabled', disabled); if (disabled) { this.svgPath.setAttribute( diff --git a/core/renderers/geras/path_object.ts b/core/renderers/geras/path_object.ts index 6b058e5a752..b6f7db711a8 100644 --- a/core/renderers/geras/path_object.ts +++ b/core/renderers/geras/path_object.ts @@ -130,8 +130,8 @@ export class PathObject extends BasePathObject { } } - override updateDisabled_(disabled: boolean) { - super.updateDisabled_(disabled); + override updateDisabledOrInvalid_(disabled: boolean) { + super.updateDisabledOrInvalid_(disabled); if (disabled) { this.svgPath.setAttribute('stroke', 'none'); } diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index 229686a0de6..b3040ffa4eb 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -53,6 +53,7 @@ export interface State { movable?: boolean; editable?: boolean; enabled?: boolean; + invalid?: string[]; inline?: boolean; data?: string; extraState?: AnyDuringMigration; @@ -160,6 +161,9 @@ function saveAttributes(block: Block, state: State) { if (!block.isEnabled()) { state['enabled'] = false; } + if (!block.isValid()) { + state['invalid'] = Array.from(block.getInvalidReasons()); + } if (!block.isOwnDeletable()) { state['deletable'] = false; } @@ -522,6 +526,11 @@ function loadAttributes(block: Block, state: State) { if (state['enabled'] === false) { block.setEnabled(false); } + if (Array.isArray(state['invalid'])) { + for (const reason of state['invalid']) { + block.setInvalidReason(true, reason); + } + } if (state['inline'] !== undefined) { block.setInputsInline(state['inline']); } diff --git a/core/xml.ts b/core/xml.ts index 0f2c4f143c7..805db837765 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -250,6 +250,15 @@ export function blockToDom( if (!block.isEnabled()) { element.setAttribute('disabled', 'true'); } + if (!block.isValid()) { + // Set the value of the attribute to a comma-separated list of reasons. + // Use encodeURIComponent to escape commas in the reasons so that they + // won't be confused with separator commas. + element.setAttribute( + 'invalid-reasons', + Array.from(block.getInvalidReasons()).map(encodeURIComponent).join(','), + ); + } if (!block.isOwnDeletable()) { element.setAttribute('deletable', 'false'); } @@ -970,6 +979,14 @@ function domToBlockHeadless( if (disabled) { block.setEnabled(disabled !== 'true' && disabled !== 'disabled'); } + const invalidReasons = xmlBlock.getAttribute('invalid-reasons'); + if (invalidReasons !== null) { + for (const reason of invalidReasons.split(',')) { + // Use decodeURIComponent to restore characters that were encoded in the + // value, such as commas. + block.setInvalidReason(true, decodeURIComponent(reason)); + } + } const deletable = xmlBlock.getAttribute('deletable'); if (deletable) { block.setDeletable(deletable === 'true'); diff --git a/demos/blockfactory/blocks.js b/demos/blockfactory/blocks.js index b93ae573945..aa0cb5647e2 100644 --- a/demos/blockfactory/blocks.js +++ b/demos/blockfactory/blocks.js @@ -880,7 +880,7 @@ function fieldNameCheck(referenceBlock) { var blocks = referenceBlock.workspace.getAllBlocks(false); for (var i = 0, block; block = blocks[i]; i++) { var otherName = block.getFieldValue('FIELDNAME'); - if (!block.disabled && !block.getInheritedDisabled() && + if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid() && otherName && otherName.toLowerCase() === name) { count++; } @@ -905,7 +905,7 @@ function inputNameCheck(referenceBlock) { var blocks = referenceBlock.workspace.getAllBlocks(false); for (var i = 0, block; block = blocks[i]; i++) { var otherName = block.getFieldValue('INPUTNAME'); - if (!block.disabled && !block.getInheritedDisabled() && + if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid() && otherName && otherName.toLowerCase() === name) { count++; } diff --git a/demos/blockfactory/factory_utils.js b/demos/blockfactory/factory_utils.js index 148498360e4..a99259039de 100644 --- a/demos/blockfactory/factory_utils.js +++ b/demos/blockfactory/factory_utils.js @@ -163,7 +163,7 @@ FactoryUtils.formatJson_ = function(blockType, rootBlock) { var contentsBlock = rootBlock.getInputTargetBlock('INPUTS'); var lastInput = null; while (contentsBlock) { - if (!contentsBlock.disabled && !contentsBlock.getInheritedDisabled()) { + if (!contentsBlock.disabled && contentsBlock.isValid() && !contentsBlock.getInheritedDisabledOrInvalid()) { var fields = FactoryUtils.getFieldsJson_( contentsBlock.getInputTargetBlock('FIELDS')); for (var i = 0; i < fields.length; i++) { @@ -277,7 +277,7 @@ FactoryUtils.formatJavaScript_ = function(blockType, rootBlock, workspace) { 'input_end_row': 'appendEndRowInput'}; var contentsBlock = rootBlock.getInputTargetBlock('INPUTS'); while (contentsBlock) { - if (!contentsBlock.disabled && !contentsBlock.getInheritedDisabled()) { + if (!contentsBlock.disabled && contentsBlock.isValid() && !contentsBlock.getInheritedDisabledOrInvalid()) { var name = ''; // Dummy inputs don't have names. Other inputs do. if (contentsBlock.type !== 'input_dummy' && @@ -377,7 +377,7 @@ FactoryUtils.connectionLineJs_ = function(functionName, typeName, workspace) { FactoryUtils.getFieldsJs_ = function(block) { var fields = []; while (block) { - if (!block.disabled && !block.getInheritedDisabled()) { + if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid()) { switch (block.type) { case 'field_static': // Result: 'hello' @@ -484,7 +484,7 @@ FactoryUtils.getFieldsJs_ = function(block) { FactoryUtils.getFieldsJson_ = function(block) { var fields = []; while (block) { - if (!block.disabled && !block.getInheritedDisabled()) { + if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid()) { switch (block.type) { case 'field_static': // Result: 'hello' diff --git a/msg/json/en.json b/msg/json/en.json index d9bddec31e9..c143da252ff 100644 --- a/msg/json/en.json +++ b/msg/json/en.json @@ -1,7 +1,7 @@ { "@metadata": { "author": "Ellen Spertus ", - "lastupdated": "2023-12-08 18:42:04.679586", + "lastupdated": "2024-03-08 22:38:32.330785", "locale": "en", "messagedocumentation" : "qqq" }, @@ -369,6 +369,7 @@ "PROCEDURES_DEFNORETURN_PROCEDURE": "do something", "PROCEDURES_BEFORE_PARAMS": "with:", "PROCEDURES_CALL_BEFORE_PARAMS": "with:", + "PROCEDURES_CALL_DISABLED_DEF_WARNING": "Can't run the user-defined function '%1' because the definition block is disabled.", "PROCEDURES_DEFNORETURN_DO": "", "PROCEDURES_DEFNORETURN_TOOLTIP": "Creates a function with no output.", "PROCEDURES_DEFNORETURN_COMMENT": "Describe this function...", diff --git a/msg/json/qqq.json b/msg/json/qqq.json index 60509d798e2..39ea18ab4b2 100644 --- a/msg/json/qqq.json +++ b/msg/json/qqq.json @@ -375,6 +375,7 @@ "PROCEDURES_DEFNORETURN_PROCEDURE": "default name - This acts as a placeholder for the name of a function on a function definition block, as shown on [https://blockly-demo.appspot.com/static/apps/code/index.html?lang=en#w7cfju this block]. The user will replace it with the function's name.", "PROCEDURES_BEFORE_PARAMS": "block text - This precedes the list of parameters on a function's definition block. See [https://blockly-demo.appspot.com/static/apps/code/index.html?lang=en#voztpd this sample function with parameters].", "PROCEDURES_CALL_BEFORE_PARAMS": "block text - This precedes the list of parameters on a function's caller block. See [https://blockly-demo.appspot.com/static/apps/code/index.html?lang=en#voztpd this sample function with parameters].", + "PROCEDURES_CALL_DISABLED_DEF_WARNING": "warning - This appears if a block that runs a function can't run because the function definition block is disabled. See [https://blockly-demo.appspot.com/static/demos/code/index.html#q947d7 this sample of a disabled function definition and call block].", "PROCEDURES_DEFNORETURN_DO": "{{Optional}}\nblock text - This appears next to the function's 'body', the blocks that should be run when the function is called, as shown in [https://blockly-demo.appspot.com/static/apps/code/index.html?lang=en#voztpd this sample function definition].", "PROCEDURES_DEFNORETURN_TOOLTIP": "tooltip", "PROCEDURES_DEFNORETURN_COMMENT": "Placeholder text that the user is encouraged to replace with a description of what their function does.", diff --git a/msg/messages.js b/msg/messages.js index a313b269a79..e159eb8d85a 100644 --- a/msg/messages.js +++ b/msg/messages.js @@ -1489,6 +1489,12 @@ Blockly.Msg.PROCEDURES_BEFORE_PARAMS = 'with:'; /// function with parameters]. Blockly.Msg.PROCEDURES_CALL_BEFORE_PARAMS = 'with:'; /** @type {string} */ +/// warning - This appears if a block that runs a function can't run because the function +/// definition block is disabled. See +/// [https://blockly-demo.appspot.com/static/demos/code/index.html#q947d7 this sample of a +/// disabled function definition and call block]. +Blockly.Msg.PROCEDURES_CALL_DISABLED_DEF_WARNING = 'Can\'t run the user-defined function "%1" because the definition block is disabled.'; +/** @type {string} */ /// {{Optional|Supply translation only if your language requires it. Most do not.}} /// block text - This appears next to the function's "body", the blocks that should be /// run when the function is called, as shown in diff --git a/tests/browser/test/basic_playground_test.js b/tests/browser/test/basic_playground_test.js index 091f2f6b5ce..d91b95f087e 100644 --- a/tests/browser/test/basic_playground_test.js +++ b/tests/browser/test/basic_playground_test.js @@ -28,7 +28,11 @@ async function getIsCollapsed(browser, blockId) { async function getIsDisabled(browser, blockId) { return await browser.execute((blockId) => { const block = Blockly.getMainWorkspace().getBlockById(blockId); - return !block.isEnabled() || block.getInheritedDisabled(); + return ( + !block.isEnabled() || + !block.isValid() || + block.getInheritedDisabledOrInvalid() + ); }, blockId); } diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 3184d409d49..b2cf132d85a 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -2359,8 +2359,8 @@ suite('Blocks', function () { .getTopBlocks(false)[0] .getChildren()[0]; chai.assert.isTrue( - innerBlock.visuallyDisabled, - 'block should have visuallyDisabled set because it is disabled', + innerBlock.visuallyDisabledOrInvalid, + 'block should have visuallyDisabledOrInvalid set because it is disabled', ); chai.assert.isFalse( innerBlock.isEnabled(), @@ -2384,8 +2384,8 @@ suite('Blocks', function () { .getTopBlocks(false)[0] .getChildren()[0]; chai.assert.isTrue( - innerBlock.visuallyDisabled, - 'block should have visuallyDisabled set because it is disabled', + innerBlock.visuallyDisabledOrInvalid, + 'block should have visuallyDisabledOrInvalid set because it is disabled', ); chai.assert.isFalse( innerBlock.isEnabled(), @@ -2438,7 +2438,7 @@ suite('Blocks', function () { await Blockly.renderManagement.finishQueuedRenders(); for (const child of this.parent.getDescendants(false)) { chai.assert.isTrue( - child.visuallyDisabled, + child.visuallyDisabledOrInvalid, `block ${child.id} should be visually disabled`, ); } @@ -2455,7 +2455,7 @@ suite('Blocks', function () { 'child1 should be enabled', ); chai.assert.isFalse( - this.child1.visuallyDisabled, + this.child1.visuallyDisabledOrInvalid, 'child1 should not be visually disabled', ); @@ -2464,7 +2464,7 @@ suite('Blocks', function () { 'child2 should be disabled', ); chai.assert.isTrue( - this.child2.visuallyDisabled, + this.child2.visuallyDisabledOrInvalid, 'child2 should be visually disabled', ); @@ -2473,7 +2473,7 @@ suite('Blocks', function () { 'child3 should be enabled', ); chai.assert.isFalse( - this.child3.visuallyDisabled, + this.child3.visuallyDisabledOrInvalid, 'child3 should not be visually disabled', ); @@ -2482,7 +2482,7 @@ suite('Blocks', function () { 'child34 should be enabled', ); chai.assert.isFalse( - this.child4.visuallyDisabled, + this.child4.visuallyDisabledOrInvalid, 'child4 should not be visually disabled', ); }); diff --git a/tests/mocha/blocks/loops_test.js b/tests/mocha/blocks/loops_test.js new file mode 100644 index 00000000000..8740b5ef789 --- /dev/null +++ b/tests/mocha/blocks/loops_test.js @@ -0,0 +1,55 @@ +/** + * @license + * Copyright 2024 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as Blockly from '../../../build/src/core/blockly.js'; +import { + sharedTestSetup, + sharedTestTeardown, +} from '../test_helpers/setup_teardown.js'; + +suite('Loops', function () { + setup(function () { + sharedTestSetup.call(this, {fireEventsNow: false}); + this.workspace = Blockly.inject('blocklyDiv', {}); + }); + + teardown(function () { + sharedTestTeardown.call(this); + }); + + suite('controls_flow_statements blocks', function () { + test('break block is invalid outside of loop block', function () { + const breakBlock = Blockly.serialization.blocks.append( + {'type': 'controls_flow_statements'}, + this.workspace, + ); + this.clock.runAll(); + chai.assert.isFalse( + breakBlock.isValid(), + 'Expected the break block to be invalid', + ); + }); + + test('break block is valid inside of loop block', function () { + const loopBlock = Blockly.serialization.blocks.append( + {'type': 'controls_repeat'}, + this.workspace, + ); + const breakBlock = Blockly.serialization.blocks.append( + {'type': 'controls_flow_statements'}, + this.workspace, + ); + loopBlock + .getInput('DO') + .connection.connect(breakBlock.previousConnection); + this.clock.runAll(); + chai.assert.isTrue( + breakBlock.isValid(), + 'Expected the break block to be valid', + ); + }); + }); +}); diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index f9d027c99f1..92257d31837 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -806,7 +806,7 @@ suite('Procedures', function () { suite('enabling and disabling procedure blocks', function () { test( 'if a procedure definition is disabled, the procedure caller ' + - 'is also disabled', + 'is invalid', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); @@ -815,15 +815,32 @@ suite('Procedures', function () { this.clock.runAll(); chai.assert.isFalse( - callBlock.isEnabled(), - 'Expected the caller block to be disabled', + callBlock.isValid(), + 'Expected the caller block to be invalid', + ); + }, + ); + + test( + 'if a procedure definition is invalid, the procedure caller ' + + 'is also invalid', + function () { + const defBlock = createProcDefBlock(this.workspace); + const callBlock = createProcCallBlock(this.workspace); + + defBlock.setInvalidReason(true, 'test reason'); + this.clock.runAll(); + + chai.assert.isFalse( + callBlock.isValid(), + 'Expected the caller block to be invalid', ); }, ); test( 'if a procedure definition is enabled, the procedure caller ' + - 'is also enabled', + 'is valid', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); @@ -834,8 +851,8 @@ suite('Procedures', function () { this.clock.runAll(); chai.assert.isTrue( - callBlock.isEnabled(), - 'Expected the caller block to be enabled', + callBlock.isValid(), + 'Expected the caller block to be valid', ); }, ); @@ -863,6 +880,36 @@ suite('Procedures', function () { ); }); + suite('procedures_ifreturn blocks', function () { + test('ifreturn block is invalid outside of def block', function () { + const ifreturnBlock = Blockly.serialization.blocks.append( + {'type': 'procedures_ifreturn'}, + this.workspace, + ); + this.clock.runAll(); + chai.assert.isFalse( + ifreturnBlock.isValid(), + 'Expected the ifreturn block to be invalid', + ); + }); + + test('ifreturn block is valid inside of def block', function () { + const defBlock = createProcDefBlock(this.workspace); + const ifreturnBlock = Blockly.serialization.blocks.append( + {'type': 'procedures_ifreturn'}, + this.workspace, + ); + defBlock + .getInput('STACK') + .connection.connect(ifreturnBlock.previousConnection); + this.clock.runAll(); + chai.assert.isTrue( + ifreturnBlock.isValid(), + 'Expected the ifreturn block to be valid', + ); + }); + }); + suite('deleting procedure blocks', function () { test( 'when the procedure definition block is deleted, all of its ' + diff --git a/tests/mocha/event_test.js b/tests/mocha/event_test.js index e59cb4911a1..360819fbd76 100644 --- a/tests/mocha/event_test.js +++ b/tests/mocha/event_test.js @@ -1402,7 +1402,7 @@ suite('Events', function () { sinon.assert.calledOnce(genUidStub); // When block is created using domToWorkspace, 5 events are fired: - // 1. varCreate (events disabled) + // 1. varCreate (events invalid) // 2. varCreate // 3. block create // 4. move (no-op, is filtered out) @@ -1457,7 +1457,7 @@ suite('Events', function () { teardown(function () { workspaceTeardown.call(this, this.workspace); }); - test('Created orphan block is disabled', function () { + test('Created orphan block is invalid', function () { this.workspace.addChangeListener(eventUtils.disableOrphans); const block = this.workspace.newBlock('controls_for'); block.initSvg(); @@ -1467,11 +1467,11 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isFalse( - block.isEnabled(), - 'Expected orphan block to be disabled after creation', + block.isValid(), + 'Expected orphan block to be invalid after creation', ); }); - test('Created procedure block is enabled', function () { + test('Created procedure block is valid', function () { this.workspace.addChangeListener(eventUtils.disableOrphans); // Procedure block is never an orphan @@ -1483,8 +1483,8 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isTrue( - functionBlock.isEnabled(), - 'Expected top-level procedure block to be enabled', + functionBlock.isValid(), + 'Expected top-level procedure block to be valid', ); }); test('Moving a block to top-level disables it', function () { @@ -1507,8 +1507,8 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isFalse( - block.isEnabled(), - 'Expected disconnected block to be disabled', + block.isValid(), + 'Expected disconnected block to be invalid', ); }); test('Giving block a parent enables it', function () { @@ -1528,8 +1528,8 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isTrue( - block.isEnabled(), - 'Expected block to be enabled after connecting to parent', + block.isValid(), + 'Expected block to be valid after connecting to parent', ); }); test('disableOrphans events are not undoable', function () { @@ -1551,12 +1551,12 @@ suite('Events', function () { // Fire all events this.clock.runAll(); - const disabledEvents = this.workspace.getUndoStack().filter(function (e) { - return e.element === 'disabled'; + const invalidEvents = this.workspace.getUndoStack().filter(function (e) { + return e.element === 'invalid'; }); chai.assert.isEmpty( - disabledEvents, - 'Undo stack should not contain any disabled events', + invalidEvents, + 'Undo stack should not contain any invalid events', ); }); }); diff --git a/tests/mocha/generator_test.js b/tests/mocha/generator_test.js index 9ac67b27ca3..815ad3ede65 100644 --- a/tests/mocha/generator_test.js +++ b/tests/mocha/generator_test.js @@ -81,6 +81,7 @@ suite('Generator', function () { this.blockToCodeTest = function ( generator, blockDisabled, + blockInvalid, opt_thisOnly, expectedCode, opt_message, @@ -93,6 +94,7 @@ suite('Generator', function () { }; rowBlock.nextConnection.connect(stackBlock.previousConnection); rowBlock.disabled = blockDisabled; + rowBlock.setInvalidReason(blockInvalid, 'test reason'); const code = generator.blockToCode(rowBlock, opt_thisOnly); delete generator.forBlock['stack_block']; @@ -115,11 +117,18 @@ suite('Generator', function () { const name = testCase[1]; test(name, function () { generator.init(this.workspace); - this.blockToCodeTest(generator, false, true, 'row_block'); this.blockToCodeTest( generator, - false, - false, + /* blockDisabled = */ false, + /* blockInvalid = */ false, + /* opt_thisOnly = */ true, + 'row_block', + ); + this.blockToCodeTest( + generator, + /* blockDisabled = */ false, + /* blockInvalid = */ false, + /* opt_thisOnly = */ false, 'row_blockstack_block', 'thisOnly=false', ); @@ -132,11 +141,42 @@ suite('Generator', function () { const generator = testCase[0]; const name = testCase[1]; test(name, function () { - this.blockToCodeTest(generator, true, true, ''); this.blockToCodeTest( generator, - true, - false, + /* blockDisabled = */ true, + /* blockInvalid = */ false, + /* opt_thisOnly = */ true, + '', + ); + this.blockToCodeTest( + generator, + /* blockDisabled = */ true, + /* blockInvalid = */ false, + /* opt_thisOnly = */ false, + 'stack_block', + 'thisOnly=false', + ); + }); + }); + }); + + suite('Invalid block', function () { + testCase.forEach(function (testCase) { + const generator = testCase[0]; + const name = testCase[1]; + test(name, function () { + this.blockToCodeTest( + generator, + /* blockDisabled = */ false, + /* blockInvalid = */ true, + /* opt_thisOnly = */ true, + '', + ); + this.blockToCodeTest( + generator, + /* blockDisabled = */ false, + /* blockInvalid = */ true, + /* opt_thisOnly = */ false, 'stack_block', 'thisOnly=false', ); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index d08d24ad8c3..4868ee06cc1 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -100,6 +100,7 @@ import './layering_test.js'; import './blocks/lists_test.js'; import './blocks/logic_ternary_test.js'; + import './blocks/loops_test.js'; import './metrics_test.js'; import './mutator_test.js'; import './names_test.js'; From 75d2fdb3cbd5080123b8aae562c0bf69e550c6d0 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Mon, 25 Mar 2024 11:25:53 -0700 Subject: [PATCH 2/9] Rename the new json property from invalid to invalidReasons. --- core/serialization/blocks.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index b3040ffa4eb..b8e59b44ad7 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -53,7 +53,7 @@ export interface State { movable?: boolean; editable?: boolean; enabled?: boolean; - invalid?: string[]; + invalidReasons?: string[]; inline?: boolean; data?: string; extraState?: AnyDuringMigration; @@ -162,7 +162,7 @@ function saveAttributes(block: Block, state: State) { state['enabled'] = false; } if (!block.isValid()) { - state['invalid'] = Array.from(block.getInvalidReasons()); + state['invalidReasons'] = Array.from(block.getInvalidReasons()); } if (!block.isOwnDeletable()) { state['deletable'] = false; @@ -526,8 +526,8 @@ function loadAttributes(block: Block, state: State) { if (state['enabled'] === false) { block.setEnabled(false); } - if (Array.isArray(state['invalid'])) { - for (const reason of state['invalid']) { + if (Array.isArray(state['invalidReasons'])) { + for (const reason of state['invalidReasons']) { block.setInvalidReason(true, reason); } } From bbf4ae101e7e985a78bbd1caf2aae5ef0796e686 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Tue, 2 Apr 2024 20:46:39 -0700 Subject: [PATCH 3/9] Merged isValid into isEnabled. --- blocks/loops.ts | 2 +- blocks/procedures.ts | 15 ++- core/block.ts | 124 +++++++++----------- core/block_svg.ts | 78 ++++++------ core/constants.ts | 6 + core/contextmenu_items.ts | 8 +- core/events/events_block_change.ts | 14 +-- core/events/utils.ts | 10 +- core/flyout_base.ts | 2 +- core/generator.ts | 2 +- core/gesture.ts | 2 +- core/icons/mutator_icon.ts | 3 +- core/rendered_connection.ts | 6 +- core/renderers/common/path_object.ts | 8 +- core/renderers/geras/path_object.ts | 4 +- core/serialization/blocks.ts | 23 ++-- core/trashcan.ts | 1 + core/xml.ts | 28 +++-- demos/blockfactory/blocks.js | 4 +- demos/blockfactory/factory_utils.js | 18 +-- tests/browser/test/basic_playground_test.js | 6 +- tests/mocha/block_test.js | 38 +++--- tests/mocha/blocks/loops_test.js | 8 +- tests/mocha/blocks/procedures_test.js | 24 ++-- tests/mocha/event_test.js | 30 ++--- tests/mocha/generator_test.js | 32 +---- tests/mocha/icon_test.js | 2 +- tests/mocha/jso_serialization_test.js | 8 +- tests/mocha/serializer_test.js | 2 +- 29 files changed, 244 insertions(+), 264 deletions(-) diff --git a/blocks/loops.ts b/blocks/loops.ts index 395446a431c..041252f340c 100644 --- a/blocks/loops.ts +++ b/blocks/loops.ts @@ -378,7 +378,7 @@ const CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setInvalidReason(!valid, 'unparented control flow'); + this.setDisabledReason(!valid, 'UNPARENTED_CONTROL_FLOW'); Events.setGroup(group); } }, diff --git a/blocks/procedures.ts b/blocks/procedures.ts index e3e334db33e..384bbe0dcad 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -1104,8 +1104,7 @@ const PROCEDURE_CALL_COMMON = { } } else if ( event.type === Events.BLOCK_CHANGE && - ((event as BlockChange).element === 'disabled' || - (event as BlockChange).element === 'invalid') + (event as BlockChange).element === 'disabled' ) { const blockChangeEvent = event as BlockChange; const name = this.getProcedureCall(); @@ -1123,12 +1122,12 @@ const PROCEDURE_CALL_COMMON = { ); } Events.setGroup(event.group); - const invalid = !def.isEnabled() || !def.isValid(); - this.setInvalidReason(invalid, 'disabled procedure definition'); + const valid = def.isEnabled(); + this.setDisabledReason(!valid, 'DISABLED_PROCEDURE_DEFINITION'); this.setWarningText( - invalid - ? Msg['PROCEDURES_CALL_DISABLED_DEF_WARNING'].replace('%1', name) - : null, + valid + ? null + : Msg['PROCEDURES_CALL_DISABLED_DEF_WARNING'].replace('%1', name), ); Events.setGroup(oldGroup); } @@ -1317,7 +1316,7 @@ const PROCEDURES_IFRETURN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setInvalidReason(!valid, 'unparented ifreturn'); + this.setDisabledReason(!valid, 'UNPARENTED_IFRETURN'); Events.setGroup(group); } }, diff --git a/core/block.ts b/core/block.ts index f32346660b2..8bc5e46110c 100644 --- a/core/block.ts +++ b/core/block.ts @@ -26,6 +26,7 @@ import * as constants from './constants.js'; import {DuplicateIconType} from './icons/exceptions.js'; import type {Abstract} from './events/events_abstract.js'; import type {BlockMove} from './events/events_block_move.js'; +import * as deprecation from './utils/deprecation.js'; import * as eventUtils from './events/utils.js'; import * as Extensions from './extensions.js'; import type {Field} from './field.js'; @@ -166,8 +167,7 @@ export class Block implements IASTNodeLocation { inputList: Input[] = []; inputsInline?: boolean; icons: IIcon[] = []; - private disabled = false; - private invalidReasons = new Set(); + private disabledReasons = new Set(); tooltip: Tooltip.TipInfo = ''; contextMenu = true; @@ -1391,30 +1391,65 @@ export class Block implements IASTNodeLocation { } /** - * Get whether this block is enabled or not. This should reflect whether the user intentionally disabled a block. + * Get whether this block is enabled or not. A block is considered enabled + * if there aren't any reasons why it would be disabled. A block may still + * be disabled for other reasons even if the user attempts to manually + * enable it, such as when the block is in an invalid location. * * @returns True if enabled. */ isEnabled(): boolean { - return !this.disabled; + return this.disabledReasons.size === 0; } /** - * Set whether the block is enabled or not. The user can toggle whether a block is disabled from a context menu option. + * @deprecated v11 - Set whether the block is manually enabled or disabled. + * The user can toggle whether a block is disabled from a context menu + * option. A block may still be disabled for other reasons even if the user + * attempts to manually enable it, such as when the block is in an invalid + * location. This method is deprecated and setDisabledReason should be used + * instead. * * @param enabled True if enabled. */ setEnabled(enabled: boolean) { - if (this.isEnabled() !== enabled) { - const oldValue = this.disabled; - this.disabled = !enabled; + deprecation.warn( + 'setEnabled', + 'v11', + 'v12', + 'Use the setDisabledReason method of Block instead.', + ); + this.setDisabledReason(!enabled, constants.MANUALLY_DISABLED); + } + + /** + * Add or remove a reason why the block might be disabled. If a block has + * any reasons to be disabled, then the block itself will be considered + * disabled. A block could be disabled for multiple independent reasons + * simultaneously, such as when the user manually disables it, or the block + * is invalid. + * + * @param disabled If true, then the block should be considered disabled for + * at least the provided reason, otherwise the block is no longer disabled + * for that reason. + * @param reason A language-neutral identifier for a reason why the block + * could be disabled. Call this method again with the same identifier to + * update whether the block is currently disabled for this reason. + */ + setDisabledReason(disabled: boolean, reason: string): void { + if (this.disabledReasons.has(reason) !== disabled) { + if (disabled) { + this.disabledReasons.add(reason); + } else { + this.disabledReasons.delete(reason); + } eventUtils.fire( new (eventUtils.get(eventUtils.BLOCK_CHANGE))( this, 'disabled', - null, - oldValue, - !enabled, + reason, + /* oldValue= */ !disabled, + /* newValue= */ disabled, ), ); } @@ -1426,10 +1461,10 @@ export class Block implements IASTNodeLocation { * * @returns True if disabled. */ - getInheritedDisabledOrInvalid(): boolean { + getInheritedDisabled(): boolean { let ancestor = this.getSurroundParent(); while (ancestor) { - if (ancestor.disabled || !ancestor.isValid()) { + if (!ancestor.isEnabled()) { return true; } ancestor = ancestor.getSurroundParent(); @@ -1439,67 +1474,24 @@ export class Block implements IASTNodeLocation { } /** - * Get whether this block is valid or not. A block is considered valid if - * there aren't any reasons why it would be invalid. - * - * @returns True if valid. - */ - isValid(): boolean { - return this.invalidReasons.size === 0; - } - - /** - * Add or remove a reason why the block might be invalid. If a block has - * any reasons to be invalid, then the block itself will be considered - * invalid. A block could be invalid for multiple independent reasons - * simultaneously. Automatic processes in the Blockly library and in plugins - * can call this to cooperatively determine whether the block is invalid. - * - * @param invalid If true, then the block should be considered invalid for - * at least the provided reason, otherwise the block is no longer invalid - * for that reason. - * @param reason A language-neutral identifier for a reason why the block - * could be invalid. Call this method again with the same identifier to - * update whether the block is currently invalid for this reason. - */ - setInvalidReason(invalid: boolean, reason: string): void { - if (this.invalidReasons.has(reason) !== invalid) { - if (invalid) { - this.invalidReasons.add(reason); - } else { - this.invalidReasons.delete(reason); - } - eventUtils.fire( - new (eventUtils.get(eventUtils.BLOCK_CHANGE))( - this, - 'invalid', - null, - /* oldValue= */ {[reason]: !invalid}, - /* newValue= */ {[reason]: invalid}, - ), - ); - } - } - - /** - * Get whether the block is currently invalid for the provided reason. + * Get whether the block is currently disabled for the provided reason. * * @param reason A language-neutral identifier for a reason why the block - * could be invalid. - * @returns Whether the block is invalid for the provided reason. + * could be disabled. + * @returns Whether the block is disabled for the provided reason. */ - hasInvalidReason(reason: string): boolean { - return this.invalidReasons.has(reason); + hasDisabledReason(reason: string): boolean { + return this.disabledReasons.has(reason); } /** - * Get a set of reasons why the block is currently invalid, if any. If the - * block is valid, this set will be empty. + * Get a set of reasons why the block is currently disabled, if any. If the + * block is enabled, this set will be empty. * - * @returns The set of reasons why the block is invalid, if any. + * @returns The set of reasons why the block is disabled, if any. */ - getInvalidReasons(): ReadonlySet { - return this.invalidReasons; + getDisabledReasons(): ReadonlySet { + return this.disabledReasons; } /** diff --git a/core/block_svg.ts b/core/block_svg.ts index 51934ad13d8..90d6ab3e596 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -29,6 +29,7 @@ import { LegacyContextMenuOption, } from './contextmenu_registry.js'; import type {BlockMove} from './events/events_block_move.js'; +import * as deprecation from './utils/deprecation.js'; import * as eventUtils from './events/utils.js'; import type {Field} from './field.js'; import {FieldLabel} from './field_label.js'; @@ -124,7 +125,7 @@ export class BlockSvg /** Is this block a BlockSVG? */ override readonly rendered = true; - private visuallyDisabledOrInvalid = false; + private visuallyDisabled = false; /** * Is this block currently rendering? Used to stop recursive render calls @@ -535,7 +536,7 @@ export class BlockSvg } if (!collapsed) { - this.updateDisabledOrInvalid(); + this.updateDisabled(); this.removeInput(collapsedInputName); return; } @@ -882,21 +883,18 @@ export class BlockSvg * * @internal */ - updateDisabledOrInvalid() { - const disabledOrInvalid = - !this.isEnabled() || - !this.isValid() || - this.getInheritedDisabledOrInvalid(); + updateDisabled() { + const disabled = !this.isEnabled() || this.getInheritedDisabled(); - if (this.visuallyDisabledOrInvalid === disabledOrInvalid) { - this.getNextBlock()?.updateDisabledOrInvalid(); + if (this.visuallyDisabled === disabled) { + this.getNextBlock()?.updateDisabled(); return; } this.applyColour(); - this.visuallyDisabledOrInvalid = disabledOrInvalid; + this.visuallyDisabled = disabled; for (const child of this.getChildren(false)) { - child.updateDisabledOrInvalid(); + child.updateDisabled(); } } @@ -1022,40 +1020,48 @@ export class BlockSvg } /** - * Set whether the block is enabled or not. + * @deprecated v11 - Set whether the block is manually enabled or disabled. + * The user can toggle whether a block is disabled from a context menu + * option. A block may still be disabled for other reasons even if the user + * attempts to manually enable it, such as when the block is in an invalid + * location. This method is deprecated and setDisabledReason should be used + * instead. * * @param enabled True if enabled. */ override setEnabled(enabled: boolean) { - if (this.isEnabled() !== enabled) { - super.setEnabled(enabled); - if (!this.getInheritedDisabledOrInvalid()) { - this.updateDisabledOrInvalid(); - } + deprecation.warn( + 'setEnabled', + 'v11', + 'v12', + 'Use the setDisabledReason method of BlockSvg instead.', + ); + const wasEnabled = this.isEnabled(); + super.setEnabled(enabled); + if (this.isEnabled() !== wasEnabled && !this.getInheritedDisabled()) { + this.updateDisabled(); } } /** - * Add or remove a reason why the block might be invalid. If a block has - * any reasons to be invalid, then the block itself will be considered - * invalid. A block could be invalid for multiple independent reasons - * simultaneously. Automatic processes in the Blockly library and in plugins - * can call this to cooperatively determine whether the block is invalid. + * Add or remove a reason why the block might be disabled. If a block has + * any reasons to be disabled, then the block itself will be considered + * disabled. A block could be disabled for multiple independent reasons + * simultaneously, such as when the user manually disables it, or the block + * is invalid. * - * @param invalid If true, then the block should be considered invalid for - * at least the provided reason, otherwise the block is no longer invalid + * @param disabled If true, then the block should be considered disabled for + * at least the provided reason, otherwise the block is no longer disabled * for that reason. * @param reason A language-neutral identifier for a reason why the block - * could be invalid. Call this method again with the same identifier to - * update whether the block is currently invalid for this reason. - */ - override setInvalidReason(invalid: boolean, reason: string): void { - const wasValid = this.isValid(); - super.setInvalidReason(invalid, reason); - if (this.isValid() !== wasValid) { - if (!this.getInheritedDisabledOrInvalid()) { - this.updateDisabledOrInvalid(); - } + * could be disabled. Call this method again with the same identifier to + * update whether the block is currently disabled for this reason. + */ + override setDisabledReason(disabled: boolean, reason: string): void { + const wasEnabled = this.isEnabled(); + super.setDisabledReason(disabled, reason); + if (this.isEnabled() !== wasEnabled && !this.getInheritedDisabled()) { + this.updateDisabled(); } } @@ -1537,8 +1543,8 @@ export class BlockSvg this.updateCollapsed_(); } - if (!this.isEnabled() || !this.isValid()) { - this.updateDisabledOrInvalid(); + if (!this.isEnabled()) { + this.updateDisabled(); } this.workspace.getRenderer().render(this); diff --git a/core/constants.ts b/core/constants.ts index 7c3312f29da..538bd378300 100644 --- a/core/constants.ts +++ b/core/constants.ts @@ -15,3 +15,9 @@ export const COLLAPSED_INPUT_NAME = '_TEMP_COLLAPSED_INPUT'; * The language-neutral ID given to the collapsed field. */ export const COLLAPSED_FIELD_NAME = '_TEMP_COLLAPSED_FIELD'; + +/** + * The language-neutral ID for when the reason why a block is disabled is + * because the user manually disabled it, such as via the context menu. + */ +export const MANUALLY_DISABLED = 'MANUALLY_DISABLED'; diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index fd9d62d1fd6..efda5713ab6 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -13,6 +13,7 @@ import { RegistryItem, Scope, } from './contextmenu_registry.js'; +import * as constants from './constants.js'; import * as dialog from './dialog.js'; import * as Events from './events/events.js'; import * as eventUtils from './events/utils.js'; @@ -466,7 +467,7 @@ export function registerDisable() { block!.workspace.options.disable && block!.isEditable() ) { - if (block!.getInheritedDisabledOrInvalid() || !block!.isValid()) { + if (block!.getInheritedDisabled()) { return 'disabled'; } return 'enabled'; @@ -479,7 +480,10 @@ export function registerDisable() { if (!existingGroup) { eventUtils.setGroup(true); } - block!.setEnabled(!block!.isEnabled()); + block!.setDisabledReason( + !block!.hasDisabledReason(constants.MANUALLY_DISABLED), + constants.MANUALLY_DISABLED, + ); eventUtils.setGroup(existingGroup); }, scopeType: ContextMenuRegistry.ScopeType.BLOCK, diff --git a/core/events/events_block_change.ts b/core/events/events_block_change.ts index cf8198dc842..e9435e606ca 100644 --- a/core/events/events_block_change.ts +++ b/core/events/events_block_change.ts @@ -31,11 +31,14 @@ export class BlockChange extends BlockBase { override type = eventUtils.BLOCK_CHANGE; /** * The element that changed; one of 'field', 'comment', 'collapsed', - * 'disabled', 'invalid', 'inline', or 'mutation' + * 'disabled', 'inline', or 'mutation' */ element?: string; - /** The name of the field that changed, if this is a change to a field. */ + /** + * The name of the field that changed, if this is a change to a field, or + * the identifier of the reason why a block can be disabled. + */ name?: string; /** The original value of the element. */ @@ -168,12 +171,7 @@ export class BlockChange extends BlockBase { block.setCollapsed(!!value); break; case 'disabled': - block.setEnabled(!value); - break; - case 'invalid': - for (const entry of Object.entries(value as Object)) { - block.setInvalidReason(!!entry[1], entry[0]); - } + block.setDisabledReason(!!value, this.name!); break; case 'inline': block.setInputsInline(!!value); diff --git a/core/events/utils.ts b/core/events/utils.ts index 15f217df219..13fad19cc5a 100644 --- a/core/events/utils.ts +++ b/core/events/utils.ts @@ -513,13 +513,13 @@ export function get( } /** - * Set if a block is invalid depending on whether it is properly connected. + * Set if a block is disabled depending on whether it is properly connected. * Use this on applications where all blocks should be connected to a top block. * * @param event Custom data for event. */ export function disableOrphans(event: Abstract) { - const invalidReason = 'orphaned block'; + const disabledReason = 'orphaned block'; if (event.type === MOVE || event.type === CREATE) { const blockEvent = event as BlockMove | BlockCreate; if (!blockEvent.workspaceId) { @@ -538,17 +538,17 @@ export function disableOrphans(event: Abstract) { try { recordUndo = false; const parent = block.getParent(); - if (parent && !parent.hasInvalidReason(invalidReason)) { + if (parent && !parent.hasDisabledReason(disabledReason)) { const children = block.getDescendants(false); for (let i = 0, child; (child = children[i]); i++) { - child.setInvalidReason(false, invalidReason); + child.setDisabledReason(false, disabledReason); } } else if ( (block.outputConnection || block.previousConnection) && !eventWorkspace.isDragging() ) { do { - block.setInvalidReason(true, invalidReason); + block.setDisabledReason(true, disabledReason); block = block.getNextBlock(); } while (block); } diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 360b3ac2f91..1e5cdfd2c3e 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -1206,7 +1206,7 @@ export abstract class Flyout common.getBlockTypeCounts(block), ); while (block) { - block.setEnabled(enable); + block.setDisabledReason(!enable, 'WORKSPACE_AT_BLOCK_CAPACITY'); block = block.getNextBlock(); } } diff --git a/core/generator.ts b/core/generator.ts index c58556ad59d..7bb509f8ba3 100644 --- a/core/generator.ts +++ b/core/generator.ts @@ -243,7 +243,7 @@ export class CodeGenerator { if (!block) { return ''; } - if (!block.isEnabled() || !block.isValid()) { + if (!block.isEnabled()) { // Skip past this block if it is disabled. return opt_thisOnly ? '' : this.blockToCode(block.getNextBlock()); } diff --git a/core/gesture.ts b/core/gesture.ts index a18ab37f4cb..406e8d5f160 100644 --- a/core/gesture.ts +++ b/core/gesture.ts @@ -943,7 +943,7 @@ export class Gesture { 'Cannot do a block click because the target block is ' + 'undefined', ); } - if (this.targetBlock.isEnabled() && this.targetBlock.isValid()) { + if (this.targetBlock.isEnabled()) { if (!eventUtils.getGroup()) { eventUtils.setGroup(true); } diff --git a/core/icons/mutator_icon.ts b/core/icons/mutator_icon.ts index 9ccd97fc07f..7fb3fcf3b81 100644 --- a/core/icons/mutator_icon.ts +++ b/core/icons/mutator_icon.ts @@ -309,8 +309,7 @@ export class MutatorIcon extends Icon implements IHasBubble { e.isUiEvent || e.type === eventUtils.CREATE || (e.type === eventUtils.CHANGE && - (e as BlockChange).element === 'disabled') || - (e as BlockChange).element === 'invalid' + (e as BlockChange).element === 'disabled') ); } diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index fd9d2d47001..5aa6f7f7588 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -453,7 +453,7 @@ export class RenderedConnection extends Connection { super.disconnectInternal(setParent); parent.queueRender(); - child.updateDisabledOrInvalid(); + child.updateDisabled(); child.queueRender(); // Reset visibility, since the child is now a top block. child.getSvgRoot().style.display = 'block'; @@ -502,8 +502,8 @@ export class RenderedConnection extends Connection { const parentBlock = this.getSourceBlock(); const childBlock = renderedChildConnection.getSourceBlock(); - parentBlock.updateDisabledOrInvalid(); - childBlock.updateDisabledOrInvalid(); + parentBlock.updateDisabled(); + childBlock.updateDisabled(); childBlock.queueRender(); // The input the child block is connected to (if any). diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index 0aec0603440..d5c0850a1b8 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -132,11 +132,7 @@ export class PathObject implements IPathObject { this.svgPath.setAttribute('fill', this.style.colourPrimary); this.updateShadow_(block.isShadow()); - this.updateDisabledOrInvalid_( - !block.isEnabled() || - !block.isValid() || - block.getInheritedDisabledOrInvalid(), - ); + this.updateDisabled_(!block.isEnabled() || block.getInheritedDisabled()); } /** @@ -200,7 +196,7 @@ export class PathObject implements IPathObject { * * @param disabled True if disabled. */ - protected updateDisabledOrInvalid_(disabled: boolean) { + protected updateDisabled_(disabled: boolean) { this.setClass_('blocklyDisabled', disabled); if (disabled) { this.svgPath.setAttribute( diff --git a/core/renderers/geras/path_object.ts b/core/renderers/geras/path_object.ts index b6f7db711a8..6b058e5a752 100644 --- a/core/renderers/geras/path_object.ts +++ b/core/renderers/geras/path_object.ts @@ -130,8 +130,8 @@ export class PathObject extends BasePathObject { } } - override updateDisabledOrInvalid_(disabled: boolean) { - super.updateDisabledOrInvalid_(disabled); + override updateDisabled_(disabled: boolean) { + super.updateDisabled_(disabled); if (disabled) { this.svgPath.setAttribute('stroke', 'none'); } diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index b8e59b44ad7..676e83fb1cc 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -9,6 +9,8 @@ import type {Block} from '../block.js'; import type {BlockSvg} from '../block_svg.js'; import type {Connection} from '../connection.js'; +import * as constants from '../constants.js'; +import * as deprecation from '../utils/deprecation.js'; import * as eventUtils from '../events/utils.js'; import {inputTypes} from '../inputs/input_types.js'; import {isSerializable} from '../interfaces/i_serializable.js'; @@ -53,7 +55,7 @@ export interface State { movable?: boolean; editable?: boolean; enabled?: boolean; - invalidReasons?: string[]; + disabledReasons?: string[]; inline?: boolean; data?: string; extraState?: AnyDuringMigration; @@ -159,10 +161,7 @@ function saveAttributes(block: Block, state: State) { state['collapsed'] = true; } if (!block.isEnabled()) { - state['enabled'] = false; - } - if (!block.isValid()) { - state['invalidReasons'] = Array.from(block.getInvalidReasons()); + state['disabledReasons'] = Array.from(block.getDisabledReasons()); } if (!block.isOwnDeletable()) { state['deletable'] = false; @@ -524,11 +523,17 @@ function loadAttributes(block: Block, state: State) { block.setEditable(false); } if (state['enabled'] === false) { - block.setEnabled(false); + deprecation.warn( + 'enabled', + 'v11', + 'v12', + 'Set disabledReasons to ["MANUALLY_DISABLED"] instead.', + ); + block.setDisabledReason(true, constants.MANUALLY_DISABLED); } - if (Array.isArray(state['invalidReasons'])) { - for (const reason of state['invalidReasons']) { - block.setInvalidReason(true, reason); + if (Array.isArray(state['disabledReasons'])) { + for (const reason of state['disabledReasons']) { + block.setDisabledReason(true, reason); } } if (state['inline'] !== undefined) { diff --git a/core/trashcan.ts b/core/trashcan.ts index b48777c4d74..050f506a48f 100644 --- a/core/trashcan.ts +++ b/core/trashcan.ts @@ -656,6 +656,7 @@ export class Trashcan delete json['x']; delete json['y']; delete json['enabled']; + delete json['disabledReasons']; if (json['icons'] && json['icons']['comment']) { const comment = json['icons']['comment']; diff --git a/core/xml.ts b/core/xml.ts index 4dc8574bfb6..5c643c79edb 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -9,6 +9,8 @@ import type {Block} from './block.js'; import type {BlockSvg} from './block_svg.js'; import type {Connection} from './connection.js'; +import * as constants from './constants.js'; +import * as deprecation from './utils/deprecation.js'; import * as eventUtils from './events/utils.js'; import type {Field} from './field.js'; import {IconType} from './icons/icon_types.js'; @@ -272,15 +274,12 @@ export function blockToDom( element.setAttribute('collapsed', 'true'); } if (!block.isEnabled()) { - element.setAttribute('disabled', 'true'); - } - if (!block.isValid()) { // Set the value of the attribute to a comma-separated list of reasons. // Use encodeURIComponent to escape commas in the reasons so that they // won't be confused with separator commas. element.setAttribute( - 'invalid-reasons', - Array.from(block.getInvalidReasons()).map(encodeURIComponent).join(','), + 'disabled-reasons', + Array.from(block.getDisabledReasons()).map(encodeURIComponent).join(','), ); } if (!block.isOwnDeletable()) { @@ -1024,14 +1023,23 @@ function domToBlockHeadless( } const disabled = xmlBlock.getAttribute('disabled'); if (disabled) { - block.setEnabled(disabled !== 'true' && disabled !== 'disabled'); + deprecation.warn( + 'disabled', + 'v11', + 'v12', + 'Set disabled-reasons to "MANUALLY_DISABLED" instead.', + ); + block.setDisabledReason( + disabled === 'true' || disabled === 'disabled', + constants.MANUALLY_DISABLED, + ); } - const invalidReasons = xmlBlock.getAttribute('invalid-reasons'); - if (invalidReasons !== null) { - for (const reason of invalidReasons.split(',')) { + const disabledReasons = xmlBlock.getAttribute('disabled-reasons'); + if (disabledReasons !== null) { + for (const reason of disabledReasons.split(',')) { // Use decodeURIComponent to restore characters that were encoded in the // value, such as commas. - block.setInvalidReason(true, decodeURIComponent(reason)); + block.setDisabledReason(true, decodeURIComponent(reason)); } } const deletable = xmlBlock.getAttribute('deletable'); diff --git a/demos/blockfactory/blocks.js b/demos/blockfactory/blocks.js index aa0cb5647e2..15c8ac864d6 100644 --- a/demos/blockfactory/blocks.js +++ b/demos/blockfactory/blocks.js @@ -880,7 +880,7 @@ function fieldNameCheck(referenceBlock) { var blocks = referenceBlock.workspace.getAllBlocks(false); for (var i = 0, block; block = blocks[i]; i++) { var otherName = block.getFieldValue('FIELDNAME'); - if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid() && + if (block.isEnabled() && !block.getInheritedDisabled() && otherName && otherName.toLowerCase() === name) { count++; } @@ -905,7 +905,7 @@ function inputNameCheck(referenceBlock) { var blocks = referenceBlock.workspace.getAllBlocks(false); for (var i = 0, block; block = blocks[i]; i++) { var otherName = block.getFieldValue('INPUTNAME'); - if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid() && + if (block.isEnabled() && !block.getInheritedDisabled() && otherName && otherName.toLowerCase() === name) { count++; } diff --git a/demos/blockfactory/factory_utils.js b/demos/blockfactory/factory_utils.js index a99259039de..4731d1ce9e7 100644 --- a/demos/blockfactory/factory_utils.js +++ b/demos/blockfactory/factory_utils.js @@ -163,7 +163,7 @@ FactoryUtils.formatJson_ = function(blockType, rootBlock) { var contentsBlock = rootBlock.getInputTargetBlock('INPUTS'); var lastInput = null; while (contentsBlock) { - if (!contentsBlock.disabled && contentsBlock.isValid() && !contentsBlock.getInheritedDisabledOrInvalid()) { + if (contentsBlock.isEnabled() && !contentsBlock.getInheritedDisabled()) { var fields = FactoryUtils.getFieldsJson_( contentsBlock.getInputTargetBlock('FIELDS')); for (var i = 0; i < fields.length; i++) { @@ -247,7 +247,7 @@ FactoryUtils.formatJson_ = function(blockType, rootBlock) { } // Generate colour. var colourBlock = rootBlock.getInputTargetBlock('COLOUR'); - if (colourBlock && !colourBlock.disabled) { + if (colourBlock && colourBlock.isEnabled()) { var hue = parseInt(colourBlock.getFieldValue('HUE'), 10); JS.colour = hue; } @@ -277,7 +277,7 @@ FactoryUtils.formatJavaScript_ = function(blockType, rootBlock, workspace) { 'input_end_row': 'appendEndRowInput'}; var contentsBlock = rootBlock.getInputTargetBlock('INPUTS'); while (contentsBlock) { - if (!contentsBlock.disabled && contentsBlock.isValid() && !contentsBlock.getInheritedDisabledOrInvalid()) { + if (contentsBlock.isEnabled() && !contentsBlock.getInheritedDisabled()) { var name = ''; // Dummy inputs don't have names. Other inputs do. if (contentsBlock.type !== 'input_dummy' && @@ -333,7 +333,7 @@ FactoryUtils.formatJavaScript_ = function(blockType, rootBlock, workspace) { } // Generate colour. var colourBlock = rootBlock.getInputTargetBlock('COLOUR'); - if (colourBlock && !colourBlock.disabled) { + if (colourBlock && colourBlock.isEnabled()) { var hue = parseInt(colourBlock.getFieldValue('HUE'), 10); if (!isNaN(hue)) { code.push(' this.setColour(' + hue + ');'); @@ -377,7 +377,7 @@ FactoryUtils.connectionLineJs_ = function(functionName, typeName, workspace) { FactoryUtils.getFieldsJs_ = function(block) { var fields = []; while (block) { - if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid()) { + if (block.isEnabled() && !block.getInheritedDisabled()) { switch (block.type) { case 'field_static': // Result: 'hello' @@ -484,7 +484,7 @@ FactoryUtils.getFieldsJs_ = function(block) { FactoryUtils.getFieldsJson_ = function(block) { var fields = []; while (block) { - if (!block.disabled && block.isValid() && !block.getInheritedDisabledOrInvalid()) { + if (block.isEnabled() && !block.getInheritedDisabled()) { switch (block.type) { case 'field_static': // Result: 'hello' @@ -614,7 +614,7 @@ FactoryUtils.getOptTypesFrom = function(block, name) { FactoryUtils.getTypesFrom_ = function(block, name) { var typeBlock = block.getInputTargetBlock(name); var types; - if (!typeBlock || typeBlock.disabled) { + if (!typeBlock || !typeBlock.isEnabled()) { types = []; } else if (typeBlock.type === 'type_other') { types = [JSON.stringify(typeBlock.getFieldValue('TYPE'))]; @@ -1015,7 +1015,7 @@ FactoryUtils.savedBlockChanges = function(blockLibraryController) { */ FactoryUtils.getTooltipFromRootBlock_ = function(rootBlock) { var tooltipBlock = rootBlock.getInputTargetBlock('TOOLTIP'); - if (tooltipBlock && !tooltipBlock.disabled) { + if (tooltipBlock && tooltipBlock.isEnabled()) { return tooltipBlock.getFieldValue('TEXT'); } return ''; @@ -1029,7 +1029,7 @@ FactoryUtils.getTooltipFromRootBlock_ = function(rootBlock) { */ FactoryUtils.getHelpUrlFromRootBlock_ = function(rootBlock) { var helpUrlBlock = rootBlock.getInputTargetBlock('HELPURL'); - if (helpUrlBlock && !helpUrlBlock.disabled) { + if (helpUrlBlock && helpUrlBlock.isEnabled()) { return helpUrlBlock.getFieldValue('TEXT'); } return ''; diff --git a/tests/browser/test/basic_playground_test.js b/tests/browser/test/basic_playground_test.js index d91b95f087e..091f2f6b5ce 100644 --- a/tests/browser/test/basic_playground_test.js +++ b/tests/browser/test/basic_playground_test.js @@ -28,11 +28,7 @@ async function getIsCollapsed(browser, blockId) { async function getIsDisabled(browser, blockId) { return await browser.execute((blockId) => { const block = Blockly.getMainWorkspace().getBlockById(blockId); - return ( - !block.isEnabled() || - !block.isValid() || - block.getInheritedDisabledOrInvalid() - ); + return !block.isEnabled() || block.getInheritedDisabled(); }, blockId); } diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 344e08c9af0..e158391fc12 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -2313,15 +2313,15 @@ suite('Blocks', function () { .getInput('STATEMENT') .connection.connect(blockB.previousConnection); // Disable the block and collapse it. - blockA.setEnabled(false); + blockA.setDisabledReason(true, 'test reason'); blockA.setCollapsed(true); // Enable the block before expanding it. - blockA.setEnabled(true); + blockA.setDisabledReason(false, 'test reason'); blockA.setCollapsed(false); // The child blocks should be enabled. - chai.assert.isFalse(blockB.disabled); + chai.assert.isTrue(blockB.isEnabled()); chai.assert.isFalse( blockB.getSvgRoot().classList.contains('blocklyDisabled'), ); @@ -2334,18 +2334,18 @@ suite('Blocks', function () { .connection.connect(blockB.previousConnection); // Disable the child block. - blockB.setEnabled(false); + blockB.setDisabledReason(true, 'test reason'); // Collapse and disable the parent block. blockA.setCollapsed(false); - blockA.setEnabled(false); + blockA.setDisabledReason(true, 'test reason'); // Enable the parent block. - blockA.setEnabled(true); + blockA.setDisabledReason(false, 'test reason'); blockA.setCollapsed(true); // Child blocks should stay disabled if they have been set. - chai.assert.isTrue(blockB.disabled); + chai.assert.isFalse(blockB.isEnabled()); }); test('Disabled blocks from JSON should have proper disabled status', function () { // Nested c-shaped blocks, inner block is disabled @@ -2365,8 +2365,8 @@ suite('Blocks', function () { .getTopBlocks(false)[0] .getChildren()[0]; chai.assert.isTrue( - innerBlock.visuallyDisabledOrInvalid, - 'block should have visuallyDisabledOrInvalid set because it is disabled', + innerBlock.visuallyDisabled, + 'block should have visuallyDisabled set because it is disabled', ); chai.assert.isFalse( innerBlock.isEnabled(), @@ -2390,8 +2390,8 @@ suite('Blocks', function () { .getTopBlocks(false)[0] .getChildren()[0]; chai.assert.isTrue( - innerBlock.visuallyDisabledOrInvalid, - 'block should have visuallyDisabledOrInvalid set because it is disabled', + innerBlock.visuallyDisabled, + 'block should have visuallyDisabled set because it is disabled', ); chai.assert.isFalse( innerBlock.isEnabled(), @@ -2440,19 +2440,19 @@ suite('Blocks', function () { this.child4 = this.workspace.getBlockById('child4'); }); test('Disabling parent block visually disables all descendants', async function () { - this.parent.setEnabled(false); + this.parent.setDisabledReason(true, 'test reason'); await Blockly.renderManagement.finishQueuedRenders(); for (const child of this.parent.getDescendants(false)) { chai.assert.isTrue( - child.visuallyDisabledOrInvalid, + child.visuallyDisabled, `block ${child.id} should be visually disabled`, ); } }); test('Child blocks regain original status after parent is re-enabled', async function () { - this.parent.setEnabled(false); + this.parent.setDisabledReason(true, 'test reason'); await Blockly.renderManagement.finishQueuedRenders(); - this.parent.setEnabled(true); + this.parent.setDisabledReason(false, 'test reason'); await Blockly.renderManagement.finishQueuedRenders(); // child2 is disabled, rest should be enabled @@ -2461,7 +2461,7 @@ suite('Blocks', function () { 'child1 should be enabled', ); chai.assert.isFalse( - this.child1.visuallyDisabledOrInvalid, + this.child1.visuallyDisabled, 'child1 should not be visually disabled', ); @@ -2470,7 +2470,7 @@ suite('Blocks', function () { 'child2 should be disabled', ); chai.assert.isTrue( - this.child2.visuallyDisabledOrInvalid, + this.child2.visuallyDisabled, 'child2 should be visually disabled', ); @@ -2479,7 +2479,7 @@ suite('Blocks', function () { 'child3 should be enabled', ); chai.assert.isFalse( - this.child3.visuallyDisabledOrInvalid, + this.child3.visuallyDisabled, 'child3 should not be visually disabled', ); @@ -2488,7 +2488,7 @@ suite('Blocks', function () { 'child34 should be enabled', ); chai.assert.isFalse( - this.child4.visuallyDisabledOrInvalid, + this.child4.visuallyDisabled, 'child4 should not be visually disabled', ); }); diff --git a/tests/mocha/blocks/loops_test.js b/tests/mocha/blocks/loops_test.js index 8740b5ef789..3bbfdac1084 100644 --- a/tests/mocha/blocks/loops_test.js +++ b/tests/mocha/blocks/loops_test.js @@ -28,8 +28,8 @@ suite('Loops', function () { ); this.clock.runAll(); chai.assert.isFalse( - breakBlock.isValid(), - 'Expected the break block to be invalid', + breakBlock.isEnabled(), + 'Expected the break block to be disabled', ); }); @@ -47,8 +47,8 @@ suite('Loops', function () { .connection.connect(breakBlock.previousConnection); this.clock.runAll(); chai.assert.isTrue( - breakBlock.isValid(), - 'Expected the break block to be valid', + breakBlock.isEnabled(), + 'Expected the break block to be enabled', ); }); }); diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 92257d31837..56351293217 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -811,11 +811,11 @@ suite('Procedures', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); - defBlock.setEnabled(false); + defBlock.setDisabledReason(true, 'MANUALLY_DISABLED'); this.clock.runAll(); chai.assert.isFalse( - callBlock.isValid(), + callBlock.isEnabled(), 'Expected the caller block to be invalid', ); }, @@ -828,11 +828,11 @@ suite('Procedures', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); - defBlock.setInvalidReason(true, 'test reason'); + defBlock.setDisabledReason(true, 'test reason'); this.clock.runAll(); chai.assert.isFalse( - callBlock.isValid(), + callBlock.isEnabled(), 'Expected the caller block to be invalid', ); }, @@ -844,14 +844,14 @@ suite('Procedures', function () { function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); - defBlock.setEnabled(false); + defBlock.setDisabledReason(true, 'MANUALLY_DISABLED'); this.clock.runAll(); - defBlock.setEnabled(true); + defBlock.setDisabledReason(false, 'MANUALLY_DISABLED'); this.clock.runAll(); chai.assert.isTrue( - callBlock.isValid(), + callBlock.isEnabled(), 'Expected the caller block to be valid', ); }, @@ -864,12 +864,12 @@ suite('Procedures', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); this.clock.runAll(); - callBlock.setEnabled(false); + callBlock.setDisabledReason(true, 'MANUALLY_DISABLED'); this.clock.runAll(); - defBlock.setEnabled(false); + defBlock.setDisabledReason(true, 'MANUALLY_DISABLED'); this.clock.runAll(); - defBlock.setEnabled(true); + defBlock.setDisabledReason(false, 'MANUALLY_DISABLED'); this.clock.runAll(); chai.assert.isFalse( @@ -888,7 +888,7 @@ suite('Procedures', function () { ); this.clock.runAll(); chai.assert.isFalse( - ifreturnBlock.isValid(), + ifreturnBlock.isEnabled(), 'Expected the ifreturn block to be invalid', ); }); @@ -904,7 +904,7 @@ suite('Procedures', function () { .connection.connect(ifreturnBlock.previousConnection); this.clock.runAll(); chai.assert.isTrue( - ifreturnBlock.isValid(), + ifreturnBlock.isEnabled(), 'Expected the ifreturn block to be valid', ); }); diff --git a/tests/mocha/event_test.js b/tests/mocha/event_test.js index 3e9a13058f3..518118c5e7d 100644 --- a/tests/mocha/event_test.js +++ b/tests/mocha/event_test.js @@ -1424,7 +1424,7 @@ suite('Events', function () { sinon.assert.calledOnce(genUidStub); // When block is created using domToWorkspace, 5 events are fired: - // 1. varCreate (events invalid) + // 1. varCreate (events disabled) // 2. varCreate // 3. block create // 4. move (no-op, is filtered out) @@ -1479,7 +1479,7 @@ suite('Events', function () { teardown(function () { workspaceTeardown.call(this, this.workspace); }); - test('Created orphan block is invalid', function () { + test('Created orphan block is disabled', function () { this.workspace.addChangeListener(eventUtils.disableOrphans); const block = this.workspace.newBlock('controls_for'); block.initSvg(); @@ -1489,11 +1489,11 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isFalse( - block.isValid(), - 'Expected orphan block to be invalid after creation', + block.isEnabled(), + 'Expected orphan block to be disabled after creation', ); }); - test('Created procedure block is valid', function () { + test('Created procedure block is enabled', function () { this.workspace.addChangeListener(eventUtils.disableOrphans); // Procedure block is never an orphan @@ -1505,8 +1505,8 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isTrue( - functionBlock.isValid(), - 'Expected top-level procedure block to be valid', + functionBlock.isEnabled(), + 'Expected top-level procedure block to be enabled', ); }); test('Moving a block to top-level disables it', function () { @@ -1529,8 +1529,8 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isFalse( - block.isValid(), - 'Expected disconnected block to be invalid', + block.isEnabled(), + 'Expected disconnected block to be disabled', ); }); test('Giving block a parent enables it', function () { @@ -1550,8 +1550,8 @@ suite('Events', function () { this.clock.runAll(); chai.assert.isTrue( - block.isValid(), - 'Expected block to be valid after connecting to parent', + block.isEnabled(), + 'Expected block to be enabled after connecting to parent', ); }); test('disableOrphans events are not undoable', function () { @@ -1573,12 +1573,12 @@ suite('Events', function () { // Fire all events this.clock.runAll(); - const invalidEvents = this.workspace.getUndoStack().filter(function (e) { - return e.element === 'invalid'; + const disabledEvents = this.workspace.getUndoStack().filter(function (e) { + return e.element === 'disabled'; }); chai.assert.isEmpty( - invalidEvents, - 'Undo stack should not contain any invalid events', + disabledEvents, + 'Undo stack should not contain any disabled events', ); }); }); diff --git a/tests/mocha/generator_test.js b/tests/mocha/generator_test.js index 815ad3ede65..3a2679dca81 100644 --- a/tests/mocha/generator_test.js +++ b/tests/mocha/generator_test.js @@ -81,7 +81,6 @@ suite('Generator', function () { this.blockToCodeTest = function ( generator, blockDisabled, - blockInvalid, opt_thisOnly, expectedCode, opt_message, @@ -93,8 +92,7 @@ suite('Generator', function () { return 'stack_block'; }; rowBlock.nextConnection.connect(stackBlock.previousConnection); - rowBlock.disabled = blockDisabled; - rowBlock.setInvalidReason(blockInvalid, 'test reason'); + rowBlock.setDisabledReason(blockDisabled, 'test reason'); const code = generator.blockToCode(rowBlock, opt_thisOnly); delete generator.forBlock['stack_block']; @@ -120,14 +118,12 @@ suite('Generator', function () { this.blockToCodeTest( generator, /* blockDisabled = */ false, - /* blockInvalid = */ false, /* opt_thisOnly = */ true, 'row_block', ); this.blockToCodeTest( generator, /* blockDisabled = */ false, - /* blockInvalid = */ false, /* opt_thisOnly = */ false, 'row_blockstack_block', 'thisOnly=false', @@ -144,38 +140,12 @@ suite('Generator', function () { this.blockToCodeTest( generator, /* blockDisabled = */ true, - /* blockInvalid = */ false, /* opt_thisOnly = */ true, '', ); this.blockToCodeTest( generator, /* blockDisabled = */ true, - /* blockInvalid = */ false, - /* opt_thisOnly = */ false, - 'stack_block', - 'thisOnly=false', - ); - }); - }); - }); - - suite('Invalid block', function () { - testCase.forEach(function (testCase) { - const generator = testCase[0]; - const name = testCase[1]; - test(name, function () { - this.blockToCodeTest( - generator, - /* blockDisabled = */ false, - /* blockInvalid = */ true, - /* opt_thisOnly = */ true, - '', - ); - this.blockToCodeTest( - generator, - /* blockDisabled = */ false, - /* blockInvalid = */ true, /* opt_thisOnly = */ false, 'stack_block', 'thisOnly=false', diff --git a/tests/mocha/icon_test.js b/tests/mocha/icon_test.js index 63c723495e0..3463d8ad83b 100644 --- a/tests/mocha/icon_test.js +++ b/tests/mocha/icon_test.js @@ -157,7 +157,7 @@ suite('Icon', function () { block.addIcon(icon); applyColourSpy.resetHistory(); - block.setEnabled(false); + block.setDisabledReason(true, 'test reason'); chai.assert.isTrue( applyColourSpy.calledOnce, 'Expected applyColour to be called', diff --git a/tests/mocha/jso_serialization_test.js b/tests/mocha/jso_serialization_test.js index 66b6f4c9784..9a28e31c7de 100644 --- a/tests/mocha/jso_serialization_test.js +++ b/tests/mocha/jso_serialization_test.js @@ -89,16 +89,16 @@ suite('JSO Serialization', function () { suite('Enabled', function () { test('False', function () { const block = this.workspace.newBlock('row_block'); - block.setEnabled(false); + block.setDisabledReason(true, 'test reason'); const jso = Blockly.serialization.blocks.save(block); - assertProperty(jso, 'enabled', false); + assertProperty(jso, 'disabledReasons', ['test reason']); }); test('True', function () { const block = this.workspace.newBlock('row_block'); - block.setEnabled(true); + block.setDisabledReason(false, 'test reason'); const jso = Blockly.serialization.blocks.save(block); - assertNoProperty(jso, 'enabled'); + assertNoProperty(jso, 'disabledReasons'); }); }); diff --git a/tests/mocha/serializer_test.js b/tests/mocha/serializer_test.js index 290f7fa45e8..35fcd0c9096 100644 --- a/tests/mocha/serializer_test.js +++ b/tests/mocha/serializer_test.js @@ -81,7 +81,7 @@ Serializer.Attributes.Collapsed = new SerializerTestCase( Serializer.Attributes.Disabled = new SerializerTestCase( 'Disabled', '' + - '' + + '' + '', ); Serializer.Attributes.NotDeletable = new SerializerTestCase( From e960e3ffa563b5317b665c0a8484c58b8fec0cae Mon Sep 17 00:00:00 2001 From: John Nesky Date: Tue, 2 Apr 2024 21:10:56 -0700 Subject: [PATCH 4/9] Minor fixes. --- blocks/loops.ts | 2 +- blocks/procedures.ts | 8 ++++---- core/block.ts | 11 +++++++++++ core/contextmenu_items.ts | 6 +++--- core/events/utils.ts | 2 +- tests/mocha/blocks/procedures_test.js | 8 ++++---- 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/blocks/loops.ts b/blocks/loops.ts index 041252f340c..dec09751b9e 100644 --- a/blocks/loops.ts +++ b/blocks/loops.ts @@ -378,7 +378,7 @@ const CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setDisabledReason(!valid, 'UNPARENTED_CONTROL_FLOW'); + this.setDisabledReason(!valid, 'CONTROL_FLOW_NOT_IN_LOOP'); Events.setGroup(group); } }, diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 384bbe0dcad..42c86949907 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -1280,17 +1280,17 @@ const PROCEDURES_IFRETURN = { ) { return; // Don't change state at the start of a drag. } - let valid = false; + let legal = false; // Is the block nested in a procedure? let block = this; // eslint-disable-line @typescript-eslint/no-this-alias do { if (this.FUNCTION_TYPES.includes(block.type)) { - valid = true; + legal = true; break; } block = block.getSurroundParent()!; } while (block); - if (valid) { + if (legal) { // If needed, toggle whether this block has a return value. if (block.type === 'procedures_defnoreturn' && this.hasReturnValue_) { this.removeInput('VALUE'); @@ -1316,7 +1316,7 @@ const PROCEDURES_IFRETURN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setDisabledReason(!valid, 'UNPARENTED_IFRETURN'); + this.setDisabledReason(!legal, 'UNPARENTED_IFRETURN'); Events.setGroup(group); } }, diff --git a/core/block.ts b/core/block.ts index 8bc5e46110c..12b4c85d4c5 100644 --- a/core/block.ts +++ b/core/block.ts @@ -1402,6 +1402,17 @@ export class Block implements IASTNodeLocation { return this.disabledReasons.size === 0; } + /** @deprecated v11 - Get whether the block is manually disabled. */ + get disabled() { + deprecation.warn( + 'disabled', + 'v11', + 'v12', + 'Use the isEnabled or hasDisabledReason methods of Block instead.', + ); + return this.hasDisabledReason(constants.MANUALLY_DISABLED); + } + /** * @deprecated v11 - Set whether the block is manually enabled or disabled. * The user can toggle whether a block is disabled from a context menu diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index efda5713ab6..d5616c01d69 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -456,9 +456,9 @@ export function registerCollapseExpandBlock() { export function registerDisable() { const disableOption: RegistryItem = { displayText(scope: Scope) { - return scope.block!.isEnabled() - ? Msg['DISABLE_BLOCK'] - : Msg['ENABLE_BLOCK']; + return scope.block!.hasDisabledReason(constants.MANUALLY_DISABLED) + ? Msg['ENABLE_BLOCK'] + : Msg['DISABLE_BLOCK']; }, preconditionFn(scope: Scope) { const block = scope.block; diff --git a/core/events/utils.ts b/core/events/utils.ts index 13fad19cc5a..b7836618a00 100644 --- a/core/events/utils.ts +++ b/core/events/utils.ts @@ -519,7 +519,7 @@ export function get( * @param event Custom data for event. */ export function disableOrphans(event: Abstract) { - const disabledReason = 'orphaned block'; + const disabledReason = 'ORPHANED_BLOCK'; if (event.type === MOVE || event.type === CREATE) { const blockEvent = event as BlockMove | BlockCreate; if (!blockEvent.workspaceId) { diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 56351293217..109d3b2d4eb 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -806,7 +806,7 @@ suite('Procedures', function () { suite('enabling and disabling procedure blocks', function () { test( 'if a procedure definition is disabled, the procedure caller ' + - 'is invalid', + 'is also disabled', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); @@ -816,7 +816,7 @@ suite('Procedures', function () { chai.assert.isFalse( callBlock.isEnabled(), - 'Expected the caller block to be invalid', + 'Expected the caller block to be disabled', ); }, ); @@ -840,7 +840,7 @@ suite('Procedures', function () { test( 'if a procedure definition is enabled, the procedure caller ' + - 'is valid', + 'is also enabled', function () { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); @@ -852,7 +852,7 @@ suite('Procedures', function () { chai.assert.isTrue( callBlock.isEnabled(), - 'Expected the caller block to be valid', + 'Expected the caller block to be enabled', ); }, ); From aeb84021cffe8a197a3937552f0f63ff338de059 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Tue, 2 Apr 2024 21:53:35 -0700 Subject: [PATCH 5/9] More minor fixes. --- core/block.ts | 17 ++++++++++++++--- core/block_svg.ts | 2 +- core/serialization/blocks.ts | 2 +- core/xml.ts | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/core/block.ts b/core/block.ts index 12b4c85d4c5..34503cf0bd8 100644 --- a/core/block.ts +++ b/core/block.ts @@ -1403,16 +1403,27 @@ export class Block implements IASTNodeLocation { } /** @deprecated v11 - Get whether the block is manually disabled. */ - get disabled() { + get disabled(): boolean { deprecation.warn( 'disabled', 'v11', 'v12', - 'Use the isEnabled or hasDisabledReason methods of Block instead.', + 'the isEnabled or hasDisabledReason methods of Block', ); return this.hasDisabledReason(constants.MANUALLY_DISABLED); } + /** @deprecated v11 - Set whether the block is manually disabled. */ + set disabled(value: boolean) { + deprecation.warn( + 'disabled', + 'v11', + 'v12', + 'the setDisabledReason method of Block', + ); + this.setDisabledReason(value, constants.MANUALLY_DISABLED); + } + /** * @deprecated v11 - Set whether the block is manually enabled or disabled. * The user can toggle whether a block is disabled from a context menu @@ -1428,7 +1439,7 @@ export class Block implements IASTNodeLocation { 'setEnabled', 'v11', 'v12', - 'Use the setDisabledReason method of Block instead.', + 'the setDisabledReason method of Block', ); this.setDisabledReason(!enabled, constants.MANUALLY_DISABLED); } diff --git a/core/block_svg.ts b/core/block_svg.ts index 90d6ab3e596..37f17ddf0bf 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -1034,7 +1034,7 @@ export class BlockSvg 'setEnabled', 'v11', 'v12', - 'Use the setDisabledReason method of BlockSvg instead.', + 'the setDisabledReason method of BlockSvg', ); const wasEnabled = this.isEnabled(); super.setEnabled(enabled); diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index 676e83fb1cc..6e945c378d6 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -527,7 +527,7 @@ function loadAttributes(block: Block, state: State) { 'enabled', 'v11', 'v12', - 'Set disabledReasons to ["MANUALLY_DISABLED"] instead.', + 'disabledReasons to ["MANUALLY_DISABLED"]', ); block.setDisabledReason(true, constants.MANUALLY_DISABLED); } diff --git a/core/xml.ts b/core/xml.ts index 5c643c79edb..64df838365f 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -1027,7 +1027,7 @@ function domToBlockHeadless( 'disabled', 'v11', 'v12', - 'Set disabled-reasons to "MANUALLY_DISABLED" instead.', + 'disabled-reasons to "MANUALLY_DISABLED"', ); block.setDisabledReason( disabled === 'true' || disabled === 'disabled', From dd1ff7352117df019f9c2ab8d55c6c4e661b8a54 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Tue, 2 Apr 2024 22:00:39 -0700 Subject: [PATCH 6/9] Reverting some stuff that didn't need to change. --- blocks/loops.ts | 8 +++++--- core/block.ts | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/blocks/loops.ts b/blocks/loops.ts index dec09751b9e..8729619a467 100644 --- a/blocks/loops.ts +++ b/blocks/loops.ts @@ -372,13 +372,15 @@ const CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { ) { return; } - const valid = !!this.getSurroundLoop(); - this.setWarningText(valid ? null : Msg['CONTROLS_FLOW_STATEMENTS_WARNING']); + const enabled = !!this.getSurroundLoop(); + this.setWarningText( + enabled ? null : Msg['CONTROLS_FLOW_STATEMENTS_WARNING'], + ); if (!this.isInFlyout) { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setDisabledReason(!valid, 'CONTROL_FLOW_NOT_IN_LOOP'); + this.setDisabledReason(!enabled, 'CONTROL_FLOW_NOT_IN_LOOP'); Events.setGroup(group); } }, diff --git a/core/block.ts b/core/block.ts index 34503cf0bd8..96e22065ca6 100644 --- a/core/block.ts +++ b/core/block.ts @@ -1403,7 +1403,7 @@ export class Block implements IASTNodeLocation { } /** @deprecated v11 - Get whether the block is manually disabled. */ - get disabled(): boolean { + private get disabled(): boolean { deprecation.warn( 'disabled', 'v11', @@ -1414,7 +1414,7 @@ export class Block implements IASTNodeLocation { } /** @deprecated v11 - Set whether the block is manually disabled. */ - set disabled(value: boolean) { + private set disabled(value: boolean) { deprecation.warn( 'disabled', 'v11', From f05dc9696eebd21f076b06f4ba6b60f425498b75 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Fri, 12 Apr 2024 17:38:09 -0700 Subject: [PATCH 7/9] Addressing PR feedback. --- blocks/loops.ts | 10 +++++++++- blocks/procedures.ts | 20 ++++++++++++++++++-- core/contextmenu_items.ts | 9 ++++++++- core/events/utils.ts | 16 ++++++++++++---- core/flyout_base.ts | 12 +++++++++++- core/serialization/blocks.ts | 2 +- core/xml.ts | 2 +- tests/mocha/jso_serialization_test.js | 17 ++++++++++++++--- tests/mocha/serializer_test.js | 7 +++++++ 9 files changed, 81 insertions(+), 14 deletions(-) diff --git a/blocks/loops.ts b/blocks/loops.ts index 8729619a467..50e4fcff96f 100644 --- a/blocks/loops.ts +++ b/blocks/loops.ts @@ -334,6 +334,11 @@ export type ControlFlowInLoopBlock = Block & ControlFlowInLoopMixin; interface ControlFlowInLoopMixin extends ControlFlowInLoopMixinType {} type ControlFlowInLoopMixinType = typeof CONTROL_FLOW_IN_LOOP_CHECK_MIXIN; +/** + * The language-neutral ID for when the reason why a block is disabled is + * because the block is only valid inside of a loop. + */ +const CONTROL_FLOW_NOT_IN_LOOP_DISABLED_REASON = 'CONTROL_FLOW_NOT_IN_LOOP'; /** * This mixin adds a check to make sure the 'controls_flow_statements' block * is contained in a loop. Otherwise a warning is added to the block. @@ -380,7 +385,10 @@ const CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setDisabledReason(!enabled, 'CONTROL_FLOW_NOT_IN_LOOP'); + this.setDisabledReason( + !enabled, + CONTROL_FLOW_NOT_IN_LOOP_DISABLED_REASON, + ); Events.setGroup(group); } }, diff --git a/blocks/procedures.ts b/blocks/procedures.ts index d82c2056e9c..32139264429 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -763,6 +763,13 @@ type CallExtraState = { params?: string[]; }; +/** + * The language-neutral ID for when the reason why a block is disabled is + * because the block's corresponding procedure definition is disabled. + */ +const DISABLED_PROCEDURE_DEFINITION_DISABLED_REASON = + 'DISABLED_PROCEDURE_DEFINITION'; + /** * Common properties for the procedure_callnoreturn and * procedure_callreturn blocks. @@ -1124,7 +1131,10 @@ const PROCEDURE_CALL_COMMON = { } Events.setGroup(event.group); const valid = def.isEnabled(); - this.setDisabledReason(!valid, 'DISABLED_PROCEDURE_DEFINITION'); + this.setDisabledReason( + !valid, + DISABLED_PROCEDURE_DEFINITION_DISABLED_REASON, + ); this.setWarningText( valid ? null @@ -1217,6 +1227,12 @@ interface IfReturnMixin extends IfReturnMixinType { } type IfReturnMixinType = typeof PROCEDURES_IFRETURN; +/** + * The language-neutral ID for when the reason why a block is disabled is + * because the block is only valid inside of a procedure body. + */ +const UNPARENTED_IFRETURN_DISABLED_REASON = 'UNPARENTED_IFRETURN'; + const PROCEDURES_IFRETURN = { /** * Block for conditionally returning a value from a procedure. @@ -1317,7 +1333,7 @@ const PROCEDURES_IFRETURN = { const group = Events.getGroup(); // Makes it so the move and the disable event get undone together. Events.setGroup(e.group); - this.setDisabledReason(!legal, 'UNPARENTED_IFRETURN'); + this.setDisabledReason(!legal, UNPARENTED_IFRETURN_DISABLED_REASON); Events.setGroup(group); } }, diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index d5616c01d69..1c8d98cdb08 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -467,7 +467,14 @@ export function registerDisable() { block!.workspace.options.disable && block!.isEditable() ) { - if (block!.getInheritedDisabled()) { + // Determine whether this block is currently disabled for any reason + // other than the manual reason that this context menu item controls. + const disabledReasons = block!.getDisabledReasons(); + const isDisabledForOtherReason = + disabledReasons.size > + (disabledReasons.has(constants.MANUALLY_DISABLED) ? 1 : 0); + + if (block!.getInheritedDisabled() || isDisabledForOtherReason) { return 'disabled'; } return 'enabled'; diff --git a/core/events/utils.ts b/core/events/utils.ts index 39da9a756c5..eacf0490673 100644 --- a/core/events/utils.ts +++ b/core/events/utils.ts @@ -188,6 +188,12 @@ export const COMMENT_COLLAPSE = 'comment_collapse'; */ export const FINISHED_LOADING = 'finished_loading'; +/** + * The language-neutral ID for when the reason why a block is disabled is + * because the block is not descended from a root block. + */ +const ORPHANED_BLOCK_DISABLED_REASON = 'ORPHANED_BLOCK'; + /** * Type of events that cause objects to be bumped back into the visible * portion of the workspace. @@ -522,7 +528,6 @@ export function get( * @param event Custom data for event. */ export function disableOrphans(event: Abstract) { - const disabledReason = 'ORPHANED_BLOCK'; if (event.type === MOVE || event.type === CREATE) { const blockEvent = event as BlockMove | BlockCreate; if (!blockEvent.workspaceId) { @@ -541,17 +546,20 @@ export function disableOrphans(event: Abstract) { try { recordUndo = false; const parent = block.getParent(); - if (parent && !parent.hasDisabledReason(disabledReason)) { + if ( + parent && + !parent.hasDisabledReason(ORPHANED_BLOCK_DISABLED_REASON) + ) { const children = block.getDescendants(false); for (let i = 0, child; (child = children[i]); i++) { - child.setDisabledReason(false, disabledReason); + child.setDisabledReason(false, ORPHANED_BLOCK_DISABLED_REASON); } } else if ( (block.outputConnection || block.previousConnection) && !eventWorkspace.isDragging() ) { do { - block.setDisabledReason(true, disabledReason); + block.setDisabledReason(true, ORPHANED_BLOCK_DISABLED_REASON); block = block.getNextBlock(); } while (block); } diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 10bd4c39cde..ce1959a377f 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -43,6 +43,13 @@ enum FlyoutItemType { BUTTON = 'button', } +/** + * The language-neutral ID for when the reason why a block is disabled is + * because the workspace is at block capacity. + */ +const WORKSPACE_AT_BLOCK_CAPACITY_DISABLED_REASON = + 'WORKSPACE_AT_BLOCK_CAPACITY'; + /** * Class for a flyout. */ @@ -1239,7 +1246,10 @@ export abstract class Flyout common.getBlockTypeCounts(block), ); while (block) { - block.setDisabledReason(!enable, 'WORKSPACE_AT_BLOCK_CAPACITY'); + block.setDisabledReason( + !enable, + WORKSPACE_AT_BLOCK_CAPACITY_DISABLED_REASON, + ); block = block.getNextBlock(); } } diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index 6e945c378d6..7a59ca0bc36 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -527,7 +527,7 @@ function loadAttributes(block: Block, state: State) { 'enabled', 'v11', 'v12', - 'disabledReasons to ["MANUALLY_DISABLED"]', + 'disabledReasons with the value ["' + constants.MANUALLY_DISABLED + '"]', ); block.setDisabledReason(true, constants.MANUALLY_DISABLED); } diff --git a/core/xml.ts b/core/xml.ts index 64df838365f..3f90b7390d8 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -1027,7 +1027,7 @@ function domToBlockHeadless( 'disabled', 'v11', 'v12', - 'disabled-reasons to "MANUALLY_DISABLED"', + 'disabled-reasons with the value "' + constants.MANUALLY_DISABLED + '"', ); block.setDisabledReason( disabled === 'true' || disabled === 'disabled', diff --git a/tests/mocha/jso_serialization_test.js b/tests/mocha/jso_serialization_test.js index 9a28e31c7de..72a74ad3d6a 100644 --- a/tests/mocha/jso_serialization_test.js +++ b/tests/mocha/jso_serialization_test.js @@ -86,20 +86,31 @@ suite('JSO Serialization', function () { }); }); - suite('Enabled', function () { - test('False', function () { + suite('DisabledReasons', function () { + test('One reason', function () { const block = this.workspace.newBlock('row_block'); block.setDisabledReason(true, 'test reason'); const jso = Blockly.serialization.blocks.save(block); assertProperty(jso, 'disabledReasons', ['test reason']); }); - test('True', function () { + test('Zero reasons', function () { const block = this.workspace.newBlock('row_block'); block.setDisabledReason(false, 'test reason'); const jso = Blockly.serialization.blocks.save(block); assertNoProperty(jso, 'disabledReasons'); }); + + test('Multiple reasons', function () { + const block = this.workspace.newBlock('row_block'); + block.setDisabledReason(true, 'test reason 1'); + block.setDisabledReason(true, 'test reason 2'); + const jso = Blockly.serialization.blocks.save(block); + assertProperty(jso, 'disabledReasons', [ + 'test reason 1', + 'test reason 2', + ]); + }); }); suite('Inline', function () { diff --git a/tests/mocha/serializer_test.js b/tests/mocha/serializer_test.js index 35fcd0c9096..b10f48df515 100644 --- a/tests/mocha/serializer_test.js +++ b/tests/mocha/serializer_test.js @@ -84,6 +84,12 @@ Serializer.Attributes.Disabled = new SerializerTestCase( '' + '', ); +Serializer.Attributes.DisabledWithEncodedComma = new SerializerTestCase( + 'DisabledWithEncodedComma', + '' + + '' + + '', +); Serializer.Attributes.NotDeletable = new SerializerTestCase( 'Deletable', '' + @@ -106,6 +112,7 @@ Serializer.Attributes.testCases = [ Serializer.Attributes.Basic, Serializer.Attributes.Collapsed, Serializer.Attributes.Disabled, + Serializer.Attributes.DisabledWithEncodedComma, Serializer.Attributes.NotDeletable, Serializer.Attributes.NotMovable, Serializer.Attributes.NotEditable, From 2d9015e9fa3ed1b0bacceac81114241a11efb866 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Wed, 17 Apr 2024 19:33:06 -0700 Subject: [PATCH 8/9] Update the BlockInfo interface to match State. --- core/block.ts | 19 +++++++------- core/contextmenu_items.ts | 10 +++---- core/events/events_block_change.ts | 42 ++++++++++++++++++++++++++---- core/flyout_base.ts | 7 +++++ core/serialization/blocks.ts | 6 ++--- core/utils/toolbox.ts | 1 + core/xml.ts | 6 ++--- 7 files changed, 66 insertions(+), 25 deletions(-) diff --git a/core/block.ts b/core/block.ts index 96e22065ca6..52191d63c3c 100644 --- a/core/block.ts +++ b/core/block.ts @@ -25,6 +25,7 @@ import {ConnectionType} from './connection_type.js'; import * as constants from './constants.js'; import {DuplicateIconType} from './icons/exceptions.js'; import type {Abstract} from './events/events_abstract.js'; +import type {BlockChange} from './events/events_block_change.js'; import type {BlockMove} from './events/events_block_move.js'; import * as deprecation from './utils/deprecation.js'; import * as eventUtils from './events/utils.js'; @@ -1465,15 +1466,15 @@ export class Block implements IASTNodeLocation { } else { this.disabledReasons.delete(reason); } - eventUtils.fire( - new (eventUtils.get(eventUtils.BLOCK_CHANGE))( - this, - 'disabled', - reason, - /* oldValue= */ !disabled, - /* newValue= */ disabled, - ), - ); + const blockChangeEvent = new (eventUtils.get(eventUtils.BLOCK_CHANGE))( + this, + 'disabled', + /* name= */ null, + /* oldValue= */ !disabled, + /* newValue= */ disabled, + ) as BlockChange; + blockChangeEvent.setDisabledReason(reason); + eventUtils.fire(blockChangeEvent); } } diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index 1c8d98cdb08..8aa6c1e7212 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -13,7 +13,7 @@ import { RegistryItem, Scope, } from './contextmenu_registry.js'; -import * as constants from './constants.js'; +import {MANUALLY_DISABLED} from './constants.js'; import * as dialog from './dialog.js'; import * as Events from './events/events.js'; import * as eventUtils from './events/utils.js'; @@ -456,7 +456,7 @@ export function registerCollapseExpandBlock() { export function registerDisable() { const disableOption: RegistryItem = { displayText(scope: Scope) { - return scope.block!.hasDisabledReason(constants.MANUALLY_DISABLED) + return scope.block!.hasDisabledReason(MANUALLY_DISABLED) ? Msg['ENABLE_BLOCK'] : Msg['DISABLE_BLOCK']; }, @@ -472,7 +472,7 @@ export function registerDisable() { const disabledReasons = block!.getDisabledReasons(); const isDisabledForOtherReason = disabledReasons.size > - (disabledReasons.has(constants.MANUALLY_DISABLED) ? 1 : 0); + (disabledReasons.has(MANUALLY_DISABLED) ? 1 : 0); if (block!.getInheritedDisabled() || isDisabledForOtherReason) { return 'disabled'; @@ -488,8 +488,8 @@ export function registerDisable() { eventUtils.setGroup(true); } block!.setDisabledReason( - !block!.hasDisabledReason(constants.MANUALLY_DISABLED), - constants.MANUALLY_DISABLED, + !block!.hasDisabledReason(MANUALLY_DISABLED), + MANUALLY_DISABLED, ); eventUtils.setGroup(existingGroup); }, diff --git a/core/events/events_block_change.ts b/core/events/events_block_change.ts index e9435e606ca..4c8ac534b34 100644 --- a/core/events/events_block_change.ts +++ b/core/events/events_block_change.ts @@ -15,6 +15,7 @@ import type {Block} from '../block.js'; import type {BlockSvg} from '../block_svg.js'; import {IconType} from '../icons/icon_types.js'; import {hasBubble} from '../interfaces/i_has_bubble.js'; +import {MANUALLY_DISABLED} from '../constants.js'; import * as registry from '../registry.js'; import * as utilsXml from '../utils/xml.js'; import {Workspace} from '../workspace.js'; @@ -35,10 +36,7 @@ export class BlockChange extends BlockBase { */ element?: string; - /** - * The name of the field that changed, if this is a change to a field, or - * the identifier of the reason why a block can be disabled. - */ + /** The name of the field that changed, if this is a change to a field. */ name?: string; /** The original value of the element. */ @@ -47,6 +45,12 @@ export class BlockChange extends BlockBase { /** The new value of the element. */ newValue: unknown; + /** + * If element is 'disabled', this is the language-neutral identifier of the + * reason why the block was or was not disabled. + */ + disabledReason?: string; + /** * @param opt_block The changed block. Undefined for a blank event. * @param opt_element One of 'field', 'comment', 'disabled', etc. @@ -89,6 +93,9 @@ export class BlockChange extends BlockBase { json['name'] = this.name; json['oldValue'] = this.oldValue; json['newValue'] = this.newValue; + if (this.disabledReason) { + json['disabledReason'] = this.disabledReason; + } return json; } @@ -115,9 +122,30 @@ export class BlockChange extends BlockBase { newEvent.name = json['name']; newEvent.oldValue = json['oldValue']; newEvent.newValue = json['newValue']; + if (json['disabledReason'] !== undefined) { + newEvent.disabledReason = json['disabledReason']; + } return newEvent; } + /** + * Set the language-neutral identifier for the reason why the block was or was + * not disabled. This is only valid for events where element is 'disabled'. + * Defaults to 'MANUALLY_DISABLED'. + * + * @param disabledReason The identifier of the reason why the block was or was + * not disabled. + */ + setDisabledReason(disabledReason: string) { + if (this.element !== 'disabled') { + throw new Error( + 'Cannot set the disabled reason for a BlockChange event if the ' + + 'element is not "disabled".', + ); + } + this.disabledReason = disabledReason; + } + /** * Does this event record any change of state? * @@ -171,7 +199,10 @@ export class BlockChange extends BlockBase { block.setCollapsed(!!value); break; case 'disabled': - block.setDisabledReason(!!value, this.name!); + block.setDisabledReason( + !!value, + this.disabledReason ?? MANUALLY_DISABLED, + ); break; case 'inline': block.setInputsInline(!!value); @@ -222,6 +253,7 @@ export interface BlockChangeJson extends BlockBaseJson { name?: string; newValue: unknown; oldValue: unknown; + disabledReason?: string; } registry.register(registry.Type.EVENT, eventUtils.CHANGE, BlockChange); diff --git a/core/flyout_base.ts b/core/flyout_base.ts index ce1959a377f..92711282423 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -22,6 +22,7 @@ import * as eventUtils from './events/utils.js'; import {FlyoutButton} from './flyout_button.js'; import {FlyoutMetricsManager} from './flyout_metrics_manager.js'; import type {IFlyout} from './interfaces/i_flyout.js'; +import {MANUALLY_DISABLED} from './constants.js'; import type {Options} from './options.js'; import {ScrollbarPair} from './scrollbar_pair.js'; import * as blocks from './serialization/blocks.js'; @@ -844,6 +845,12 @@ export abstract class Flyout blockInfo['enabled'] = blockInfo['disabled'] !== 'true' && blockInfo['disabled'] !== true; } + if ( + blockInfo['disabledReasons'] === undefined && + blockInfo['enabled'] === false + ) { + blockInfo['disabledReasons'] = [MANUALLY_DISABLED]; + } block = blocks.appendInternal( blockInfo as blocks.State, this.workspace_, diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index 7a59ca0bc36..be596096568 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -9,7 +9,7 @@ import type {Block} from '../block.js'; import type {BlockSvg} from '../block_svg.js'; import type {Connection} from '../connection.js'; -import * as constants from '../constants.js'; +import {MANUALLY_DISABLED} from '../constants.js'; import * as deprecation from '../utils/deprecation.js'; import * as eventUtils from '../events/utils.js'; import {inputTypes} from '../inputs/input_types.js'; @@ -527,9 +527,9 @@ function loadAttributes(block: Block, state: State) { 'enabled', 'v11', 'v12', - 'disabledReasons with the value ["' + constants.MANUALLY_DISABLED + '"]', + 'disabledReasons with the value ["' + MANUALLY_DISABLED + '"]', ); - block.setDisabledReason(true, constants.MANUALLY_DISABLED); + block.setDisabledReason(true, MANUALLY_DISABLED); } if (Array.isArray(state['disabledReasons'])) { for (const reason of state['disabledReasons']) { diff --git a/core/utils/toolbox.ts b/core/utils/toolbox.ts index 17fd327c67a..fbf031ac612 100644 --- a/core/utils/toolbox.ts +++ b/core/utils/toolbox.ts @@ -21,6 +21,7 @@ export interface BlockInfo { type?: string; gap?: string | number; disabled?: string | boolean; + disabledReasons?: string[]; enabled?: boolean; id?: string; x?: number; diff --git a/core/xml.ts b/core/xml.ts index 3f90b7390d8..686c7108263 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -9,7 +9,7 @@ import type {Block} from './block.js'; import type {BlockSvg} from './block_svg.js'; import type {Connection} from './connection.js'; -import * as constants from './constants.js'; +import {MANUALLY_DISABLED} from './constants.js'; import * as deprecation from './utils/deprecation.js'; import * as eventUtils from './events/utils.js'; import type {Field} from './field.js'; @@ -1027,11 +1027,11 @@ function domToBlockHeadless( 'disabled', 'v11', 'v12', - 'disabled-reasons with the value "' + constants.MANUALLY_DISABLED + '"', + 'disabled-reasons with the value "' + MANUALLY_DISABLED + '"', ); block.setDisabledReason( disabled === 'true' || disabled === 'disabled', - constants.MANUALLY_DISABLED, + MANUALLY_DISABLED, ); } const disabledReasons = xmlBlock.getAttribute('disabled-reasons'); From 9e5dfa8fb517117c08176017652a8bab4518fe99 Mon Sep 17 00:00:00 2001 From: John Nesky Date: Wed, 17 Apr 2024 19:42:41 -0700 Subject: [PATCH 9/9] Make BlockChange.disabledReason private. --- core/events/events_block_change.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/events/events_block_change.ts b/core/events/events_block_change.ts index 4c8ac534b34..570b7f58e2c 100644 --- a/core/events/events_block_change.ts +++ b/core/events/events_block_change.ts @@ -49,7 +49,7 @@ export class BlockChange extends BlockBase { * If element is 'disabled', this is the language-neutral identifier of the * reason why the block was or was not disabled. */ - disabledReason?: string; + private disabledReason?: string; /** * @param opt_block The changed block. Undefined for a blank event.