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.
2026-03-15 17:50:09 +01:00 · 2020-01-24 07:49:31 -08:00
parent 7ef5fe8cec
commit 025d087e6a
7 changed files with 191 additions and 139 deletions
--- 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);
 };

 /**
--- 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;
 };

 /**
--- 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;
--- 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);
--- 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.
--- 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) {
--- 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);