Fix not being able to modify collapsed blocks (#2074)

appinventor/blocklyeditor/ploverConfig.js

@@ -144,6 +144,8 @@
     "./src/warning.js",
     "./src/toolboxController.js",
     "./src/field.js",
+    "./src/rendered_connection.js",
+    "./src/input.js",
 
     // Dialog Utiltiy
     "./src/util.js",

appinventor/blocklyeditor/src/block.js

@@ -176,3 +176,46 @@ Blockly.Block.prototype.domToMutation = null;
  * @type {?function(this: Blockly.BlockSvg):!Element}
  */
 Blockly.Block.prototype.mutationToDom = null;
+
+/**
+ * Create a human-readable text representation of this block and any children.
+ * @param {number=} opt_maxLength Truncate the string to this length.
+ * @param {string=} opt_emptyToken The placeholder string used to denote an
+ *     empty field. If not specified, '?' is used.
+ * @return {string} Text of block.
+ */
+Blockly.Block.prototype.toString = function(opt_maxLength, opt_emptyToken) {
+  // This function is overridden so that it doesn't use the collapsed shortcut.
+  var text = '';
+  var emptyFieldPlaceholder = opt_emptyToken || '?';
+  for (var i = 0, input;
+       (input = this.inputList[i]) &&
+       // We always want to go over if possible so that it shows the ellipsis.
+       // +1 is to account for a trailing space.
+       (!opt_maxLength || text.length <= opt_maxLength + 1);
+       i++) {
+
+    if (input.name == Blockly.BlockSvg.COLLAPSED_INPUT_NAME) {
+      continue;
+    }
+    for (var j = 0, field; (field = input.fieldRow[j]); j++) {
+      text += field.getText() + ' ';
+    }
+    if (input.connection) {
+      var child = input.connection.targetBlock();
+      if (child) {
+        var charsLeft = opt_maxLength ? opt_maxLength - text.length : undefined;
+        text += child.toString(charsLeft, opt_emptyToken) + ' ';
+      } else {
+        text += emptyFieldPlaceholder + ' ';
+      }
+    }
+  }
+  text = goog.string.trim(text) || '???';
+  if (opt_maxLength) {
+    // TODO (Blockly): Improve truncation so that text from this block is
+    //  given priority.
+    text = goog.string.truncate(text, opt_maxLength);
+  }
+  return text;
+};

appinventor/blocklyeditor/src/block_svg.js

@@ -36,6 +36,25 @@ Blockly.BlockSvg.prototype.error = null;
  */
 Blockly.BlockSvg.prototype.isBad = false;
 
+/**
+ * This is to prevent any recursive render calls. It is not elegant in the
+ * least, but it is pretty future proof.
+ * @type {boolean}
+ */
+Blockly.BlockSvg.prototype.isRendering = false;
+
+/**
+ * The language-neutral id given to the collapsed input.
+ * @const {string}
+ */
+Blockly.BlockSvg.COLLAPSED_INPUT_NAME = '_TEMP_COLLAPSED_INPUT';
+
+/**
+ * The language-neutral id given to the collapsed field.
+ * @const {string}
+ */
+Blockly.BlockSvg.COLLAPSED_FIELD_NAME = '_TEMP_COLLAPSED_FIELD';
+
 /**
  * Returns a list of mutator, comment, and warning icons.
  * @return {!Array} List of icons.
@@ -209,56 +228,68 @@ Blockly.BlockSvg.prototype.render = (function(func) {
 if (Blockly.Instrument.useRenderDown) {
 
 Blockly.BlockSvg.prototype.render = function(opt_bubble) {
-  this.renderDown();
-
-  // Render all blocks above this one (propagate a reflow).
   if (opt_bubble !== false) {
-    if (this.parentBlock_) {
-      var top = this.parentBlock_;
-      while (top.parentBlock_) top = top.parentBlock_;
-      top.render(false);
-    } else {
+    var root = this.getRootBlock();
+    if (root != this) {  // If 'this' is not the top.
+      root.render(false);
+      return;
+    }
+  }
+  if (this.isRendering || !Blockly.BlockSvg.isRenderingOn) {
+    return;
+  }
+  goog.asserts.assertObject(this.svgGroup_,
+    ' Uninitialized block cannot be rendered. Call block.initSvg()');
+
+  this.isRendering = true;
+  try {  // Make sure we set isRendering back to false if something goes wrong.
+    this.renderDown_();
+    if (!this.getParent()) {  // This is a top level block.
+      // Fire an event so the scrollbars can resize.
       this.workspace.resizeContents();
     }
+  } finally {
+    this.isRendering = false;
   }
 };
 
 /**
  * [lyn, 04/01/14] Render a tree of blocks from top down rather than bottom up.
  * This is in contrast to render(), which renders a block and all its antecedents.
+ * @private Should only be called from render.
  */
-Blockly.BlockSvg.prototype.renderDown = function() {
-  if (Blockly.BlockSvg.isRenderingOn) {
-    goog.asserts.assertObject(this.svgGroup_,
-      ' Uninitialized block cannot be renderedDown.  Call block.initSvg()');
-    // Recursively renderDown all my children (as long as I'm not collapsed)
-    if (! (Blockly.Instrument.avoidRenderDownOnCollapsedSubblocks && this.isCollapsed())) {
-      var childBlocks = this.childBlocks_;
-      for (var c = 0, childBlock; childBlock = childBlocks[c]; c++) {
-        childBlock.renderDown();
-      }
-      this.renderHere();
-    } else {
-      // nextConnection is a "child" block, but needs to be rendered because it's not collapsed.
-      if (Blockly.Instrument.avoidRenderDownOnCollapsedSubblocks &&
-        this.nextConnection && this.nextConnection.targetBlock()) {
-        this.nextConnection.targetBlock().renderDown();
-      }
-      this.renderHere();
+Blockly.BlockSvg.prototype.renderDown_ = function() {
+  // Recursively renderDown all my children (as long as I'm not collapsed)
+  if (!this.isCollapsed() || !Blockly.Instrument
+      .avoidRenderDownOnCollapsedSubblocks) {
+    var childBlocks = this.getChildren();  // Includes next block.
+    for (var i = 0, childBlock; (childBlock = childBlocks[i]); i++) {
+      childBlock.render(false);
+    }
+  } else {  // Always render next block.
+    var nextBlock = this.getNextBlock();
+    if (nextBlock) {
+      nextBlock.render(false);
     }
-    Blockly.Instrument.stats.renderDownCalls++; //***lyn
   }
+  this.renderHere_();
+  Blockly.Instrument.stats.renderDownCalls++; //***lyn
   // [lyn, 04/08/14] Because renderDown is recursive, doesn't make sense to track its time here.
 };
 
 /**
  * Render this block. Assumes descendants have already been rendered.
+ * @private Should only be called from renderDown_.
  */
-Blockly.BlockSvg.prototype.renderHere = function(opt_bubble) {
+Blockly.BlockSvg.prototype.renderHere_ = function() {
   var start = new Date().getTime();
   Blockly.Field.startCache();
   this.rendered = true;
 
+  if (this.isCollapsed()) {
+    this.updateCollapsed_();
+  }
+
   // Now render me (even if I am collapsed, since still need to show collapsed block)
   var cursorX = Blockly.BlockSvg.SEP_SPACE_X;
   if (this.RTL) {
@@ -292,6 +323,44 @@ Blockly.BlockSvg.prototype.renderHere = function(opt_bubble) {
   Blockly.Instrument.stats.renderHereTime += timeDiff;
 };
 
+/**
+ * Makes sure that when the block is collapsed, it is rendered correctly for
+ * that state.
+ * @private
+ */
+Blockly.BlockSvg.prototype.updateCollapsed_ = function() {
+  var collapsed = this.isCollapsed();
+  var collapsedInputName = Blockly.BlockSvg.COLLAPSED_INPUT_NAME;
+  var collapsedFieldName = Blockly.BlockSvg.COLLAPSED_FIELD_NAME;
+
+  for (var i = 0, input; (input = this.inputList[i]); i++) {
+    if (input.name == collapsedInputName) {
+      continue;
+    }
+    input.setVisible(!collapsed);
+  }
+
+  if (!collapsed) {
+    this.removeInput(collapsedInputName);
+    return;
+  }
+
+  var icons = this.getIcons();
+  for (var i = 0, icon; (icon = icons[i]); i++) {
+    icon.setVisible(false);
+  }
+
+  var text = this.toString(Blockly.COLLAPSE_CHARS);
+  var field = this.getField(collapsedFieldName);
+  if (field) {
+    field.setValue(text);
+    return;
+  }
+  var input = this.getInput(collapsedInputName) ||
+      this.appendDummyInput(collapsedInputName);
+  input.appendField(new Blockly.FieldLabel(text), collapsedFieldName);
+};
+
   /**
    * Set whether the block is collapsed or not.
    * @param {boolean} collapsed True if collapsed.
@@ -300,36 +369,11 @@ Blockly.BlockSvg.prototype.renderHere = function(opt_bubble) {
     if (this.collapsed_ == collapsed) {
       return;
     }
-    var renderList = [];
-    // Show/hide the inputs.
-    for (var x = 0, input; input = this.inputList[x]; x++) {
-      // No need to collect renderList if rendering down.
-      input.setVisible(!collapsed);
-    }
-
-    var COLLAPSED_INPUT_NAME = '_TEMP_COLLAPSED_INPUT';
-    if (collapsed) {
-      var icons = this.getIcons();
-      for (var i = 0; i < icons.length; i++) {
-        icons[i].setVisible(false);
-      }
-      var text = this.toString(Blockly.COLLAPSE_CHARS);
-      this.appendDummyInput(COLLAPSED_INPUT_NAME).appendField(text).init();
-    } else {
-      this.removeInput(COLLAPSED_INPUT_NAME);
-      // Clear any warnings inherited from enclosed blocks.
-      this.setWarningText(null);
-    }
-    Blockly.BlockSvg.superClass_.setCollapsed.call(this, collapsed);
-
-    if (!renderList.length) {
-      // No child blocks, just render this block.
-      renderList[0] = this;
-    }
-    if (this.rendered) {
-      for (var i = 0, block; block = renderList[i]; i++) {
-        block.render();
-      }
+    this.collapsed_ = collapsed;
+    if (!collapsed) {
+      this.updateCollapsed_();
+    } else if (this.rendered) {
+      this.render();
     }
   };
 }

appinventor/blocklyeditor/src/blocks/components.js

@@ -408,7 +408,6 @@ Blockly.Blocks.component_event = {
     if (this.instanceName == oldname) {
       this.instanceName = newname;
       this.componentDropDown.setValue(this.instanceName);
-      Blockly.Blocks.Utilities.renameCollapsed(this, 0);
       return true;
     }
     return false;
@@ -792,7 +791,6 @@ Blockly.Blocks.component_method = {
       //var title = this.inputList[0].titleRow[0];
       //title.setText('call ' + this.instanceName + '.' + this.methodType.name);
       this.componentDropDown.setValue(this.instanceName);
-      Blockly.Blocks.Utilities.renameCollapsed(this, 0);
       return true;
     }
     return false;
@@ -1198,7 +1196,6 @@ Blockly.Blocks.component_set_get = {
       //var title = this.inputList[0].titleRow[0];
       //title.setText(this.instanceName + '.');
       this.componentDropDown.setValue(this.instanceName);
-      Blockly.Blocks.Utilities.renameCollapsed(this, 0);
       return true;
     }
     return false;
@@ -1350,7 +1347,6 @@ Blockly.Blocks.component_component_block = {
       //var title = this.inputList[0].titleRow[0];
       //title.setText(this.instanceName);
       this.componentDropDown.setValue(this.instanceName);
-      Blockly.Blocks.Utilities.renameCollapsed(this, 0);
       return true;
     }
     return false;

appinventor/blocklyeditor/src/blocks/lexical-variables.js

@@ -138,7 +138,6 @@ Blockly.Blocks['lexical_variable_get'] = {
     // console.log("Renaming lexical variable from " + oldName + " to " + newName);
     if (oldName === this.getFieldValue('VAR')) {
         this.setFieldValue(newName, 'VAR');
-        Blockly.Blocks.Utilities.renameCollapsed(this, 0);
     }
   },
   renameFree: function (freeSubstitution) {

appinventor/blocklyeditor/src/blocks/procedures.js

@@ -249,17 +249,9 @@ Blockly.Blocks['procedures_defnoreturn'] = {
       var newArguments = procDecl.arguments_;
       newArguments[paramIndex] = newParamName;
 
-      var procName = procDecl.getFieldValue('NAME');
-
       // 1. Change all callers so label reflects new name
       Blockly.Procedures.mutateCallers(procDecl);
 
-      var callers = Blockly.Procedures.getCallers(procName, procWorkspace);
-      for (var x = 0; x < callers.length; x++) {
-        var block = callers[x];
-        Blockly.Blocks.Utilities.renameCollapsed(block, 0);
-      }
-
       // 2. If there's an open mutator, change the name in the corresponding slot.
       if (procDecl.mutator && procDecl.mutator.rootBlock_) {
         // Iterate through mutatorarg param blocks and change name of one at paramIndex
@@ -694,7 +686,6 @@ Blockly.Blocks['procedures_callnoreturn'] = {
   renameProcedure: function(oldName, newName) {
     if (Blockly.Names.equals(oldName, this.getFieldValue('PROCNAME'))) {
       this.setFieldValue(newName, 'PROCNAME');
-      Blockly.Blocks.Utilities.renameCollapsed(this, 0);
     }
   },
   // [lyn, 10/27/13] Renamed "fromChange" parameter to "startTracking", because it should be true in any situation

appinventor/blocklyeditor/src/blocks/utilities.js

@@ -83,31 +83,5 @@ Blockly.Blocks.Utilities.wrapSentence = function(str, len) {
 
 Blockly.Blocks.Utilities.MAX_COLLAPSE = 4;
 
-Blockly.Blocks.Utilities.renameCollapsed = function(block, n) {
-  if(n > Blockly.Blocks.Utilities.MAX_COLLAPSE) return;
-  if (block.isCollapsed()) {
-    var COLLAPSED_INPUT_NAME = '_TEMP_COLLAPSED_INPUT';
-    block.removeInput(COLLAPSED_INPUT_NAME);
-    block.collapsed_ = false;
-    var text = block.toString(Blockly.COLLAPSE_CHARS);
-    block.collapsed_ = true;
-    block.appendDummyInput(COLLAPSED_INPUT_NAME).appendField(text);
-
-    if(block.type.indexOf("procedures_call") != -1) {
-      block.moveInputBefore(COLLAPSED_INPUT_NAME, 'ARG0');
-    }
-  }
-
-  if(block.parentBlock_) {
-    Blockly.Blocks.Utilities.renameCollapsed(block.parentBlock_, n+1);
-  }
-}
-
 // unicode multiplication symbol
 Blockly.Blocks.Utilities.times_symbol = '\u00D7';
-
-
-
-
-
-

appinventor/blocklyeditor/src/input.js

+// -*- mode: java; c-basic-offset: 2; -*-
+// Copyright © 2020 Massachusetts Institute of Technology. All rights reserved.
+
+/*
+ * Copyright 2012 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ * Statement of Changes:
+ * - Changed the source to set the field visibility to match
+ *   the input visibility when appending a field. This supports modifying
+ *   collapsed blocks.
+ */
+
+/**
+ * @license
+ * @fileoverview Visual blocks editor for MIT App Inventor
+ * Add additional "class methods" to Blockly.Input
+ */
+
+/**
+ * Add an item to the end of the input's field row.
+ *
+ * This is overridden so that we can properly set the visibility of the field
+ * when it is appended.
+ * @param {string|!Blockly.Field} field Something to add as a field.
+ * @param {string=} opt_name Language-neutral identifier which may used to find
+ *     this field again.  Should be unique to the host block.
+ * @return {!Blockly.Input} The input being append to (to allow chaining).
+ */
+Blockly.Input.prototype.appendField = function(field, opt_name) {
+  // Empty string, Null or undefined generates no field, unless field is named.
+  if (!field && !opt_name) {
+    return this;
+  }
+  // Generate a FieldLabel when given a plain text field.
+  if (goog.isString(field)) {
+    field = new Blockly.FieldLabel(/** @type {string} */ (field));
+  }
+  field.setSourceBlock(this.sourceBlock_);
+  if (this.sourceBlock_.rendered) {
+    field.init();
+  }
+  field.name = opt_name;
+  field.setVisible(this.isVisible());
+
+  if (field.prefixField) {
+    // Add any prefix.
+    this.appendField(field.prefixField);
+  }
+  // Add the field to the field row.
+  this.fieldRow.push(field);
+  if (field.suffixField) {
+    // Add any suffix.
+    this.appendField(field.suffixField);
+  }
+
+  //If it's a COLLAPSE_TEXT input, hide it by default
+  if (opt_name === 'COLLAPSED_TEXT')
+    this.sourceBlock_.getTitle_(opt_name).getRootElement().style.display = 'none';
+
+  if (this.sourceBlock_.rendered) {
+    this.sourceBlock_.render();
+    // Adding a field will cause the block to change shape.
+    this.sourceBlock_.bumpNeighbours_();
+  }
+  return this;
+};
+
+/**
+ * Sets whether this input is visible or not.
+ * Used to collapse/uncollapse a block.
+ * 
+ * This is overridden so that it does not set the child's rendered
+ * property to false.
+ * @param {boolean} visible True if visible.
+ * @return {!Array.<!Blockly.Block>} List of blocks to render.
+ */
+Blockly.Input.prototype.setVisible = function(visible) {
+  var renderList = [];
+  if (this.visible_ == visible) {
+    return renderList;
+  }
+  this.visible_ = visible;
+
+  var display = visible ? 'block' : 'none';
+  for (var y = 0, field; field = this.fieldRow[y]; y++) {
+    field.setVisible(visible);
+  }
+  if (this.connection) {
+    // Has a connection.
+    if (visible) {
+      renderList = this.connection.unhideAll();
+    } else {
+      this.connection.hideAll();
+    }
+    var child = this.connection.targetBlock();
+    if (child) {
+      child.getSvgRoot().style.display = display;
+    }
+  }
+  return renderList;
+};

appinventor/blocklyeditor/src/rendered_connection.js

+// -*- mode: java; c-basic-offset: 2; -*-
+// Copyright © 2020 Massachusetts Institute of Technology. All rights reserved.
+
+/**
+ * @license
+ * @fileoverview Visual blocks editor for MIT App Inventor
+ * Add additional "class methods" to Blockly.RenderedConnection
+ */
+
+var oldConnect = Blockly.RenderedConnection.prototype.connect_;
+
+/**
+ * Connect two connections together.  This is the connection on the superior
+ * block.  Rerender blocks as needed.
+ *
+ * This function is overridden so that we can properly update the visibility
+ * of the child block to match the input (if this connection is an input
+ * connection).
+ * @param {!Blockly.Connection} childConnection Connection on inferior block.
+ * @private
+ */
+Blockly.RenderedConnection.prototype.connect_ = function(childConnection) {
+  oldConnect.call(this, childConnection);
+
+  var input = this.getInput();
+  if (!input) {
+    return;
+  }
+
+  var visible = input.isVisible();
+  var block = childConnection.getSourceBlock();
+  block.getSvgRoot().style.display = visible ? 'block' : 'none';
+};
+
+Blockly.RenderedConnection.prototype.input_;
+
+/**
+ * Returns the input this connection belongs to, or null if this connection
+ * does not belong to an input.
+ * @return {Blockly.Input} The input this block is connected to.
+ */
+Blockly.RenderedConnection.prototype.getInput = function() {
+  if (this.input_ !== undefined) {
+    return this.input_;
+  }
+
+  var inputs = this.sourceBlock_.inputList;
+  for (var i = 0, input; (input = inputs[i]); i++) {
+    if (input.connection == this) {
+      this.input_ = input;
+      return input;
+    }
+  }
+  this.input_ = null;
+  return null;
+};
+
+var oldDisconnectInternal = Blockly.RenderedConnection.
+    prototype.disconnectInternal_;
+/**
+ * Disconnect two blocks that are connected by this connection.
+ *
+ * This function is overridden so that we can set the block to visible
+ * when it is disconnected. This handles the case where the block was "inside"
+ * of a collapsed block.
+ * @param {!Blockly.Block} parentBlock The superior block.
+ * @param {!Blockly.Block} childBlock The inferior block.
+ * @private
+ */
+Blockly.RenderedConnection.prototype.disconnectInternal_ = function(parentBlock,
+    childBlock) {
+  oldDisconnectInternal.call(this, parentBlock, childBlock);
+
+  // Reset visibility, since this block is now a top block.
+  childBlock.getSvgRoot().style.display = 'block';
+};

appinventor/blocklyeditor/src/workspace_svg.js

@@ -324,7 +324,7 @@ Blockly.WorkspaceSvg.prototype.render = function(blocks) {
       for (var t = 0, topBlock; topBlock = topBlocks[t]; t++) {
         Blockly.Instrument.timer(
           function () {
-            topBlock.renderDown();
+            topBlock.render(false);
           },
           function (result, timeDiffInner) {
             Blockly.Instrument.stats.renderDownTime += timeDiffInner;

appinventor/blocklyeditor/tests/com/google/appinventor/mocha/index.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Mocha Tests for Blockly</title>
+
+    <link href="https://unpkg.com/mocha@5.2.0/mocha.css" rel="stylesheet" />
+    <script src="../../../../../../build/blocklyeditor/blockly-all.js"></script>
+    <script src="../../../../../../blocklyeditor/src/msg/en/_messages.js"></script>
+</head>
+<body>
+
+<div id="mocha"></div>
+<div id="failureCount" style="display:none" tests_failed="unset"></div>
+<script src="https://unpkg.com/chai/chai.js"></script>
+<script src="https://unpkg.com/mocha@5.2.0/mocha.js"></script>
+<script src="https://unpkg.com/sinon/pkg/sinon.js"></script>
+<script>
+  mocha.setup({
+    ui: 'tdd'
+  });
+</script>
+
+<script src="block.js"></script>
+
+<div id="blocklyDiv"></div>
+
+<script>
+  mocha.run(function(failures) {
+    var failureDiv = document.getElementById('failureCount');
+    failureDiv.setAttribute('tests_failed', failures);
+  });
+</script>
+</body>
+</html>