diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 1457971d898..546f93606c8 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -38,6 +38,7 @@ import {config} from '../core/config.js'; import {defineBlocks} from '../core/common.js'; import '../core/icons/comment_icon.js'; import '../core/icons/warning_icon.js'; +import * as common from '../core/common.js'; /** A dictionary of the block definitions provided by this module. */ export const blocks: {[key: string]: BlockDefinition} = {}; @@ -1157,7 +1158,7 @@ const PROCEDURE_CALL_COMMON = { const def = Procedures.getDefinition(name, workspace); if (def) { (workspace as WorkspaceSvg).centerOnBlock(def.id); - (def as BlockSvg).select(); + common.setSelected(def as BlockSvg); } }, }); diff --git a/core/block_svg.ts b/core/block_svg.ts index c7ab9289e4a..c5f91f28300 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -241,37 +241,19 @@ export class BlockSvg return this.style.colourTertiary; } + // TODO: Before merging, is it better to just remove these or to leave them + // but with change behavior? Not sure what the better UX is. /** * Selects this block. Highlights the block visually and fires a select event * if the block is not already selected. */ select() { + // TODO: Before merging, file an issue to allow shadows to be selected. if (this.isShadow() && this.getParent()) { // Shadow blocks should not be selected. this.getParent()!.select(); return; } - if (common.getSelected() === this) { - return; - } - let oldId = null; - if (common.getSelected()) { - oldId = common.getSelected()!.id; - // Unselect any previously selected block. - eventUtils.disable(); - try { - common.getSelected()!.unselect(); - } finally { - eventUtils.enable(); - } - } - const event = new (eventUtils.get(eventUtils.SELECTED))( - oldId, - this.id, - this.workspace.id, - ); - eventUtils.fire(event); - common.setSelected(this); this.addSelect(); } @@ -280,17 +262,6 @@ export class BlockSvg * event if the block is currently selected. */ unselect() { - if (common.getSelected() !== this) { - return; - } - const event = new (eventUtils.get(eventUtils.SELECTED))( - this.id, - null, - this.workspace.id, - ); - event.workspaceId = this.workspace.id; - eventUtils.fire(event); - common.setSelected(null); this.removeSelect(); } diff --git a/core/bubbles/bubble.ts b/core/bubbles/bubble.ts index 28f12c4fa21..35b9e7dde0a 100644 --- a/core/bubbles/bubble.ts +++ b/core/bubbles/bubble.ts @@ -16,13 +16,16 @@ import {Rect} from '../utils/rect.js'; import {Size} from '../utils/size.js'; import {Svg} from '../utils/svg.js'; import {WorkspaceSvg} from '../workspace_svg.js'; +import * as common from '../common.js'; +import {ISelectable} from '../blockly.js'; +import * as idGenerator from '../utils/idgenerator.js'; /** * The abstract pop-up bubble class. This creates a UI that looks like a speech * bubble, where it has a "tail" that points to the block, and a "head" that * displays arbitrary svg elements. */ -export abstract class Bubble implements IBubble { +export abstract class Bubble implements IBubble, ISelectable { /** The width of the border around the bubble. */ static readonly BORDER_WIDTH = 6; @@ -50,6 +53,8 @@ export abstract class Bubble implements IBubble { /** Distance between arrow point and anchor point. */ static readonly ANCHOR_RADIUS = 8; + public id: string; + /** The SVG group containing all parts of the bubble. */ protected svgRoot: SVGGElement; @@ -89,10 +94,11 @@ export abstract class Bubble implements IBubble { * when automatically positioning. */ constructor( - protected readonly workspace: WorkspaceSvg, + public readonly workspace: WorkspaceSvg, protected anchor: Coordinate, protected ownerRect?: Rect, ) { + this.id = idGenerator.getNextUniqueId(); this.svgRoot = dom.createSvgElement( Svg.G, {'class': 'blocklyBubble'}, @@ -209,6 +215,7 @@ export abstract class Bubble implements IBubble { /** Passes the pointer event off to the gesture system. */ private onMouseDown(e: PointerEvent) { this.workspace.getGesture(e)?.handleBubbleStart(e, this); + common.setSelected(this); } /** Positions the bubble relative to its anchor. Does not render its tail. */ @@ -640,4 +647,12 @@ export abstract class Bubble implements IBubble { revertDrag(): void { this.dragStrategy.revertDrag(); } + + select(): void { + // Bubbles don't have any visual for being selected. + } + + unselect(): void { + // Bubbles don't have any visual for being selected. + } } diff --git a/core/bubbles/mini_workspace_bubble.ts b/core/bubbles/mini_workspace_bubble.ts index f459654fac8..74317d57bc1 100644 --- a/core/bubbles/mini_workspace_bubble.ts +++ b/core/bubbles/mini_workspace_bubble.ts @@ -47,7 +47,7 @@ export class MiniWorkspaceBubble extends Bubble { /** @internal */ constructor( workspaceOptions: BlocklyOptions, - protected readonly workspace: WorkspaceSvg, + public readonly workspace: WorkspaceSvg, protected anchor: Coordinate, protected ownerRect?: Rect, ) { diff --git a/core/bubbles/text_bubble.ts b/core/bubbles/text_bubble.ts index 6f50d303b0e..020ab4f2ec1 100644 --- a/core/bubbles/text_bubble.ts +++ b/core/bubbles/text_bubble.ts @@ -20,7 +20,7 @@ export class TextBubble extends Bubble { constructor( private text: string, - protected readonly workspace: WorkspaceSvg, + public readonly workspace: WorkspaceSvg, protected anchor: Coordinate, protected ownerRect?: Rect, ) { diff --git a/core/bubbles/textinput_bubble.ts b/core/bubbles/textinput_bubble.ts index 7a8a44bdf5a..a3c572faf9a 100644 --- a/core/bubbles/textinput_bubble.ts +++ b/core/bubbles/textinput_bubble.ts @@ -70,7 +70,7 @@ export class TextInputBubble extends Bubble { * when automatically positioning. */ constructor( - protected readonly workspace: WorkspaceSvg, + public readonly workspace: WorkspaceSvg, protected anchor: Coordinate, protected ownerRect?: Rect, ) { diff --git a/core/clipboard/block_paster.ts b/core/clipboard/block_paster.ts index 0bb707c3654..fefc9947dd2 100644 --- a/core/clipboard/block_paster.ts +++ b/core/clipboard/block_paster.ts @@ -13,6 +13,7 @@ import {Coordinate} from '../utils/coordinate.js'; import {WorkspaceSvg} from '../workspace_svg.js'; import * as eventUtils from '../events/utils.js'; import {config} from '../config.js'; +import * as common from '../common.js'; export class BlockPaster implements IPaster { static TYPE = 'block'; @@ -43,7 +44,7 @@ export class BlockPaster implements IPaster { if (eventUtils.isEnabled() && !block.isShadow()) { eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CREATE))(block)); } - block.select(); + common.setSelected(block); return block; } } diff --git a/core/comments/rendered_workspace_comment.ts b/core/comments/rendered_workspace_comment.ts index ee713d9ffd8..e7ad7e96789 100644 --- a/core/comments/rendered_workspace_comment.ts +++ b/core/comments/rendered_workspace_comment.ts @@ -16,10 +16,12 @@ import * as dom from '../utils/dom.js'; import {IDraggable} from '../interfaces/i_draggable.js'; import {CommentDragStrategy} from '../dragging/comment_drag_strategy.js'; import * as browserEvents from '../browser_events.js'; +import * as common from '../common.js'; +import {ISelectable} from '../interfaces/i_selectable.js'; export class RenderedWorkspaceComment extends WorkspaceComment - implements IBoundedElement, IRenderedElement, IDraggable + implements IBoundedElement, IRenderedElement, IDraggable, ISelectable { /** The class encompassing the svg elements making up the workspace comment. */ private view: CommentView; @@ -160,6 +162,7 @@ export class RenderedWorkspaceComment const gesture = this.workspace.getGesture(e); if (gesture) { gesture.handleCommentStart(e, this); + common.setSelected(this); } } @@ -187,4 +190,12 @@ export class RenderedWorkspaceComment revertDrag(): void { this.dragStrategy.revertDrag(); } + + select(): void { + // TODO: Before merging, file an issue to implement this. + } + + unselect(): void { + // TODO: Before merging, file an issue to implement this. + } } diff --git a/core/common.ts b/core/common.ts index 62592135038..fba960a5b5a 100644 --- a/core/common.ts +++ b/core/common.ts @@ -13,6 +13,7 @@ import {BlockDefinition, Blocks} from './blocks.js'; import type {Connection} from './connection.js'; import type {Workspace} from './workspace.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +import * as eventUtils from './events/utils.js'; /** Database of all workspaces. */ const WorkspaceDB_ = Object.create(null); @@ -105,7 +106,18 @@ export function getSelected(): ISelectable | null { * @internal */ export function setSelected(newSelection: ISelectable | null) { + if (selected === newSelection) return; + + const event = new (eventUtils.get(eventUtils.SELECTED))( + selected?.id ?? null, + newSelection?.id ?? null, + newSelection?.workspace.id ?? selected?.workspace.id ?? '', + ); + eventUtils.fire(event); + + selected?.unselect(); selected = newSelection; + selected?.select(); } /** diff --git a/core/contextmenu.ts b/core/contextmenu.ts index 78431c93f3a..d1d5f5e4aec 100644 --- a/core/contextmenu.ts +++ b/core/contextmenu.ts @@ -29,6 +29,7 @@ import * as WidgetDiv from './widgetdiv.js'; import {WorkspaceCommentSvg} from './workspace_comment_svg.js'; import type {WorkspaceSvg} from './workspace_svg.js'; import * as Xml from './xml.js'; +import * as common from './common.js'; /** * Which block is the context menu attached to? @@ -261,7 +262,7 @@ export function callbackFactory( if (eventUtils.isEnabled() && !newBlock.isShadow()) { eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CREATE))(newBlock)); } - newBlock.select(); + common.setSelected(newBlock); return newBlock; }; } @@ -374,7 +375,7 @@ export function workspaceCommentOption( if (ws.rendered) { comment.initSvg(); comment.render(); - comment.select(); + common.setSelected(comment); } } diff --git a/core/dragging/block_drag_strategy.ts b/core/dragging/block_drag_strategy.ts index 559a6080a0b..84afe2a138f 100644 --- a/core/dragging/block_drag_strategy.ts +++ b/core/dragging/block_drag_strategy.ts @@ -65,7 +65,10 @@ export class BlockDragStrategy implements IDragStrategy { this.block.isOwnMovable() && !this.block.isShadow() && !this.block.isDeadOrDying() && - !this.workspace.options.readOnly + !this.workspace.options.readOnly && + // We never drag blocks in the flyout, only create new blocks that are + // dragged. + !this.block.isInFlyout ); } diff --git a/core/gesture.ts b/core/gesture.ts index a6ca1552818..b644f7576c6 100644 --- a/core/gesture.ts +++ b/core/gesture.ts @@ -30,13 +30,12 @@ import * as internalConstants from './internal_constants.js'; import * as Tooltip from './tooltip.js'; import * as Touch from './touch.js'; import {Coordinate} from './utils/coordinate.js'; -import {WorkspaceCommentSvg} from './workspace_comment_svg.js'; import {WorkspaceDragger} from './workspace_dragger.js'; import type {WorkspaceSvg} from './workspace_svg.js'; import type {IIcon} from './interfaces/i_icon.js'; import {IDragger} from './interfaces/i_dragger.js'; import * as registry from './registry.js'; -import {IDraggable} from './interfaces/i_draggable.js'; +import {IDraggable, isDraggable} from './interfaces/i_draggable.js'; import {RenderedWorkspaceComment} from './comments.js'; /** @@ -298,71 +297,7 @@ export class Gesture { // The start block is no longer relevant, because this is a drag. this.startBlock = null; this.targetBlock = this.flyout.createBlock(this.targetBlock); - this.targetBlock.select(); - return true; - } - return false; - } - - /** - * Update this gesture to record whether a bubble is being dragged. - * This function should be called on a pointermove event the first time - * the drag radius is exceeded. It should be called no more than once per - * gesture. If a bubble should be dragged this function creates the necessary - * BubbleDragger and starts the drag. - * - * @returns True if a bubble is being dragged. - */ - private updateIsDraggingBubble(e: PointerEvent): boolean { - if (!this.startBubble) { - return false; - } - - this.startDraggingBubble(e); - return true; - } - - /** - * Update this gesture to record whether a comment is being dragged. - * This function should be called on a pointermove event the first time - * the drag radius is exceeded. It should be called no more than once per - * gesture. - * - * @returns True if a comment is being dragged. - */ - private updateIsDraggingComment(e: PointerEvent): boolean { - if (!this.startComment) { - return false; - } - - this.startDraggingComment(e); - return true; - } - - /** - * Check whether to start a block drag. If a block should be dragged, either - * from the flyout or in the workspace, create the necessary BlockDragger and - * start the drag. - * - * This function should be called on a pointermove event the first time - * the drag radius is exceeded. It should be called no more than once per - * gesture. If a block should be dragged, either from the flyout or in the - * workspace, this function creates the necessary BlockDragger and starts the - * drag. - * - * @returns True if a block is being dragged. - */ - private updateIsDraggingBlock(e: PointerEvent): boolean { - if (!this.targetBlock) { - return false; - } - if (this.flyout) { - if (this.updateIsDraggingFromFlyout()) { - this.startDraggingBlock(e); - return true; - } - } else if (this.targetBlock.isMovable()) { - this.startDraggingBlock(e); + common.setSelected(this.targetBlock); return true; } return false; @@ -403,76 +338,30 @@ export class Gesture { * gesture. */ private updateIsDragging(e: PointerEvent) { - // Sanity check. - if (this.calledUpdateIsDragging) { - throw Error('updateIsDragging_ should only be called once per gesture.'); - } - this.calledUpdateIsDragging = true; - - // First check if it was a bubble drag. Bubbles always sit on top of - // blocks. - if (this.updateIsDraggingBubble(e)) { - return; - } - // Then check if it was a block drag. - if (this.updateIsDraggingBlock(e)) { - return; - } - if (this.updateIsDraggingComment(e)) { - return; - } - // Then check if it's a workspace drag. - this.updateIsDraggingWorkspace(); - } - - /** Start dragging the selected block. */ - private startDraggingBlock(e: PointerEvent) { - this.dragging = true; - this.dragger = this.createDragger(this.targetBlock!, this.startWorkspace_!); - this.dragger.onDragStart(e); - this.dragger.onDrag(e, this.currentDragDeltaXY); - } - - /** Start dragging the selected bubble. */ - private startDraggingBubble(e: PointerEvent) { - if (!this.startBubble) { - throw new Error( - 'Cannot update dragging the bubble because the start ' + - 'bubble is undefined', - ); - } if (!this.startWorkspace_) { throw new Error( - 'Cannot update dragging the bubble because the start ' + - 'workspace is undefined', + 'Cannot update dragging because the start workspace is undefined', ); } - this.dragging = true; - this.dragger = this.createDragger(this.startBubble, this.startWorkspace_); - this.dragger.onDragStart(e); - this.dragger.onDrag(e, this.currentDragDeltaXY); - } - - /** Start dragging the selected comment. */ - private startDraggingComment(e: PointerEvent) { - if (!this.startComment) { - throw new Error( - 'Cannot update dragging the comment because the start ' + - 'comment is undefined', - ); - } - if (!this.startWorkspace_) { - throw new Error( - 'Cannot update dragging the comment because the start ' + - 'workspace is undefined', - ); + if (this.calledUpdateIsDragging) { + throw Error('updateIsDragging_ should only be called once per gesture.'); } + this.calledUpdateIsDragging = true; - this.dragging = true; - this.dragger = this.createDragger(this.startComment, this.startWorkspace_); - this.dragger.onDragStart(e); - this.dragger.onDrag(e, this.currentDragDeltaXY); + // If we drag a block out of the flyout, it updates `common.getSelected` + // to return the new block. + if (this.flyout) this.updateIsDraggingFromFlyout(); + + const selected = common.getSelected(); + if (selected && isDraggable(selected) && selected.isMovable()) { + this.dragging = true; + this.dragger = this.createDragger(selected, this.startWorkspace_); + this.dragger.onDragStart(e); + this.dragger.onDrag(e, this.currentDragDeltaXY); + } else { + this.updateIsDraggingWorkspace(); + } } private createDragger( @@ -532,10 +421,6 @@ export class Gesture { Tooltip.block(); - if (this.targetBlock) { - this.targetBlock.select(); - } - if (browserEvents.isRightButton(e)) { this.handleRightClick(e); return; @@ -873,6 +758,13 @@ export class Gesture { } this.setStartWorkspace(ws); this.mostRecentEvent = e; + + if (!this.startBlock && !this.startBubble && !this.startComment) { + // Selection determines what things start drags. So to drag the workspace, + // we need to deselect anything that was previously selected. + common.setSelected(null); + } + this.doStart(e); } @@ -964,13 +856,7 @@ export class Gesture { * modify only this code. */ /** Execute a bubble click. */ - private doBubbleClick() { - // TODO (#1673): Consistent handling of single clicks. - if (this.startBubble instanceof WorkspaceCommentSvg) { - this.startBubble.setFocus(); - this.startBubble.select(); - } - } + private doBubbleClick() {} /** Execute a field click. */ private doFieldClick() { @@ -1138,6 +1024,7 @@ export class Gesture { // If the gesture already went through a bubble, don't set the start block. if (!this.startBlock && !this.startBubble) { this.startBlock = block; + common.setSelected(this.startBlock); if (block.isInFlyout && block !== block.getRootBlock()) { this.setTargetBlock(block.getRootBlock()); } else { diff --git a/core/interfaces/i_selectable.ts b/core/interfaces/i_selectable.ts index 8090ad94a1b..7cf9ad98c6f 100644 --- a/core/interfaces/i_selectable.ts +++ b/core/interfaces/i_selectable.ts @@ -6,12 +6,16 @@ // Former goog.module ID: Blockly.ISelectable +import type {Workspace} from '../workspace.js'; + /** * The interface for an object that is selectable. */ export interface ISelectable { id: string; + workspace: Workspace; + /** Select this. Highlight it visually. */ select(): void; @@ -23,6 +27,7 @@ export interface ISelectable { export function isSelectable(obj: Object): obj is ISelectable { return ( typeof (obj as any).id === 'string' && + (obj as any).workspace !== undefined && (obj as any).select !== undefined && (obj as any).unselect !== undefined );