From 7bba4fa59c35ceb08d8c4c9f20760821480bce67 Mon Sep 17 00:00:00 2001 From: alschmiedt Date: Wed, 21 Aug 2019 16:33:16 -0700 Subject: [PATCH] Moves the cursor to correct location when block is deleted (#2887) * Moves the cursor to correct location when block is deleted * Moves cursor on block mutation --- core/block.js | 6 +++ core/block_svg.js | 4 ++ core/keyboard_nav/cursor_svg.js | 8 +--- core/keyboard_nav/navigation.js | 73 +++++++++++++++++++++++++++++++++ core/mutator.js | 4 ++ tests/mocha/navigation_test.js | 68 +++++++++++++++++++++++++++--- 6 files changed, 150 insertions(+), 13 deletions(-) diff --git a/core/block.js b/core/block.js index d63612aaf..680ea2034 100644 --- a/core/block.js +++ b/core/block.js @@ -287,6 +287,12 @@ Blockly.Block.prototype.dispose = function(healStack) { if (this.onchangeWrapper_) { this.workspace.removeChangeListener(this.onchangeWrapper_); } + + if (Blockly.keyboardAccessibilityMode) { + // No-op if this is called from the block_svg class. + Blockly.navigation.moveCursorOnBlockDelete(this); + } + this.unplug(healStack); if (Blockly.Events.isEnabled()) { Blockly.Events.fire(new Blockly.Events.BlockDelete(this)); diff --git a/core/block_svg.js b/core/block_svg.js index c6d5b261d..6bce7a363 100644 --- a/core/block_svg.js +++ b/core/block_svg.js @@ -907,6 +907,10 @@ Blockly.BlockSvg.prototype.dispose = function(healStack, animate) { Blockly.ContextMenu.hide(); } + if (Blockly.keyboardAccessibilityMode) { + Blockly.navigation.moveCursorOnBlockDelete(this); + } + if (animate && this.rendered) { this.unplug(healStack); Blockly.blockAnimations.disposeUiEffect(this); diff --git a/core/keyboard_nav/cursor_svg.js b/core/keyboard_nav/cursor_svg.js index 90ae5268d..5417eb362 100644 --- a/core/keyboard_nav/cursor_svg.js +++ b/core/keyboard_nav/cursor_svg.js @@ -138,18 +138,12 @@ Blockly.CursorSvg.prototype.setParent_ = function(newParent) { if (newParent == oldParent) { return; } - var svgRoot = this.getSvgRoot(); if (newParent) { newParent.appendChild(svgRoot); + this.parent_ = newParent; } - // If we are losing a parent, we want to move our DOM element to the - // root of the workspace. - else if (oldParent) { - this.workspace_.getCanvas().appendChild(svgRoot); - } - this.parent_ = newParent; }; /**************************/ diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 0d059b5f9..d855d354a 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -662,6 +662,79 @@ Blockly.navigation.handleEnterForWS = function() { /** Helper Functions **/ /**********************/ +/** + * Finds the source block of the location on a given node. + * @param {Blockly.ASTNode} node The node to find the source block on. + * @return {Blockly.Block} The source block of the location on the given node, + * or null if the node is of type workspace. + * @private + */ +Blockly.navigation.getSourceBlock_ = function(node) { + if (!node) { + return null; + } + if (node.getType() === Blockly.ASTNode.types.BLOCK) { + return node.getLocation(); + } else if (node.getType() === Blockly.ASTNode.types.WORKSPACE || + node.getType() === Blockly.ASTNode.types.STACK) { + return null; + } else { + return node.getLocation().getSourceBlock(); + } +}; + +/** + * Before a block is deleted move the cursor to the appropriate position. + * @param {!Blockly.Block} deletedBlock The block that is being deleted. + * @package + */ +Blockly.navigation.moveCursorOnBlockDelete = function(deletedBlock) { + var cursor = Blockly.navigation.cursor_; + if (cursor) { + var curNode = cursor.getCurNode(); + var block = Blockly.navigation.getSourceBlock_(curNode); + + if (block === deletedBlock) { + // If the block has a parent move the cursor to their connection point. + if (block.getParent()) { + var topConnection = block.previousConnection ? + block.previousConnection : block.outputConnection; + if (topConnection) { + cursor.setLocation( + Blockly.ASTNode.createConnectionNode(topConnection.targetConnection)); + } + } else { + // If the block is by itself move the cursor to the workspace. + cursor.setLocation(Blockly.ASTNode.createWorkspaceNode(block.workspace, + block.getRelativeToSurfaceXY())); + } + // If the cursor is on a block whose parent is being deleted, move the + // cursor to the workspace. + } else if (deletedBlock.getChildren().indexOf(block) > -1) { + cursor.setLocation(Blockly.ASTNode.createWorkspaceNode(block.workspace, + block.getRelativeToSurfaceXY())); + } + } +}; + +/** + * When a block that the cursor is on is mutated move the cursor to the block + * level. + * @param {!Blockly.Block} mutatedBlock The block that is being mutated. + * @package + */ +Blockly.navigation.moveCursorOnBlockMutation = function(mutatedBlock) { + var cursor = Blockly.navigation.cursor_; + if (cursor) { + var curNode = cursor.getCurNode(); + var block = Blockly.navigation.getSourceBlock_(curNode); + + if (block === mutatedBlock) { + cursor.setLocation(Blockly.ASTNode.createBlockNode(block)); + } + } +}; + /** * Handler for all the keyboard navigation events. * @param {Event} e The keyboard event. diff --git a/core/mutator.js b/core/mutator.js index b2a2f21f3..7a283e011 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -373,6 +373,10 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { if (block.rendered) { block.render(); } + + if (oldMutation != newMutation && Blockly.keyboardAccessibilityMode) { + Blockly.navigation.moveCursorOnBlockMutation(block); + } // Don't update the bubble until the drag has ended, to avoid moving blocks // under the cursor. if (!this.workspace_.isDragging()) { diff --git a/tests/mocha/navigation_test.js b/tests/mocha/navigation_test.js index c7958b18c..722c094cd 100644 --- a/tests/mocha/navigation_test.js +++ b/tests/mocha/navigation_test.js @@ -94,7 +94,8 @@ suite('Navigation', function() { chai.assert.equal(Blockly.navigation.currentState_, Blockly.navigation.STATE_FLYOUT); - chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), "FirstCategory-FirstBlock"); + chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), + "FirstCategory-FirstBlock"); }); test('Focuses workspace from toolbox (e)', function() { @@ -153,17 +154,20 @@ suite('Navigation', function() { chai.assert.isTrue(Blockly.navigation.onKeyPress(this.mockEvent)); chai.assert.equal(Blockly.navigation.currentState_, Blockly.navigation.STATE_FLYOUT); - chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), "FirstCategory-FirstBlock"); + chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), + "FirstCategory-FirstBlock"); }); test('Previous', function() { Blockly.navigation.selectNextBlockInFlyout(); - chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), "FirstCategory-SecondBlock"); + chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), + "FirstCategory-SecondBlock"); this.mockEvent.keyCode = Blockly.utils.KeyCodes.W; chai.assert.isTrue(Blockly.navigation.onKeyPress(this.mockEvent)); chai.assert.equal(Blockly.navigation.currentState_, Blockly.navigation.STATE_FLYOUT); - chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), "FirstCategory-FirstBlock"); + chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), + "FirstCategory-FirstBlock"); }); test('Next', function() { @@ -171,7 +175,8 @@ suite('Navigation', function() { chai.assert.isTrue(Blockly.navigation.onKeyPress(this.mockEvent)); chai.assert.equal(Blockly.navigation.currentState_, Blockly.navigation.STATE_FLYOUT); - chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), "FirstCategory-SecondBlock"); + chai.assert.equal(Blockly.navigation.flyoutBlock_.getFieldValue("TEXT"), + "FirstCategory-SecondBlock"); }); test('Out', function() { @@ -386,6 +391,12 @@ suite('Navigation', function() { this.basicBlock2 = basicBlock2; }); + teardown(function() { + delete Blockly.Blocks['basic_block']; + this.workspace.dispose(); + Blockly.navigation.currentCategory_ = null; + }); + test('Insert from flyout with a valid connection marked', function() { var previousConnection = this.basicBlock.previousConnection; var prevNode = Blockly.ASTNode.createConnectionNode(previousConnection); @@ -431,12 +442,57 @@ suite('Navigation', function() { chai.assert.isNotNull(insertedBlock); }); + }); + suite('Test cursor move on block delete', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([{ + "type": "basic_block", + "message0": "", + "previousStatement": null, + "nextStatement": null, + }]); + var toolbox = document.getElementById('toolbox-categories'); + this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox}); + Blockly.navigation.focusWorkspace(); + this.basicBlockA = this.workspace.newBlock('basic_block'); + this.basicBlockB = this.workspace.newBlock('basic_block'); + Blockly.keyboardAccessibilityMode = true; + }); teardown(function() { delete Blockly.Blocks['basic_block']; this.workspace.dispose(); - Blockly.navigation.currentCategory_ = null; + }); + + test('Delete block - has parent ', function() { + this.basicBlockA.nextConnection.connect(this.basicBlockB.previousConnection); + var astNode = Blockly.ASTNode.createBlockNode(this.basicBlockB); + // Set the cursor to be on the child block + this.workspace.cursor.setLocation(astNode); + // Remove the child block + this.basicBlockB.dispose(); + chai.assert.equal(this.workspace.cursor.getCurNode().getType(), + Blockly.ASTNode.types.NEXT); + }); + + test('Delete block - no parent ', function() { + var astNode = Blockly.ASTNode.createBlockNode(this.basicBlockB); + this.workspace.cursor.setLocation(astNode); + this.basicBlockB.dispose(); + chai.assert.equal(this.workspace.cursor.getCurNode().getType(), + Blockly.ASTNode.types.WORKSPACE); + }); + + test('Delete parent block', function() { + this.basicBlockA.nextConnection.connect(this.basicBlockB.previousConnection); + var astNode = Blockly.ASTNode.createBlockNode(this.basicBlockB); + // Set the cursor to be on the child block + this.workspace.cursor.setLocation(astNode); + // Remove the parent block + this.basicBlockA.dispose(); + chai.assert.equal(this.workspace.cursor.getCurNode().getType(), + Blockly.ASTNode.types.WORKSPACE); }); }); });