From 025d087e6a9e81dbc5987e17a7b3441168e64ee8 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 24 Jan 2020 07:49:31 -0800 Subject: [PATCH] fix: Fixed insertion preview logic. And fixed markers not handling typed statements. (#3526) * Added planned functions. * Added default getConnectionPreviewMethod method. * Moved show preview logic into their new functions. * Fixed default getConnectinPreviewMethod logic. * Reverted to lastConnectionInRow call (for output connections) for behavioral backwards-compat. * Added zelos logic. * Removed unused functions, and added docs. --- core/block_svg.js | 4 +- core/insertion_marker_manager.js | 225 ++++++++++++------------- core/renderers/common/i_path_object.js | 2 +- core/renderers/common/path_object.js | 2 +- core/renderers/common/renderer.js | 73 ++++++-- core/renderers/zelos/path_object.js | 2 +- core/renderers/zelos/renderer.js | 22 ++- 7 files changed, 191 insertions(+), 139 deletions(-) diff --git a/core/block_svg.js b/core/block_svg.js index d42f5018c..491d8ff44 100644 --- a/core/block_svg.js +++ b/core/block_svg.js @@ -1760,8 +1760,8 @@ Blockly.BlockSvg.prototype.getHeightWidth = function() { * @param {boolean} add True if highlighting should be added. * @package */ -Blockly.BlockSvg.prototype.highlightForReplacement = function(add) { - this.pathObject.updateReplacementHighlight(add); +Blockly.BlockSvg.prototype.fadeForReplacement = function(add) { + this.pathObject.updateReplacementFade(add); }; /** diff --git a/core/insertion_marker_manager.js b/core/insertion_marker_manager.js index 92860e643..8a5a5f6b9 100644 --- a/core/insertion_marker_manager.js +++ b/core/insertion_marker_manager.js @@ -116,20 +116,19 @@ Blockly.InsertionMarkerManager = function(block) { this.markerConnection_ = null; /** - * Whether we are currently highlighting the block (shadow or real) that would - * be replaced if the drag were released immediately. - * @type {boolean} - * @private - */ - this.highlightingBlock_ = false; - - /** - * The block that is being highlighted for replacement, or null. + * The block that currently has an input being highlighted, or null. * @type {Blockly.BlockSvg} * @private */ this.highlightedBlock_ = null; + /** + * The block being faded to indicate replacement, or null. + * @type {Blockly.BlockSvg} + * @private + */ + this.fadedBlock_ = null; + /** * The connections on the dragging blocks that are available to connect to * other blocks. This includes all open connections on the top block, as well @@ -141,6 +140,17 @@ Blockly.InsertionMarkerManager = function(block) { this.availableConnections_ = this.initAvailableConnections_(); }; +/** + * An enum describing different kinds of previews the InsertionMarkerManager + * could display. + * @enum {number} + */ +Blockly.InsertionMarkerManager.PREVIEW_TYPE = { + INSERTION_MARKER: 0, + INPUT_OUTLINE: 1, + REPLACEMENT_FADE: 2, +}; + /** * Sever all links from this object. * @package @@ -210,7 +220,7 @@ Blockly.InsertionMarkerManager.prototype.applyConnections = function() { }; /** - * Update highlighted connections based on the most recent move location. + * Update connections based on the most recent move location. * @param {!Blockly.utils.Coordinate} dxy Position relative to drag start, * in workspace units. * @param {?number} deleteArea One of {@link Blockly.DELETE_AREA_TRASH}, @@ -394,48 +404,6 @@ Blockly.InsertionMarkerManager.prototype.getStartRadius_ = function() { return Blockly.SNAP_RADIUS; }; -/** - * Whether ending the drag would replace a block or insert a block. - * @return {boolean} True if dropping the block immediately would replace - * another block. False if dropping the block immediately would result in - * the block being inserted in a block stack. - * @private - */ -Blockly.InsertionMarkerManager.prototype.shouldReplace_ = function() { - var closest = this.closestConnection_; - var local = this.localConnection_; - - // Dragging a block over an existing block in an input. - if (local.type == Blockly.OUTPUT_VALUE) { - // Insert the dragged block into the stack if possible. - if (closest && - this.workspace_.getRenderer() - .shouldInsertDraggedBlock(this.topBlock_, closest)) { - return false; // Insert. - } - // Otherwise replace the existing block and bump it out. - return true; // Replace. - } - - // Connecting to a statement input of c-block is an insertion, even if that - // c-block is terminal (e.g. forever). - if (local == local.getSourceBlock().getFirstStatementConnection()) { - return false; // Insert. - } - - // Dragging a terminal block over another (connected) terminal block will - // replace, not insert. - var isTerminalBlock = !this.topBlock_.nextConnection; - var isConnectedTerminal = isTerminalBlock && - local.type == Blockly.PREVIOUS_STATEMENT && closest.isConnected(); - if (isConnectedTerminal) { - return true; // Replace. - } - - // Otherwise it's an insertion. - return false; -}; - /** * Whether ending the drag would delete the block. * @param {!Object} candidate An object containing a local connection, a closest @@ -498,16 +466,28 @@ Blockly.InsertionMarkerManager.prototype.maybeShowPreview_ = function(candidate) * @private */ Blockly.InsertionMarkerManager.prototype.showPreview_ = function() { - if (this.shouldReplace_()) { - this.highlightBlock_(); - } else { // Should insert - this.connectMarker_(); + var closest = this.closestConnection_; + var renderer = this.workspace_.getRenderer(); + var method = renderer.getConnectionPreviewMethod( + /** @type {!Blockly.RenderedConnection} */ (closest), + /** @type {!Blockly.RenderedConnection} */ (this.localConnection_), + this.topBlock_); + + switch (method) { + case Blockly.InsertionMarkerManager.PREVIEW_TYPE.INPUT_OUTLINE: + this.showInsertionInputOutline_(); + break; + case Blockly.InsertionMarkerManager.PREVIEW_TYPE.INSERTION_MARKER: + this.showInsertionMarker_(); + break; + case Blockly.InsertionMarkerManager.PREVIEW_TYPE.REPLACEMENT_FADE: + this.showReplacementFade_(); + break; } - // Also highlight the actual connection, as a nod to previous behaviour. - if (this.closestConnection_ && this.closestConnection_.targetBlock() && - this.workspace_.getRenderer() - .shouldHighlightConnection(this.closestConnection_)) { - this.closestConnection_.highlight(); + + // Optionally highlight the actual connection, as a nod to previous behaviour. + if (closest && renderer.shouldHighlightConnection(closest)) { + closest.highlight(); } }; @@ -555,53 +535,57 @@ Blockly.InsertionMarkerManager.prototype.hidePreview_ = function() { .shouldHighlightConnection(this.closestConnection_)) { this.closestConnection_.unhighlight(); } - if (this.highlightingBlock_) { - this.unhighlightBlock_(); + if (this.fadedBlock_) { + this.hideReplacementFade_(); + } else if (this.highlightedBlock_) { + this.hideInsertionInputOutline_(); } else if (this.markerConnection_) { - this.disconnectMarker_(); + this.hideInsertionMarker_(); } }; /** - * Add highlighting showing which block will be replaced. + * Shows an insertion marker connected to the appropriate blocks (based on + * manager state). * @private */ -Blockly.InsertionMarkerManager.prototype.highlightBlock_ = function() { - var closest = this.closestConnection_; +Blockly.InsertionMarkerManager.prototype.showInsertionMarker_ = function() { var local = this.localConnection_; - if (closest.targetBlock()) { - this.highlightedBlock_ = closest.targetBlock(); - closest.targetBlock().highlightForReplacement(true); - } else if (local.type == Blockly.OUTPUT_VALUE) { - this.highlightedBlock_ = closest.getSourceBlock(); - closest.getSourceBlock().highlightShapeForInput(closest, true); - } - this.highlightingBlock_ = true; -}; - -/** - * Get rid of the highlighting marking the block that will be replaced. - * @private - */ -Blockly.InsertionMarkerManager.prototype.unhighlightBlock_ = function() { var closest = this.closestConnection_; - // If there's no block in place, but we're still connecting to a value input, - // then we must have been highlighting an input shape. - if (closest.type == Blockly.INPUT_VALUE && !closest.isConnected()) { - this.highlightedBlock_.highlightShapeForInput(closest, false); - } else { - this.highlightedBlock_.highlightForReplacement(false); + + var isLastInStack = this.lastOnStack_ && local == this.lastOnStack_; + var imBlock = isLastInStack ? this.lastMarker_ : this.firstMarker_; + var imConn = imBlock.getMatchingConnection(local.getSourceBlock(), local); + + if (imConn == this.markerConnection_) { + throw Error('Made it to showInsertionMarker_ even though the marker isn\'t ' + + 'changing'); } - this.highlightedBlock_ = null; - this.highlightingBlock_ = false; + + // Render disconnected from everything else so that we have a valid + // connection location. + imBlock.render(); + imBlock.rendered = true; + imBlock.getSvgRoot().setAttribute('visibility', 'visible'); + + if (imConn && closest) { + // Position so that the existing block doesn't move. + imBlock.positionNearConnection(imConn, closest); + } + if (closest) { + // Connect() also renders the insertion marker. + imConn.connect(closest); + } + + this.markerConnection_ = imConn; }; /** - * Disconnect the insertion marker block in a manner that returns the stack to - * original state. + * Disconnects and hides the current insertion marker. Should return the blocks + * to their original state. * @private */ -Blockly.InsertionMarkerManager.prototype.disconnectMarker_ = function() { +Blockly.InsertionMarkerManager.prototype.hideInsertionMarker_ = function() { if (!this.markerConnection_) { console.log('No insertion marker connection to disconnect'); return; @@ -649,38 +633,41 @@ Blockly.InsertionMarkerManager.prototype.disconnectMarker_ = function() { }; /** - * Add an insertion marker connected to the appropriate blocks. + * Shows an outline around the input the closest connection belongs to. * @private */ -Blockly.InsertionMarkerManager.prototype.connectMarker_ = function() { - var local = this.localConnection_; +Blockly.InsertionMarkerManager.prototype.showInsertionInputOutline_ = function() { var closest = this.closestConnection_; + this.highlightedBlock_ = closest.getSourceBlock(); + this.highlightedBlock_.highlightShapeForInput(closest, true); +}; - var isLastInStack = this.lastOnStack_ && local == this.lastOnStack_; - var imBlock = isLastInStack ? this.lastMarker_ : this.firstMarker_; - var imConn = imBlock.getMatchingConnection(local.getSourceBlock(), local); +/** + * Hides any visible input outlines. + * @private + */ +Blockly.InsertionMarkerManager.prototype.hideInsertionInputOutline_ = function() { + this.highlightedBlock_.highlightShapeForInput(this.closestConnection_, false); + this.highlightedBlock_ = null; +}; - if (imConn == this.markerConnection_) { - throw Error('Made it to connectMarker_ even though the marker isn\'t ' + - 'changing'); - } +/** + * Shows a replacement fade affect on the closest connection's target block + * (the block that is currently connected to it). + * @private + */ +Blockly.InsertionMarkerManager.prototype.showReplacementFade_ = function() { + this.fadedBlock_ = this.closestConnection_.targetBlock(); + this.fadedBlock_.fadeForReplacement(true); +}; - // Render disconnected from everything else so that we have a valid - // connection location. - imBlock.render(); - imBlock.rendered = true; - imBlock.getSvgRoot().setAttribute('visibility', 'visible'); - - if (imConn && closest) { - // Position so that the existing block doesn't move. - imBlock.positionNearConnection(imConn, closest); - } - if (closest) { - // Connect() also renders the insertion marker. - imConn.connect(closest); - } - - this.markerConnection_ = imConn; +/** + * Hides/Removes any visible fade affects. + * @private + */ +Blockly.InsertionMarkerManager.prototype.hideReplacementFade_ = function() { + this.fadedBlock_.fadeForReplacement(false); + this.fadedBlock_ = null; }; /** diff --git a/core/renderers/common/i_path_object.js b/core/renderers/common/i_path_object.js index 12454e41d..4680ca83a 100644 --- a/core/renderers/common/i_path_object.js +++ b/core/renderers/common/i_path_object.js @@ -127,4 +127,4 @@ Blockly.blockRendering.IPathObject.prototype.updateMovable; * @param {boolean} enable True if styling should be added. * @package */ -Blockly.blockRendering.IPathObject.prototype.updateReplacementHighlight; +Blockly.blockRendering.IPathObject.prototype.updateReplacementFade; diff --git a/core/renderers/common/path_object.js b/core/renderers/common/path_object.js index c52996638..bb926115d 100644 --- a/core/renderers/common/path_object.js +++ b/core/renderers/common/path_object.js @@ -268,7 +268,7 @@ Blockly.blockRendering.PathObject.prototype.updateMovable = function(enable) { * @param {boolean} enable True if styling should be added. * @package */ -Blockly.blockRendering.PathObject.prototype.updateReplacementHighlight = +Blockly.blockRendering.PathObject.prototype.updateReplacementFade = function(enable) { /* eslint-disable indent */ this.setClass_('blocklyReplaceable', enable); diff --git a/core/renderers/common/renderer.js b/core/renderers/common/renderer.js index 3ec261e2a..9c3df5bf5 100644 --- a/core/renderers/common/renderer.js +++ b/core/renderers/common/renderer.js @@ -29,6 +29,7 @@ goog.require('Blockly.blockRendering.Drawer'); goog.require('Blockly.blockRendering.IPathObject'); goog.require('Blockly.blockRendering.PathObject'); goog.require('Blockly.blockRendering.RenderInfo'); +goog.require('Blockly.InsertionMarkerManager'); goog.requireType('Blockly.blockRendering.Debug'); @@ -163,19 +164,69 @@ Blockly.blockRendering.Renderer.prototype.shouldHighlightConnection = }; /* eslint-enable indent */ /** - * Determine whether or not to insert a dragged block into a stack. - * @param {!Blockly.Block} block The target block. - * @param {!Blockly.Connection} conn The closest connection. - * @return {boolean} True if we should insert the dragged block into the stack. + * Checks if an orphaned block can connect to the "end" of the topBlock's + * block-clump. If the clump is a row the end is the last input. If the clump + * is a stack, the end is the last next connection. If the clump is neither, + * then this returns false. + * @param {!Blockly.BlockSvg} topBlock The top block of the block clump we want to try and + * connect to. + * @param {!Blockly.BlockSvg} orphanBlock The orphan block that wants to find + * a home. + * @param {number} localType The type of the connection being dragged. + * @return {boolean} Whether there is a home for the orphan or not. * @package */ -Blockly.blockRendering.Renderer.prototype.shouldInsertDraggedBlock = - function(block, conn) { - /* eslint-disable indent */ - return !conn.isConnected() || - !!Blockly.Connection.lastConnectionInRow(block, - conn.targetConnection.getSourceBlock()); -}; /* eslint-enable indent */ +Blockly.blockRendering.Renderer.prototype.orphanCanConnectAtEnd = + function(topBlock, orphanBlock, localType) { + var orphanConnection = null; + var lastConnection = null; + if (localType == Blockly.OUTPUT_VALUE) { // We are replacing an output. + orphanConnection = orphanBlock.outputConnection; + // TODO: I don't think this function necessarily has the correct logic, + // but for now it is being kept for behavioral backwards-compat. + lastConnection = Blockly.Connection + .lastConnectionInRow( + /** @type {!Blockly.Block} **/ (topBlock), orphanBlock); + } else { // We are replacing a previous. + orphanConnection = orphanBlock.previousConnection; + // TODO: This lives on the block while lastConnectionInRow lives on + // on the connection. Something is fishy. + lastConnection = topBlock.lastConnectionInStack(); + } + + if (!lastConnection) { + return false; + } + return orphanConnection.checkType(lastConnection); + }; + +/** + * Chooses a connection preview method based on the available connection, the + * current dragged connection, and the block being dragged. + * @param {!Blockly.RenderedConnection} closest The available connection. + * @param {!Blockly.RenderedConnection} local The connection currently being + * dragged. + * @param {!Blockly.BlockSvg} topBlock The block currently being dragged. + * @return {!Blockly.InsertionMarkerManager.PREVIEW_TYPE} The preview type + * to display. + * @package + */ +Blockly.blockRendering.Renderer.prototype.getConnectionPreviewMethod = + function(closest, local, topBlock) { + if (local.type == Blockly.OUTPUT_VALUE || + local.type == Blockly.PREVIOUS_STATEMENT) { + if (!closest.isConnected() || + this.orphanCanConnectAtEnd( + topBlock, + /** @type {!Blockly.BlockSvg} */ (closest.targetBlock()), + local.type)) { + return Blockly.InsertionMarkerManager.PREVIEW_TYPE.INSERTION_MARKER; + } + return Blockly.InsertionMarkerManager.PREVIEW_TYPE.REPLACEMENT_FADE; + } + + return Blockly.InsertionMarkerManager.PREVIEW_TYPE.INSERTION_MARKER; + }; /** * Render the block. diff --git a/core/renderers/zelos/path_object.js b/core/renderers/zelos/path_object.js index c3b64a1bb..4a84cb63a 100644 --- a/core/renderers/zelos/path_object.js +++ b/core/renderers/zelos/path_object.js @@ -149,7 +149,7 @@ Blockly.zelos.PathObject.prototype.updateSelected = function(enable) { /** * @override */ -Blockly.zelos.PathObject.prototype.updateReplacementHighlight = function( +Blockly.zelos.PathObject.prototype.updateReplacementFade = function( enable) { this.setClass_('blocklyReplaceable', enable); if (enable) { diff --git a/core/renderers/zelos/renderer.js b/core/renderers/zelos/renderer.js index 5f805baf8..9e33753c5 100644 --- a/core/renderers/zelos/renderer.js +++ b/core/renderers/zelos/renderer.js @@ -25,6 +25,7 @@ goog.provide('Blockly.zelos.Renderer'); goog.require('Blockly.blockRendering'); goog.require('Blockly.blockRendering.Renderer'); +goog.require('Blockly.InsertionMarkerManager'); goog.require('Blockly.utils.object'); goog.require('Blockly.zelos.ConstantProvider'); goog.require('Blockly.zelos.Drawer'); @@ -119,9 +120,22 @@ Blockly.zelos.Renderer.prototype.shouldHighlightConnection = function(conn) { /** * @override */ -Blockly.zelos.Renderer.prototype.shouldInsertDraggedBlock = function(_block, - _conn) { - return false; -}; +Blockly.zelos.Renderer.prototype.getConnectionPreviewMethod = + function(closest, local, topBlock) { + if (local.type == Blockly.OUTPUT_VALUE) { + if (!closest.isConnected()) { + return Blockly.InsertionMarkerManager.PREVIEW_TYPE.INPUT_OUTLINE; + } + // TODO: Returning this is a total hack, because we don't want to show + // a replacement fade, we want to show an outline affect. + // Sadly zelos does not support showing an outline around filled + // inputs, so we have to pretend like the connected block is getting + // replaced. + return Blockly.InsertionMarkerManager.PREVIEW_TYPE.REPLACEMENT_FADE; + } + + return Blockly.zelos.Renderer.superClass_ + .getConnectionPreviewMethod(closest, local, topBlock); + }; Blockly.blockRendering.register('zelos', Blockly.zelos.Renderer);