feat!: Invalid Blocks (#7958)

* feat: Invalid Blocks

* Rename the new json property from invalid to invalidReasons.

* Merged isValid into isEnabled.

* Minor fixes.

* More minor fixes.

* Reverting some stuff that didn't need to change.

* Addressing PR feedback.

* Update the BlockInfo interface to match State.

* Make BlockChange.disabledReason private.
This commit is contained in:
John Nesky
2024-04-17 19:47:51 -07:00
committed by GitHub
parent 7d8f88a4f1
commit cee7f916bb
26 changed files with 492 additions and 97 deletions

View File

@@ -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<string>();
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<string> {
return this.disabledReasons;
}
/**
* Get whether the block is collapsed or not.
*

View File

@@ -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();
}
}

View File

@@ -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';

View File

@@ -14,6 +14,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';
@@ -458,9 +459,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;
@@ -469,7 +470,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';
@@ -482,7 +490,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,

View File

@@ -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);

View File

@@ -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);
}

View File

@@ -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();
}
}

View File

@@ -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']);

View File

@@ -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'];

View File

@@ -21,6 +21,7 @@ export interface BlockInfo {
type?: string;
gap?: string | number;
disabled?: string | boolean;
disabledReasons?: string[];
enabled?: boolean;
id?: string;
x?: number;

View File

@@ -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) {