From 75a842b884b1a789edcbdca5bf4fa2dde6ef7099 Mon Sep 17 00:00:00 2001
From: Sean Lip <sll@google.com>
Date: Mon, 1 Aug 2016 19:20:20 -0700
Subject: [PATCH] Refactor common functionality. Focus on new blocks
 immediately after they are created. Fix active descendant for tricky cases
 where moving a block to a marked spot splits the existing tree.

---
 accessible/clipboard.service.js        | 26 ++++++++++----
 accessible/toolbox-tree.component.js   | 37 ++++++++++++-------
 accessible/tree.service.js             | 35 ++++++++++++------
 accessible/workspace-tree.component.js | 50 ++++++++++----------------
 4 files changed, 85 insertions(+), 63 deletions(-)

diff --git a/accessible/clipboard.service.js b/accessible/clipboard.service.js
index 3aec5cc11..4ae568b26 100644
--- a/accessible/clipboard.service.js
+++ b/accessible/clipboard.service.js
@@ -26,8 +26,9 @@ blocklyApp.ClipboardService = ng.core
   .Class({
     constructor: [blocklyApp.UtilsService, function(_utilsService) {
       this.clipboardBlockXml_ = null;
-      this.clipboardBlockSuperiorConnection_ = null;
+      this.clipboardBlockPreviousConnection_ = null;
       this.clipboardBlockNextConnection_ = null;
+      this.clipboardBlockOutputConnection_ = null;
       this.markedConnection_ = null;
       this.utilsService = _utilsService;
     }],
@@ -40,11 +41,13 @@ blocklyApp.ClipboardService = ng.core
           connection.checkType_(blockConnection));
     },
     isCompatibleWithClipboard: function(connection) {
-      var superiorConnection = this.clipboardBlockSuperiorConnection_;
+      var previousConnection = this.clipboardBlockPreviousConnection_;
       var nextConnection = this.clipboardBlockNextConnection_;
+      var outputConnection = this.clipboardBlockOutputConnection_;
       return Boolean(
-          this.areConnectionsCompatible_(connection, superiorConnection) ||
-          this.areConnectionsCompatible_(connection, nextConnection));
+          this.areConnectionsCompatible_(connection, previousConnection) ||
+          this.areConnectionsCompatible_(connection, nextConnection) ||
+          this.areConnectionsCompatible_(connection, outputConnection));
     },
     getMarkedConnectionBlock: function() {
       if (!this.markedConnection_) {
@@ -98,9 +101,9 @@ blocklyApp.ClipboardService = ng.core
     },
     copy: function(block, announce) {
       this.clipboardBlockXml_ = Blockly.Xml.blockToDom(block);
-      this.clipboardBlockSuperiorConnection_ = block.outputConnection ||
-          block.previousConnection;
+      this.clipboardBlockPreviousConnection_ = block.previousConnection;
       this.clipboardBlockNextConnection_ = block.nextConnection;
+      this.clipboardBlockOutputConnection_ = block.outputConnection;
 
       if (announce) {
         alert(
@@ -108,7 +111,16 @@ blocklyApp.ClipboardService = ng.core
             this.utilsService.getBlockDescription(block));
       }
     },
-    pasteFromClipboard: function(connection) {
+    pasteFromClipboard: function(inputConnection) {
+      var connection = inputConnection;
+      // If the connection is a 'previousConnection' and that connection is
+      // already joined to something, use the 'nextConnection' of the
+      // previous block instead in order to do an insertion.
+      if (inputConnection.type == Blockly.PREVIOUS_STATEMENT &&
+          inputConnection.isConnected()) {
+        connection = inputConnection.targetConnection;
+      }
+
       var reconstitutedBlock = Blockly.Xml.domToBlock(blocklyApp.workspace,
           this.clipboardBlockXml_);
       switch (connection.type) {
diff --git a/accessible/toolbox-tree.component.js b/accessible/toolbox-tree.component.js
index ee1486029..40e28df87 100644
--- a/accessible/toolbox-tree.component.js
+++ b/accessible/toolbox-tree.component.js
@@ -64,8 +64,9 @@ blocklyApp.ToolboxTreeComponent = ng.core
             </li>
           </ol>
         </li>
+
         <template ngFor #inputBlock [ngForOf]="block.inputList" #i="index">
-          <li role="treeitem" [id]="idMap['listItem' + i]" [attr.aria-level]="level + 1" ng-if="inputBlock.fieldRow.length"
+          <li role="treeitem" [id]="idMap['listItem' + i]" [attr.aria-level]="level + 1" *ngIf="inputBlock.fieldRow.length"
               [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['fieldLabel' + i])">
             <blockly-field *ngFor="#field of inputBlock.fieldRow" [field]="field" [disabled]="true" [mainFieldId]="idMap['fieldLabel' + i]">
             </blockly-field>
@@ -141,23 +142,25 @@ blocklyApp.ToolboxTreeComponent = ng.core
       return this.clipboardService.canBeCopiedToMarkedConnection(this.block);
     },
     copyToWorkspace: function() {
+      var blockDescription = this.getBlockDescription();
       var xml = Blockly.Xml.blockToDom(this.block);
-      Blockly.Xml.domToBlock(blocklyApp.workspace, xml);
-      alert('Added block to workspace: ' + this.getBlockDescription());
+      var newBlockId = Blockly.Xml.domToBlock(blocklyApp.workspace, xml).id;
+
+      var that = this;
+      setTimeout(function() {
+        that.treeService.focusOnBlock(newBlockId);
+        alert('Added block to workspace: ' + blockDescription);
+      });
     },
     copyToClipboard: function() {
       this.clipboardService.copy(this.block, true);
     },
     copyToMarkedSpot: function() {
-      // This involves the following steps:
-      // - Clear screenreader focus on the destination tree.
-      // - Put the block on the destination tree.
-      // - Change the current tree-level focus to the destination tree, and the
-      // screenreader focus for the destination tree to the block just moved.
       var blockDescription = this.getBlockDescription();
-      var destinationTreeId = this.treeService.getTreeIdForBlock(
+      // Clean up the active desc for the destination tree.
+      var oldDestinationTreeId = this.treeService.getTreeIdForBlock(
           this.clipboardService.getMarkedConnectionBlock().id);
-      this.treeService.clearActiveDesc(destinationTreeId);
+      this.treeService.clearActiveDesc(oldDestinationTreeId);
 
       var newBlockId = this.clipboardService.pasteToMarkedConnection(
           this.block);
@@ -165,9 +168,17 @@ blocklyApp.ToolboxTreeComponent = ng.core
       // Invoke a digest cycle, so that the DOM settles.
       var that = this;
       setTimeout(function() {
-        document.getElementById(destinationTreeId).focus();
-        that.treeService.setActiveDesc(
-            newBlockId + 'blockRoot', destinationTreeId);
+        that.treeService.focusOnBlock(newBlockId);
+
+        var newDestinationTreeId = that.treeService.getTreeIdForBlock(
+            newBlockId);
+        if (newDestinationTreeId != oldDestinationTreeId) {
+          // It is possible for the tree ID for the pasted block to change
+          // after the paste operation, e.g. when inserting a block between two
+          // existing blocks that are joined together. In this case, we need to
+          // also reset the active desc for the old destination tree.
+          that.treeService.initActiveDesc(oldDestinationTreeId);
+        }
 
         alert('Block copied to marked spot: ' + blockDescription);
       });
diff --git a/accessible/tree.service.js b/accessible/tree.service.js
index 2108c0a81..3a96ad2a8 100644
--- a/accessible/tree.service.js
+++ b/accessible/tree.service.js
@@ -153,6 +153,11 @@ blocklyApp.TreeService = ng.core
         window.scrollTo(0, activeDescNode.offsetTop);
       }
     },
+    initActiveDesc: function(treeId) {
+      // Set the active desc to the first child in this tree.
+      var tree = document.getElementById(treeId);
+      this.setActiveDesc(this.getFirstChild(tree).id, treeId);
+    },
     getTreeIdForBlock: function(blockId) {
       // Walk up the DOM until we get to the root node of the tree.
       var domNode = document.getElementById(blockId + 'blockRoot');
@@ -161,6 +166,24 @@ blocklyApp.TreeService = ng.core
       }
       return domNode.id;
     },
+    focusOnBlock: function(blockId) {
+      // Set focus to the tree containing the given block, and set the active
+      // desc for this tree to the given block.
+      var domNode = document.getElementById(blockId + 'blockRoot');
+      // Walk up the DOM until we get to the root node of the tree.
+      while (!domNode.classList.contains('blocklyTree')) {
+        domNode = domNode.parentNode;
+      }
+      domNode.focus();
+
+      // We need to wait a while to set the active desc, because domNode takes
+      // a while to be given an ID if a new tree has just been created.
+      // TODO(sll): Make this more deterministic.
+      var that = this;
+      setTimeout(function() {
+        that.setActiveDesc(blockId + 'blockRoot', domNode.id);
+      }, 100);
+    },
     onWorkspaceToolbarKeypress: function(e, treeId) {
       if (e.keyCode == 9) {
         // Tab key.
@@ -208,17 +231,7 @@ blocklyApp.TreeService = ng.core
       var activeDesc = document.getElementById(this.getActiveDescId(treeId));
       if (!activeDesc) {
         console.error('ERROR: no active descendant for current tree.');
-
-        // TODO(sll): Generalize this to other trees (outside the workspace).
-        var workspaceTreeNodes = this.getWorkspaceTreeNodes_();
-        for (var i = 0; i < workspaceTreeNodes.length; i++) {
-          if (workspaceTreeNodes[i].id == treeId) {
-            // Set the active desc to the first child in this tree.
-            this.setActiveDesc(
-                this.getFirstChild(workspaceTreeNodes[i]).id, treeId);
-            break;
-          }
-        }
+        this.initActiveDesc(treeId);
         return;
       }
 
diff --git a/accessible/workspace-tree.component.js b/accessible/workspace-tree.component.js
index e409c6980..5999dd2ea 100644
--- a/accessible/workspace-tree.component.js
+++ b/accessible/workspace-tree.component.js
@@ -167,45 +167,23 @@ blocklyApp.WorkspaceTreeComponent = ng.core
       alert('Block deleted: ' + blockDescription);
     },
     pasteToConnection_: function(connection) {
-      // This involves two steps:
-      // - Put the block on the destination tree.
-      // - Change the current tree-level focus to the destination tree, and the
-      // screenreader focus for the destination tree to the block just moved.
-      var newBlockId = null;
+      var destinationTreeId = this.treeService.getTreeIdForBlock(
+          connection.getSourceBlock().id);
+      this.treeService.clearActiveDesc(destinationTreeId);
 
-      this.treeService.clearActiveDesc(this.tree.id);
-
-      // If the connection is a 'previousConnection' and that connection is
-      // already joined to something, use the 'nextConnection' of the
-      // previous block instead in order to do an insertion.
-      if (connection.type == Blockly.PREVIOUS_STATEMENT &&
-          connection.isConnected()) {
-        newBlockId = this.clipboardService.pasteFromClipboard(
-            connection.targetConnection);
-      } else {
-        newBlockId = this.clipboardService.pasteFromClipboard(connection);
-      }
+      var newBlockId = this.clipboardService.pasteFromClipboard(connection);
 
       // Invoke a digest cycle, so that the DOM settles.
       var that = this;
       setTimeout(function() {
-        // Move the focus to the current tree.
-        document.getElementById(that.tree.id).focus();
-        // Move the screenreader focus to the newly-pasted block.
-        that.treeService.setActiveDesc(newBlockId + 'blockRoot', that.tree.id);
+        that.treeService.focusOnBlock(newBlockId);
       });
     },
     moveToMarkedSpot_: function() {
-      // This involves three steps:
-      // - Put the block on the destination tree.
-      // - Remove the block from the source tree, while preserving the
-      // screenreader focus for that tree.
-      // - Change the current tree-level focus to the destination tree, and the
-      // screenreader focus for the destination tree to the block just moved.
       var blockDescription = this.getBlockDescription();
-      var destinationTreeId = this.treeService.getTreeIdForBlock(
+      var oldDestinationTreeId = this.treeService.getTreeIdForBlock(
           this.clipboardService.getMarkedConnectionBlock().id);
-      this.treeService.clearActiveDesc(destinationTreeId);
+      this.treeService.clearActiveDesc(oldDestinationTreeId);
 
       var newBlockId = this.clipboardService.pasteToMarkedConnection(
           this.block);
@@ -217,9 +195,17 @@ blocklyApp.WorkspaceTreeComponent = ng.core
 
       // Invoke a digest cycle, so that the DOM settles.
       setTimeout(function() {
-        document.getElementById(destinationTreeId).focus();
-        that.treeService.setActiveDesc(
-            newBlockId + 'blockRoot', destinationTreeId);
+        that.treeService.focusOnBlock(newBlockId);
+
+        var newDestinationTreeId = that.treeService.getTreeIdForBlock(
+            newBlockId);
+        if (newDestinationTreeId != oldDestinationTreeId) {
+          // It is possible for the tree ID for the pasted block to change
+          // after the paste operation, e.g. when inserting a block between two
+          // existing blocks that are joined together. In this case, we need to
+          // also reset the active desc for the old destination tree.
+          that.treeService.initActiveDesc(oldDestinationTreeId);
+        }
 
         alert('Block moved to marked spot: ' + blockDescription);
       });