diff --git a/blocks/loops.ts b/blocks/loops.ts index 02d9d34be72..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. @@ -365,7 +370,11 @@ 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(); @@ -376,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.setEnabled(enabled); + this.setDisabledReason( + !enabled, + CONTROL_FLOW_NOT_IN_LOOP_DISABLED_REASON, + ); Events.setGroup(group); } }, diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 546f93606c8..32139264429 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -754,7 +754,6 @@ interface CallMixin extends CallMixinType { defType_: string; quarkIds_: string[] | null; quarkConnections_: {[id: string]: Connection}; - previousEnabledState_: boolean; } type CallMixinType = typeof PROCEDURE_CALL_COMMON; @@ -764,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,12 +1130,16 @@ const PROCEDURE_CALL_COMMON = { ); } Events.setGroup(event.group); - if (blockChangeEvent.newValue) { - this.previousEnabledState_ = this.isEnabled(); - this.setEnabled(false); - } else { - this.setEnabled(this.previousEnabledState_); - } + const valid = def.isEnabled(); + this.setDisabledReason( + !valid, + DISABLED_PROCEDURE_DEFINITION_DISABLED_REASON, + ); + this.setWarningText( + valid + ? null + : Msg['PROCEDURES_CALL_DISABLED_DEF_WARNING'].replace('%1', name), + ); Events.setGroup(oldGroup); } } @@ -1181,7 +1191,6 @@ blocks['procedures_callnoreturn'] = { this.argumentVarModels_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; - this.previousEnabledState_ = true; }, defType_: 'procedures_defnoreturn', @@ -1202,7 +1211,6 @@ blocks['procedures_callreturn'] = { this.argumentVarModels_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; - this.previousEnabledState_ = true; }, defType_: 'procedures_defreturn', @@ -1219,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. @@ -1279,7 +1293,7 @@ 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. } @@ -1319,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.setEnabled(legal); + this.setDisabledReason(!legal, UNPARENTED_IFRETURN_DISABLED_REASON); Events.setGroup(group); } }, diff --git a/core/block.ts b/core/block.ts index ca1ebd82cb0..52191d63c3c 100644 --- a/core/block.ts +++ b/core/block.ts @@ -25,7 +25,9 @@ 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'; import * as Extensions from './extensions.js'; import type {Field} from './field.js'; @@ -166,7 +168,7 @@ export class Block implements IASTNodeLocation { inputList: Input[] = []; inputsInline?: boolean; icons: IIcon[] = []; - private disabled = false; + private disabledReasons = new Set(); tooltip: Tooltip.TipInfo = ''; contextMenu = true; @@ -1390,32 +1392,89 @@ export class Block implements IASTNodeLocation { } /** - * Get whether this block is enabled or not. + * 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; + } + + /** @deprecated v11 - Get whether the block is manually disabled. */ + private get disabled(): boolean { + deprecation.warn( + 'disabled', + 'v11', + 'v12', + 'the isEnabled or hasDisabledReason methods of Block', + ); + return this.hasDisabledReason(constants.MANUALLY_DISABLED); + } + + /** @deprecated v11 - Set whether the block is manually disabled. */ + private set disabled(value: boolean) { + deprecation.warn( + 'disabled', + 'v11', + 'v12', + 'the setDisabledReason method of Block', + ); + this.setDisabledReason(value, constants.MANUALLY_DISABLED); } /** - * 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. */ setEnabled(enabled: boolean) { - if (this.isEnabled() !== enabled) { - const oldValue = this.disabled; - this.disabled = !enabled; - eventUtils.fire( - new (eventUtils.get(eventUtils.BLOCK_CHANGE))( - this, - 'disabled', - null, - oldValue, - !enabled, - ), - ); + deprecation.warn( + 'setEnabled', + 'v11', + 'v12', + 'the setDisabledReason method of Block', + ); + 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); + } + const blockChangeEvent = new (eventUtils.get(eventUtils.BLOCK_CHANGE))( + this, + 'disabled', + /* name= */ null, + /* oldValue= */ !disabled, + /* newValue= */ disabled, + ) as BlockChange; + blockChangeEvent.setDisabledReason(reason); + eventUtils.fire(blockChangeEvent); } } @@ -1428,7 +1487,7 @@ export class Block implements IASTNodeLocation { getInheritedDisabled(): boolean { let ancestor = this.getSurroundParent(); while (ancestor) { - if (ancestor.disabled) { + if (!ancestor.isEnabled()) { return true; } ancestor = ancestor.getSurroundParent(); @@ -1437,6 +1496,27 @@ export class Block implements IASTNodeLocation { return false; } + /** + * 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 disabled. + * @returns Whether the block is disabled for the provided reason. + */ + hasDisabledReason(reason: string): boolean { + return this.disabledReasons.has(reason); + } + + /** + * 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 disabled, if any. + */ + getDisabledReasons(): ReadonlySet { + return this.disabledReasons; + } + /** * Get whether the block is collapsed or not. * diff --git a/core/block_svg.ts b/core/block_svg.ts index 9fb809ade08..cc6ed3b0398 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'; @@ -985,16 +986,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.getInheritedDisabled()) { - this.updateDisabled(); - } + deprecation.warn( + 'setEnabled', + 'v11', + 'v12', + 'the setDisabledReason method of BlockSvg', + ); + 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 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. + */ + override setDisabledReason(disabled: boolean, reason: string): void { + const wasEnabled = this.isEnabled(); + super.setDisabledReason(disabled, reason); + if (this.isEnabled() !== wasEnabled && !this.getInheritedDisabled()) { + this.updateDisabled(); } } 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 a540a1341ce..8aa6c1e7212 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -13,6 +13,7 @@ import { RegistryItem, Scope, } from './contextmenu_registry.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'; @@ -455,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(MANUALLY_DISABLED) + ? Msg['ENABLE_BLOCK'] + : Msg['DISABLE_BLOCK']; }, preconditionFn(scope: Scope) { const block = scope.block; @@ -466,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(MANUALLY_DISABLED) ? 1 : 0); + + if (block!.getInheritedDisabled() || isDisabledForOtherReason) { return 'disabled'; } return 'enabled'; @@ -479,7 +487,10 @@ export function registerDisable() { if (!existingGroup) { eventUtils.setGroup(true); } - block!.setEnabled(!block!.isEnabled()); + block!.setDisabledReason( + !block!.hasDisabledReason(MANUALLY_DISABLED), + 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 3ef5c10850f..570b7f58e2c 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'; @@ -44,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. + */ + private disabledReason?: string; + /** * @param opt_block The changed block. Undefined for a blank event. * @param opt_element One of 'field', 'comment', 'disabled', etc. @@ -86,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; } @@ -112,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? * @@ -168,7 +199,10 @@ export class BlockChange extends BlockBase { block.setCollapsed(!!value); break; case 'disabled': - block.setEnabled(!value); + block.setDisabledReason( + !!value, + this.disabledReason ?? MANUALLY_DISABLED, + ); break; case 'inline': block.setInputsInline(!!value); @@ -219,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/events/utils.ts b/core/events/utils.ts index ae66f4a51fc..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. @@ -516,10 +522,8 @@ export function get( } /** - * Enable/disable a block 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. - * 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. */ @@ -542,17 +546,20 @@ export function disableOrphans(event: Abstract) { try { recordUndo = false; const parent = block.getParent(); - if (parent && parent.isEnabled()) { + if ( + parent && + !parent.hasDisabledReason(ORPHANED_BLOCK_DISABLED_REASON) + ) { const children = block.getDescendants(false); for (let i = 0, child; (child = children[i]); i++) { - child.setEnabled(true); + child.setDisabledReason(false, ORPHANED_BLOCK_DISABLED_REASON); } } else if ( (block.outputConnection || block.previousConnection) && !eventWorkspace.isDragging() ) { do { - block.setEnabled(false); + 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 58b74843359..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'; @@ -43,6 +44,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. */ @@ -837,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_, @@ -1239,7 +1253,10 @@ export abstract class Flyout common.getBlockTypeCounts(block), ); while (block) { - block.setEnabled(enable); + 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 229686a0de6..be596096568 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 {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'; import {isSerializable} from '../interfaces/i_serializable.js'; @@ -53,6 +55,7 @@ export interface State { movable?: boolean; editable?: boolean; enabled?: boolean; + disabledReasons?: string[]; inline?: boolean; data?: string; extraState?: AnyDuringMigration; @@ -158,7 +161,7 @@ function saveAttributes(block: Block, state: State) { state['collapsed'] = true; } if (!block.isEnabled()) { - state['enabled'] = false; + state['disabledReasons'] = Array.from(block.getDisabledReasons()); } if (!block.isOwnDeletable()) { state['deletable'] = false; @@ -520,7 +523,18 @@ function loadAttributes(block: Block, state: State) { block.setEditable(false); } if (state['enabled'] === false) { - block.setEnabled(false); + deprecation.warn( + 'enabled', + 'v11', + 'v12', + 'disabledReasons with the value ["' + MANUALLY_DISABLED + '"]', + ); + block.setDisabledReason(true, MANUALLY_DISABLED); + } + if (Array.isArray(state['disabledReasons'])) { + for (const reason of state['disabledReasons']) { + block.setDisabledReason(true, reason); + } } if (state['inline'] !== undefined) { block.setInputsInline(state['inline']); 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/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 7746fd1e47d..686c7108263 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 {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'; import {IconType} from './icons/icon_types.js'; @@ -272,7 +274,13 @@ export function blockToDom( element.setAttribute('collapsed', 'true'); } if (!block.isEnabled()) { - element.setAttribute('disabled', 'true'); + // 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( + 'disabled-reasons', + Array.from(block.getDisabledReasons()).map(encodeURIComponent).join(','), + ); } if (!block.isOwnDeletable()) { element.setAttribute('deletable', 'false'); @@ -1015,7 +1023,24 @@ function domToBlockHeadless( } const disabled = xmlBlock.getAttribute('disabled'); if (disabled) { - block.setEnabled(disabled !== 'true' && disabled !== 'disabled'); + deprecation.warn( + 'disabled', + 'v11', + 'v12', + 'disabled-reasons with the value "' + MANUALLY_DISABLED + '"', + ); + block.setDisabledReason( + disabled === 'true' || disabled === 'disabled', + MANUALLY_DISABLED, + ); + } + 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.setDisabledReason(true, decodeURIComponent(reason)); + } } const deletable = xmlBlock.getAttribute('deletable'); if (deletable) { diff --git a/demos/blockfactory/blocks.js b/demos/blockfactory/blocks.js index b93ae573945..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.getInheritedDisabled() && + 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.getInheritedDisabled() && + 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 148498360e4..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.getInheritedDisabled()) { + 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.getInheritedDisabled()) { + 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.getInheritedDisabled()) { + 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.getInheritedDisabled()) { + 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/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/mocha/block_test.js b/tests/mocha/block_test.js index 13514ef7443..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 @@ -2440,7 +2440,7 @@ 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( @@ -2450,9 +2450,9 @@ suite('Blocks', function () { } }); 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 diff --git a/tests/mocha/blocks/loops_test.js b/tests/mocha/blocks/loops_test.js new file mode 100644 index 00000000000..3bbfdac1084 --- /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.isEnabled(), + 'Expected the break block to be disabled', + ); + }); + + 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.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 f9d027c99f1..109d3b2d4eb 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -811,7 +811,7 @@ 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( @@ -821,16 +821,33 @@ suite('Procedures', function () { }, ); + 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.setDisabledReason(true, 'test reason'); + this.clock.runAll(); + + chai.assert.isFalse( + callBlock.isEnabled(), + 'Expected the caller block to be invalid', + ); + }, + ); + test( 'if a procedure definition is enabled, the procedure caller ' + 'is also enabled', 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( @@ -847,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( @@ -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.isEnabled(), + '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.isEnabled(), + '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/generator_test.js b/tests/mocha/generator_test.js index 9ac67b27ca3..3a2679dca81 100644 --- a/tests/mocha/generator_test.js +++ b/tests/mocha/generator_test.js @@ -92,7 +92,7 @@ suite('Generator', function () { return 'stack_block'; }; rowBlock.nextConnection.connect(stackBlock.previousConnection); - rowBlock.disabled = blockDisabled; + rowBlock.setDisabledReason(blockDisabled, 'test reason'); const code = generator.blockToCode(rowBlock, opt_thisOnly); delete generator.forBlock['stack_block']; @@ -115,11 +115,16 @@ 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, + /* opt_thisOnly = */ true, + 'row_block', + ); + this.blockToCodeTest( + generator, + /* blockDisabled = */ false, + /* opt_thisOnly = */ false, 'row_blockstack_block', 'thisOnly=false', ); @@ -132,11 +137,16 @@ 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, + /* opt_thisOnly = */ true, + '', + ); + this.blockToCodeTest( + generator, + /* blockDisabled = */ 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/index.html b/tests/mocha/index.html index 7b43146f57f..9c7b10cabe2 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -101,6 +101,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'; diff --git a/tests/mocha/jso_serialization_test.js b/tests/mocha/jso_serialization_test.js index 66b6f4c9784..72a74ad3d6a 100644 --- a/tests/mocha/jso_serialization_test.js +++ b/tests/mocha/jso_serialization_test.js @@ -86,19 +86,30 @@ 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.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 () { + 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.setEnabled(true); + block.setDisabledReason(true, 'test reason 1'); + block.setDisabledReason(true, 'test reason 2'); const jso = Blockly.serialization.blocks.save(block); - assertNoProperty(jso, 'enabled'); + assertProperty(jso, 'disabledReasons', [ + 'test reason 1', + 'test reason 2', + ]); }); }); diff --git a/tests/mocha/serializer_test.js b/tests/mocha/serializer_test.js index 290f7fa45e8..b10f48df515 100644 --- a/tests/mocha/serializer_test.js +++ b/tests/mocha/serializer_test.js @@ -81,7 +81,13 @@ Serializer.Attributes.Collapsed = new SerializerTestCase( Serializer.Attributes.Disabled = new SerializerTestCase( 'Disabled', '' + - '' + + '' + + '', +); +Serializer.Attributes.DisabledWithEncodedComma = new SerializerTestCase( + 'DisabledWithEncodedComma', + '' + + '' + '', ); Serializer.Attributes.NotDeletable = new SerializerTestCase( @@ -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,