From 36b0213533718411277f15374434e71d8f8bd800 Mon Sep 17 00:00:00 2001
From: Sean Lip <sll@google.com>
Date: Mon, 28 Nov 2016 14:26:38 -0800
Subject: [PATCH] Streamline the logic for block selection callbacks in the
 toolbox modal.

---
 accessible/block-options-modal.service.js |  4 +--
 accessible/clipboard.service.js           |  4 +--
 accessible/sidebar.component.js           |  6 ++--
 accessible/toolbox-modal.component.js     | 16 +++++++---
 accessible/toolbox-modal.service.js       | 39 ++++++++++-------------
 5 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/accessible/block-options-modal.service.js b/accessible/block-options-modal.service.js
index 3df6a1aa8..6b77b92bd 100644
--- a/accessible/block-options-modal.service.js
+++ b/accessible/block-options-modal.service.js
@@ -46,9 +46,7 @@ blocklyApp.BlockOptionsModalService = ng.core.Class({
     this.actionButtonsInfo = actionButtonsInfo;
     this.onCancelCallback = onCancelCallback;
 
-    if (this.preShowHook) {
-      this.preShowHook();
-    }
+    this.preShowHook();
     this.modalIsShown = true;
   },
   hideModal: function() {
diff --git a/accessible/clipboard.service.js b/accessible/clipboard.service.js
index 0a51c44b7..da3281cd8 100644
--- a/accessible/clipboard.service.js
+++ b/accessible/clipboard.service.js
@@ -78,9 +78,9 @@ blocklyApp.ClipboardService = ng.core.Class({
       markedSpotAncestorBlock = markedSpotAncestorBlock.getParent();
     }
 
-    return this.canBeCopiedToMarkedConnection(block);
+    return this.canBeAttachedToMarkedConnection(block);
   },
-  canBeCopiedToMarkedConnection: function(block) {
+  canBeAttachedToMarkedConnection: function(block) {
     if (!this.markedConnection_ ||
         !this.markedConnection_.getSourceBlock().workspace) {
       return false;
diff --git a/accessible/sidebar.component.js b/accessible/sidebar.component.js
index 52901867b..225195b16 100644
--- a/accessible/sidebar.component.js
+++ b/accessible/sidebar.component.js
@@ -37,7 +37,7 @@ blocklyApp.SidebarComponent = ng.core.Component({
           {{buttonConfig.text}}
         </button>
       </span>
-      <button (click)="showToolboxModalForAttachToMarkedSpot()"
+      <button (click)="showToolboxModalForAttachToMarkedConnection()"
               [attr.disabled]="isAnyConnectionMarked() ? undefined : 'disabled'"
               [attr.aria-disabled]="!isAnyConnectionMarked()"
               class="blocklySidebarButton">
@@ -97,8 +97,8 @@ blocklyApp.SidebarComponent = ng.core.Component({
   isWorkspaceEmpty: function() {
     return this.utilsService.isWorkspaceEmpty();
   },
-  showToolboxModalForAttachToMarkedSpot: function() {
-    this.toolboxModalService.showToolboxModalForAttachToMarkedSpot();
+  showToolboxModalForAttachToMarkedConnection: function() {
+    this.toolboxModalService.showToolboxModalForAttachToMarkedConnection();
   },
   showToolboxModalForCreateNewGroup: function() {
     this.toolboxModalService.showToolboxModalForCreateNewGroup();
diff --git a/accessible/toolbox-modal.component.js b/accessible/toolbox-modal.component.js
index 9802c74ea..420b99843 100644
--- a/accessible/toolbox-modal.component.js
+++ b/accessible/toolbox-modal.component.js
@@ -38,7 +38,7 @@ blocklyApp.ToolboxModalComponent = ng.core.Component({
             <div class="blocklyModalButtonContainer"
                  *ngFor="#block of toolboxCategory.blocks; #blockIndex=index">
               <button [id]="getOptionId(getOverallIndex(categoryIndex, blockIndex))"
-                      (click)="hideModal(getBlock(categoryIndex, blockIndex))"
+                      (click)="selectBlock(getBlock(categoryIndex, blockIndex))"
                       [ngClass]="{activeButton: activeButtonIndex == getOverallIndex(categoryIndex, blockIndex)}"
                       [attr.disabled]="isBlockAvailable(getBlock(categoryIndex, blockIndex)) ? undefined : 'disabled'"
                       [attr.aria-disabled]="!isBlockAvailable(getBlock(categoryIndex, blockIndex))">
@@ -83,10 +83,11 @@ blocklyApp.ToolboxModalComponent = ng.core.Component({
 
       var that = this;
       this.toolboxModalService.registerPreShowHook(
-        function(toolboxCategories, isBlockAvailable) {
+        function(toolboxCategories, isBlockAvailable, onSelectBlockCallback) {
           that.modalIsVisible = true;
           that.toolboxCategories = toolboxCategories;
           that.isBlockAvailable = isBlockAvailable;
+          that.onSelectBlockCallback = onSelectBlockCallback;
 
           var cumulativeIndex = 0;
           that.toolboxCategories.forEach(function(category) {
@@ -121,11 +122,12 @@ blocklyApp.ToolboxModalComponent = ng.core.Component({
                   var blockIndex =
                       that.activeButtonIndex - that.firstBlockIndexes[i];
                   var block = that.getBlock(categoryIndex, blockIndex);
-                  that.hideModal(block);
+                  that.selectBlock(block);
                   return;
                 }
               }
 
+              // The 'Cancel' button has been pressed.
               that.hideModal();
             },
             // Escape key: closes the modal.
@@ -187,10 +189,14 @@ blocklyApp.ToolboxModalComponent = ng.core.Component({
   getCancelOptionId: function() {
     return 'toolbox-modal-option-' + this.totalNumBlocks;
   },
+  selectBlock: function(block) {
+    this.onSelectBlockCallback(block);
+    this.hideModal();
+  },
   // Closes the modal.
-  hideModal: function(opt_block) {
+  hideModal: function() {
     this.modalIsVisible = false;
     this.keyboardInputService.clearOverride();
-    this.toolboxModalService.hideModal(opt_block);
+    this.toolboxModalService.hideModal();
   }
 });
diff --git a/accessible/toolbox-modal.service.js b/accessible/toolbox-modal.service.js
index 87e233b9d..788193872 100644
--- a/accessible/toolbox-modal.service.js
+++ b/accessible/toolbox-modal.service.js
@@ -36,8 +36,9 @@ blocklyApp.ToolboxModalService = ng.core.Class({
       this.clipboardService = _clipboardService;
 
       this.modalIsShown = false;
-      this.onHideCallback = null;
+
       this.isBlockAvailable = null;
+      this.onSelectBlockCallback = null;
       this.preShowHook = function() {
         throw Error(
             'A pre-show hook must be defined for the toolbox modal before it ' +
@@ -81,30 +82,29 @@ blocklyApp.ToolboxModalService = ng.core.Class({
   ],
   registerPreShowHook: function(preShowHook) {
     this.preShowHook = function() {
-      preShowHook(this.toolboxCategories, this.isBlockAvailable);
+      preShowHook(
+          this.toolboxCategories, this.isBlockAvailable,
+          this.onSelectBlockCallback);
     };
   },
   isModalShown: function() {
     return this.modalIsShown;
   },
-  showModal: function(onHideCallback, isBlockAvailable) {
-    this.onHideCallback = onHideCallback;
+  showModal_: function(isBlockAvailable, onSelectBlockCallback) {
     this.isBlockAvailable = isBlockAvailable;
+    this.onSelectBlockCallback = onSelectBlockCallback;
+
     this.preShowHook();
     this.modalIsShown = true;
   },
-  hideModal: function(opt_block) {
+  hideModal: function() {
     this.modalIsShown = false;
-    this.onHideCallback(opt_block);
   },
-  showToolboxModalForAttachToMarkedSpot: function() {
+  showToolboxModalForAttachToMarkedConnection: function() {
     var that = this;
-    this.showModal(function(opt_block) {
-      if (!opt_block) {
-        return;
-      }
-
-      var block = opt_block;
+    this.showModal_(function(block) {
+      return that.clipboardService.canBeAttachedToMarkedConnection(block);
+    }, function(block) {
       var blockDescription = that.utilsService.getBlockDescription(block);
 
       // Clean up the active desc for the destination tree.
@@ -131,18 +131,13 @@ blocklyApp.ToolboxModalService = ng.core.Class({
             blockDescription + ' connected. ' +
             'Now on copied block in workspace.');
       });
-    }, function(block) {
-      return that.clipboardService.canBeCopiedToMarkedConnection(block);
     });
   },
   showToolboxModalForCreateNewGroup: function() {
     var that = this;
-    this.showModal(function(opt_block) {
-      if (!opt_block) {
-        return;
-      }
-
-      var block = opt_block;
+    this.showModal_(function(block) {
+      return true;
+    }, function(block) {
       var blockDescription = that.utilsService.getBlockDescription(block);
       var xml = Blockly.Xml.blockToDom(block);
       var newBlockId = Blockly.Xml.domToBlock(blocklyApp.workspace, xml).id;
@@ -153,8 +148,6 @@ blocklyApp.ToolboxModalService = ng.core.Class({
             blockDescription + ' added to workspace. ' +
             'Now on added block in workspace.');
       });
-    }, function(block) {
-      return true;
     });
   }
 });