From d754c6d2786a4dba2eb5de5d52f54f65ec23fa87 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 9 Sep 2022 16:29:06 -0700 Subject: [PATCH] chore: remove AnyDuringMigration from gesture code (#6401) * chore: work on refactoring gesture code * chore: reorganize throw order * chore: format * chore: PR comments * chore: format --- core/gesture.ts | 173 ++++++++++++++++++------------------ core/rendered_connection.ts | 16 ++-- core/touch_gesture.ts | 14 ++- 3 files changed, 108 insertions(+), 95 deletions(-) diff --git a/core/gesture.ts b/core/gesture.ts index 9ec0d17fd..295d30721 100644 --- a/core/gesture.ts +++ b/core/gesture.ts @@ -53,33 +53,26 @@ export class Gesture { * pixels, with (0, 0) at the top left of the browser window (mouseEvent * clientX/Y). */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Coordinate'. - private mouseDownXY_: Coordinate = null as AnyDuringMigration; + private mouseDownXY_ = new Coordinate(0, 0); private currentDragDeltaXY_: Coordinate; /** * The bubble that the gesture started on, or null if it did not start on a * bubble. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'IBubble'. - private startBubble_: IBubble = null as AnyDuringMigration; + private startBubble_: IBubble|null = null; /** * The field that the gesture started on, or null if it did not start on a * field. */ - // AnyDuringMigration because: Type 'null' is not assignable to type 'Field'. - private startField_: Field = null as AnyDuringMigration; + private startField_: Field|null = null; /** * The block that the gesture started on, or null if it did not start on a * block. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'BlockSvg'. - private startBlock_: BlockSvg = null as AnyDuringMigration; + private startBlock_: BlockSvg|null = null; /** * The block that this gesture targets. If the gesture started on a @@ -87,18 +80,14 @@ export class Gesture { * gesture started in the flyout, this is the root block of the block group * that was clicked or dragged. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'BlockSvg'. - private targetBlock_: BlockSvg = null as AnyDuringMigration; + private targetBlock_: BlockSvg|null = null; /** * The workspace that the gesture started on. There may be multiple * workspaces on a page; this is more accurate than using * Blockly.common.getMainWorkspace(). */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'WorkspaceSvg'. - protected startWorkspace_: WorkspaceSvg = null as AnyDuringMigration; + protected startWorkspace_: WorkspaceSvg|null = null; /** * Whether the pointer has at any point moved out of the drag radius. @@ -107,15 +96,6 @@ export class Gesture { */ private hasExceededDragRadius_ = false; - /** Whether the workspace is currently being dragged. */ - private isDraggingWorkspace_ = false; - - /** Whether the block is currently being dragged. */ - private isDraggingBlock_ = false; - - /** Whether the bubble is currently being dragged. */ - private isDraggingBubble_ = false; - /** * A handle to use to unbind a mouse move listener at the end of a drag. * Opaque data returned from Blockly.bindEventWithChecks_. @@ -129,9 +109,7 @@ export class Gesture { protected onUpWrapper_: browserEvents.Data|null = null; /** The object tracking a bubble drag, or null if none is in progress. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'BubbleDragger'. - private bubbleDragger_: BubbleDragger = null as AnyDuringMigration; + private bubbleDragger_: BubbleDragger|null = null; /** The object tracking a block drag, or null if none is in progress. */ private blockDragger_: IBlockDragger|null = null; @@ -140,14 +118,10 @@ export class Gesture { * The object tracking a workspace or flyout workspace drag, or null if none * is in progress. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'WorkspaceDragger'. - private workspaceDragger_: WorkspaceDragger = null as AnyDuringMigration; + private workspaceDragger_: WorkspaceDragger|null = null; /** The flyout a gesture started in, if any. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'IFlyout'. - private flyout_: IFlyout = null as AnyDuringMigration; + private flyout_: IFlyout|null = null; /** Boolean for sanity-checking that some code is only called once. */ private calledUpdateIsDragging_ = false; @@ -265,17 +239,17 @@ export class Gesture { * @returns True if a block is being dragged from the flyout. */ private updateIsDraggingFromFlyout_(): boolean { - if (!this.targetBlock_) { + if (!this.targetBlock_ || + !this.flyout_?.isBlockCreatable(this.targetBlock_)) { return false; } - if (!this.flyout_.isBlockCreatable(this.targetBlock_)) { - return false; + if (!this.flyout_.targetWorkspace) { + throw new Error(`Cannot update dragging from the flyout because the ' + + 'flyout's target workspace is undefined`); } if (!this.flyout_.isScrollable() || this.flyout_.isDragTowardWorkspace(this.currentDragDeltaXY_)) { - // AnyDuringMigration because: Type 'WorkspaceSvg | null' is not - // assignable to type 'WorkspaceSvg'. - this.startWorkspace_ = this.flyout_.targetWorkspace as AnyDuringMigration; + this.startWorkspace_ = this.flyout_.targetWorkspace; this.startWorkspace_.updateScreenCalculationsIfScrolled(); // Start the event group now, so that the same event group is used for // block creation and block dragging. @@ -283,9 +257,7 @@ export class Gesture { eventUtils.setGroup(true); } // The start block is no longer relevant, because this is a drag. - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'BlockSvg'. - this.startBlock_ = null as AnyDuringMigration; + this.startBlock_ = null; this.targetBlock_ = this.flyout_.createBlock(this.targetBlock_); this.targetBlock_.select(); return true; @@ -307,13 +279,15 @@ export class Gesture { return false; } - this.isDraggingBubble_ = true; this.startDraggingBubble_(); return true; } /** - * Update this gesture to record whether a block is being dragged. + * 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 mouse/touch move 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 @@ -326,14 +300,12 @@ export class Gesture { if (!this.targetBlock_) { return false; } - if (this.flyout_) { - this.isDraggingBlock_ = this.updateIsDraggingFromFlyout_(); + if (this.updateIsDraggingFromFlyout_()) { + this.startDraggingBlock_(); + return true; + } } else if (this.targetBlock_.isMovable()) { - this.isDraggingBlock_ = true; - } - - if (this.isDraggingBlock_) { this.startDraggingBlock_(); return true; } @@ -341,24 +313,28 @@ export class Gesture { } /** - * Update this gesture to record whether a workspace is being dragged. + * Check whether to start a workspace drag. If a workspace is being dragged, + * create the necessary WorkspaceDragger and start the drag. + * * This function should be called on a mouse/touch move event the first time * the drag radius is exceeded. It should be called no more than once per * gesture. If a workspace is being dragged this function creates the * necessary WorkspaceDragger and starts the drag. */ private updateIsDraggingWorkspace_() { + if (!this.startWorkspace_) { + throw new Error( + 'Cannot update dragging the workspace because the ' + + 'start workspace is undefined'); + } + const wsMovable = this.flyout_ ? this.flyout_.isScrollable() : this.startWorkspace_ && this.startWorkspace_.isDraggable(); + if (!wsMovable) return; - if (!wsMovable) { - return; - } + this.workspaceDragger_ = new WorkspaceDragger(this.startWorkspace_); - this.workspaceDragger_ = new WorkspaceDragger((this.startWorkspace_)); - - this.isDraggingWorkspace_ = true; this.workspaceDragger_.startDrag(); } @@ -402,8 +378,19 @@ export class Gesture { // TODO (fenichel): Possibly combine this and startDraggingBlock_. /** Create a bubble dragger and start dragging the selected bubble. */ private startDraggingBubble_() { + 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'); + } + this.bubbleDragger_ = - new BubbleDragger((this.startBubble_), (this.startWorkspace_)); + new BubbleDragger(this.startBubble_, this.startWorkspace_); this.bubbleDragger_.startBubbleDrag(); this.bubbleDragger_.dragBubble( this.mostRecentEvent_, this.currentDragDeltaXY_); @@ -421,9 +408,17 @@ export class Gesture { this.cancel(); return; } + + if (!this.startWorkspace_) { + throw new Error( + 'Cannot start the gesture because the start ' + + 'workspace is undefined'); + } + this.hasStarted_ = true; blockAnimations.disconnectUiStop(); + this.startWorkspace_.updateScreenCalculationsIfScrolled(); if (this.startWorkspace_.isMutator) { // Mutator's coordinate system could be out of date because the bubble was @@ -495,11 +490,11 @@ export class Gesture { */ handleMove(e: Event) { this.updateFromEvent_(e); - if (this.isDraggingWorkspace_) { + if (this.workspaceDragger_) { this.workspaceDragger_.drag(this.currentDragDeltaXY_); - } else if (this.isDraggingBlock_) { - this.blockDragger_!.drag(this.mostRecentEvent_, this.currentDragDeltaXY_); - } else if (this.isDraggingBubble_) { + } else if (this.blockDragger_) { + this.blockDragger_.drag(this.mostRecentEvent_, this.currentDragDeltaXY_); + } else if (this.bubbleDragger_) { this.bubbleDragger_.dragBubble( this.mostRecentEvent_, this.currentDragDeltaXY_); } @@ -527,11 +522,11 @@ export class Gesture { // priority than workspaces. // The ordering within drags does not matter, because the three types of // dragging are exclusive. - if (this.isDraggingBubble_) { + if (this.bubbleDragger_) { this.bubbleDragger_.endBubbleDrag(e, this.currentDragDeltaXY_); - } else if (this.isDraggingBlock_) { - this.blockDragger_!.endDrag(e, this.currentDragDeltaXY_); - } else if (this.isDraggingWorkspace_) { + } else if (this.blockDragger_) { + this.blockDragger_.endDrag(e, this.currentDragDeltaXY_); + } else if (this.workspaceDragger_) { this.workspaceDragger_.endDrag(this.currentDragDeltaXY_); } else if (this.isBubbleClick_()) { // Bubbles are in front of all fields and blocks. @@ -564,13 +559,13 @@ export class Gesture { return; } Touch.longStop(); - if (this.isDraggingBubble_) { + if (this.bubbleDragger_) { this.bubbleDragger_.endBubbleDrag( this.mostRecentEvent_, this.currentDragDeltaXY_); - } else if (this.isDraggingBlock_) { - this.blockDragger_!.endDrag( + } else if (this.blockDragger_) { + this.blockDragger_.endDrag( this.mostRecentEvent_, this.currentDragDeltaXY_); - } else if (this.isDraggingWorkspace_) { + } else if (this.workspaceDragger_) { this.workspaceDragger_.endDrag(this.currentDragDeltaXY_); } this.dispose(); @@ -695,6 +690,11 @@ export class Gesture { /** Execute a field click. */ private doFieldClick_() { + if (!this.startField_) { + throw new Error( + 'Cannot do a field click because the start field is ' + + 'undefined'); + } this.startField_.showEditor(this.mostRecentEvent_); this.bringBlockToFront_(); } @@ -703,6 +703,11 @@ export class Gesture { private doBlockClick_() { // Block click in an autoclosing flyout. if (this.flyout_ && this.flyout_.autoClose) { + if (!this.targetBlock_) { + throw new Error( + 'Cannot do a block click because the target block is ' + + 'undefined'); + } if (this.targetBlock_.isEnabled()) { if (!eventUtils.getGroup()) { eventUtils.setGroup(true); @@ -711,6 +716,11 @@ export class Gesture { newBlock.scheduleSnapAndBump(); } } else { + if (!this.startWorkspace_) { + throw new Error( + 'Cannot do a block click because the start workspace ' + + 'is undefined'); + } // Clicks events are on the start block, even if it was a shadow. const event = new (eventUtils.get(eventUtils.CLICK))( this.startBlock_, this.startWorkspace_.id, 'block'); @@ -809,9 +819,9 @@ export class Gesture { */ private setTargetBlock_(block: BlockSvg) { if (block.isShadow()) { - // AnyDuringMigration because: Argument of type 'BlockSvg | null' is not - // assignable to parameter of type 'BlockSvg'. - this.setTargetBlock_(block.getParent() as AnyDuringMigration); + // Non-null assertion is fine b/c it is an invariant that shadows always + // have parents. + this.setTargetBlock_(block.getParent()!); } else { this.targetBlock_ = block; } @@ -906,8 +916,8 @@ export class Gesture { * @internal */ isDragging(): boolean { - return this.isDraggingWorkspace_ || this.isDraggingBlock_ || - this.isDraggingBubble_; + return !!this.workspaceDragger_ || !!this.blockDragger_ || + !!this.bubbleDragger_; } /** @@ -944,14 +954,7 @@ export class Gesture { * progress. */ getCurrentDragger(): WorkspaceDragger|BubbleDragger|IBlockDragger|null { - if (this.isDraggingBlock_) { - return this.blockDragger_; - } else if (this.isDraggingWorkspace_) { - return this.workspaceDragger_; - } else if (this.isDraggingBubble_) { - return this.bubbleDragger_; - } - return null; + return this.blockDragger_ ?? this.workspaceDragger_ ?? this.bubbleDragger_; } /** diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 049cfe966..d51a97a38 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -53,7 +53,7 @@ export class RenderedConnection extends Connection { private readonly dbOpposite_: ConnectionDB; private readonly offsetInBlock_: Coordinate; private trackedState_: TrackedState; - private highlightPath: SVGPathElement | null = null; + private highlightPath: SVGPathElement|null = null; /** Connection this connection connects to. Null if not connected. */ override targetConnection: RenderedConnection|null = null; @@ -304,13 +304,13 @@ export class RenderedConnection extends Connection { const x = this.x - xy.x; const y = this.y - xy.y; this.highlightPath = dom.createSvgElement( - Svg.PATH, { - 'class': 'blocklyHighlightedConnectionPath', - 'd': steps, - 'transform': 'translate(' + x + ',' + y + ')' + - (this.sourceBlock_.RTL ? ' scale(-1 1)' : ''), - }, - this.sourceBlock_.getSvgRoot()); + Svg.PATH, { + 'class': 'blocklyHighlightedConnectionPath', + 'd': steps, + 'transform': 'translate(' + x + ',' + y + ')' + + (this.sourceBlock_.RTL ? ' scale(-1 1)' : ''), + }, + this.sourceBlock_.getSvgRoot()); } /** Remove the highlighting around this connection. */ diff --git a/core/touch_gesture.ts b/core/touch_gesture.ts index 4b2f94d05..4fea2fdbd 100644 --- a/core/touch_gesture.ts +++ b/core/touch_gesture.ts @@ -61,8 +61,8 @@ export class TouchGesture extends Gesture { /** Boolean for whether or not the workspace supports pinch-zoom. */ private isPinchZoomEnabled_: boolean|null = null; - override onMoveWrapper_: AnyDuringMigration; - override onUpWrapper_: AnyDuringMigration; + override onMoveWrapper_: browserEvents.Data|null = null; + override onUpWrapper_: browserEvents.Data|null = null; /** * Start a gesture: update the workspace to indicate that a gesture is in @@ -72,6 +72,11 @@ export class TouchGesture extends Gesture { * @internal */ override doStart(e: MouseEvent) { + if (!this.startWorkspace_) { + throw new Error( + 'Cannot start the touch event becauase the start ' + + 'workspace is undefined'); + } this.isPinchZoomEnabled_ = this.startWorkspace_.options.zoomOptions && this.startWorkspace_.options.zoomOptions.pinch; super.doStart(e); @@ -254,6 +259,11 @@ export class TouchGesture extends Gesture { const gestureScale = scale - this.previousScale_; const delta = gestureScale > 0 ? gestureScale * ZOOM_IN_MULTIPLIER : gestureScale * ZOOM_OUT_MULTIPLIER; + if (!this.startWorkspace_) { + throw new Error( + 'Cannot handle a pinch because the start workspace ' + + 'is undefined'); + } const workspace = this.startWorkspace_; const position = browserEvents.mouseToSvg( e, workspace.getParentSvg(), workspace.getInverseScreenCTM());