Fixing the tree service so it doesn't treat unknown block deletion (#1182)

as an error, and turning off keypresses on the workspace when the
variable modals are open.

accessible/tree.service.js

@@ -33,6 +33,7 @@ goog.require('blocklyApp.AudioService');
 goog.require('blocklyApp.BlockConnectionService');
 goog.require('blocklyApp.BlockOptionsModalService');
 goog.require('blocklyApp.NotificationsService');
+goog.require('blocklyApp.VariableModalService');
 
 
 blocklyApp.TreeService = ng.core.Class({
@@ -42,14 +43,16 @@ blocklyApp.TreeService = ng.core.Class({
     blocklyApp.BlockOptionsModalService,
     blocklyApp.NotificationsService,
     blocklyApp.UtilsService,
+    blocklyApp.VariableModalService,
     function(
         audioService, blockConnectionService, blockOptionsModalService,
-        notificationsService, utilsService) {
+        notificationsService, utilsService, variableModalService) {
       this.audioService = audioService;
       this.blockConnectionService = blockConnectionService;
       this.blockOptionsModalService = blockOptionsModalService;
       this.notificationsService = notificationsService;
       this.utilsService = utilsService;
+      this.variableModalService = variableModalService;
 
       // The suffix used for all IDs of block root elements.
       this.BLOCK_ROOT_ID_SUFFIX_ = blocklyApp.BLOCK_ROOT_ID_SUFFIX;
@@ -217,11 +220,10 @@ blocklyApp.TreeService = ng.core.Class({
     var activeDesc = document.getElementById(this.getActiveDescId(treeId));
     if (activeDesc) {
       activeDesc.classList.remove('blocklyActiveDescendant');
+    }
+
+    if (this.activeDescendantIds_[treeId]) {
       delete this.activeDescendantIds_[treeId];
-    } else {
-      throw Error(
-          'The active desc element for the tree with ID ' + treeId +
-          ' is invalid.');
     }
   },
   clearAllActiveDescs: function() {
@@ -470,14 +472,16 @@ blocklyApp.TreeService = ng.core.Class({
   onKeypress: function(e, tree) {
     // TODO(sll): Instead of this, have a common ActiveContextService which
     // returns true if at least one modal is shown, and false otherwise.
-    if (this.blockOptionsModalService.isModalShown()) {
+    if (this.blockOptionsModalService.isModalShown() ||
+        this.variableModalService.isModalShown()) {
       return;
     }
 
     var treeId = tree.id;
     var activeDesc = document.getElementById(this.getActiveDescId(treeId));
     if (!activeDesc) {
-      console.error('ERROR: no active descendant for current tree.');
+      // The underlying Blockly instance may have decided blocks needed to
+      // be deleted. This is not necessarily an error, but needs to be repaired.
       this.initActiveDesc(treeId);
       activeDesc = document.getElementById(this.getActiveDescId(treeId));
     }

accessible/variable-add-modal.component.js

@@ -108,10 +108,11 @@ blocklyApp.VariableAddModalComponent = ng.core.Component({
   // Submits the name change for the variable.
   submit: function() {
     this.workspace.createVariable(this.variableName);
-    this.hideModal_();
+    this.dismissModal();
   },
   // Dismisses and closes the modal.
   dismissModal: function() {
+    this.variableModalService.hideModal();
     this.hideModal_();
   }
 })

accessible/variable-modal.service.js

@@ -25,7 +25,6 @@
 
 goog.provide('blocklyApp.VariableModalService');
 
-
 blocklyApp.VariableModalService = ng.core.Class({
   constructor: [
     function() {
@@ -67,12 +66,19 @@ blocklyApp.VariableModalService = ng.core.Class({
   // Show the remove variable modal.
   showRemoveModal_: function(oldName) {
     var count = this.getNumVariables(oldName);
+    this.modalIsShown = true;
     if (count > 1) {
       this.preRemoveShowHook(oldName, count);
-      this.modalIsShown = true;
     } else {
       var variable = blocklyApp.workspace.getVariable(oldName);
       blocklyApp.workspace.deleteVariableInternal_(variable);
+      // Allow the execution loop to finish before "closing" the modal. While
+      // the modal never opens, its being "open" should prevent other keypresses
+      // anyway.
+      var that = this;
+      setTimeout(function() {
+        that.modalIsShown = false;
+      });
     }
   },
   getNumVariables: function(oldName) {

accessible/variable-remove-modal.component.js

@@ -28,6 +28,7 @@ goog.provide('blocklyApp.VariableRemoveModalComponent');
 goog.require('blocklyApp.AudioService');
 goog.require('blocklyApp.KeyboardInputService');
 goog.require('blocklyApp.TranslatePipe');
+goog.require('blocklyApp.TreeService');
 goog.require('blocklyApp.VariableModalService');
 
 goog.require('Blockly.CommonModal');
@@ -64,9 +65,13 @@ blocklyApp.VariableRemoveModalComponent = ng.core.Component({
 })
 .Class({
   constructor: [
-    blocklyApp.AudioService, blocklyApp.KeyboardInputService, blocklyApp.VariableModalService,
-    function(audioService, keyboardService, variableService) {
+    blocklyApp.AudioService,
+    blocklyApp.KeyboardInputService,
+    blocklyApp.TreeService,
+    blocklyApp.VariableModalService,
+    function(audioService, keyboardService, treeService, variableService) {
       this.workspace = blocklyApp.workspace;
+      this.treeService = treeService;
       this.variableModalService = variableService;
       this.audioService = audioService;
       this.keyboardInputService = keyboardService
@@ -110,10 +115,11 @@ blocklyApp.VariableRemoveModalComponent = ng.core.Component({
   submit: function() {
     var variable = blocklyApp.workspace.getVariable(this.currentVariableName);
     blocklyApp.workspace.deleteVariableInternal_(variable);
-    this.hideModal_();
+    this.dismissModal();
   },
   // Dismisses and closes the modal.
   dismissModal: function() {
+    this.variableModalService.hideModal();
     this.hideModal_();
   }
 })

accessible/variable-rename-modal.component.js

@@ -111,10 +111,11 @@ blocklyApp.VariableRenameModalComponent = ng.core.Component({
   // Submits the name change for the variable.
   submit: function() {
     this.workspace.renameVariable(this.currentVariableName, this.variableName);
-    this.hideModal_();
+    this.dismissModal();
   },
   // Dismisses and closes the modal.
   dismissModal: function() {
+    this.variableModalService.hideModal();
     this.hideModal_();
   }
 })