Skip to content

Commit

Permalink
fix: upgrade keyboard navigation to use new clipboard API (google#1844)
Browse files Browse the repository at this point in the history
* fix: upgrade keyboard navigation to use new clipboard API

* chore: reorganize test suites

* chore: fixup delete tests

* chore: work on fixing copy tests

* chore: finish fixing up copy tests

* chore: fix cut and paste tests

* chore: delete paste suite

* chore: delete unnecessary test helpers

* chore: fixup other tests

* chore: document why we're stubbing cancelAnimationFrame and add it to teardown
  • Loading branch information
BeksOmega committed Sep 7, 2023
1 parent af95f13 commit 90bd48f
Show file tree
Hide file tree
Showing 7 changed files with 370 additions and 320 deletions.
16 changes: 8 additions & 8 deletions plugins/keyboard-navigation/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions plugins/keyboard-navigation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@
"devDependencies": {
"@blockly/dev-scripts": "^2.0.1",
"@blockly/dev-tools": "^7.0.3",
"blockly": "^10.0.0",
"blockly": "^10.2.0-beta.0",
"chai": "^4.2.0",
"jsdom": "^16.4.0",
"jsdom-global": "^3.0.2",
"mocha": "^7.1.0",
"sinon": "^9.0.1"
},
"peerDependencies": {
"blockly": "^10.0.0"
"blockly": "^10.2.0-beta.0"
},
"publishConfig": {
"access": "public",
Expand Down
14 changes: 10 additions & 4 deletions plugins/keyboard-navigation/src/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -1111,16 +1111,22 @@ export class Navigation {
}

/**
* Pastes the coped block to the marked location.
* Pastes the copied block to the marked location.
* @param {Blockly.BlockCopyData} copyData The data
* to paste into the workspace.
* @param {Blockly.WorkspaceSvg} workspace The workspace to paste the data
* into.
* @returns {boolean} True if the paste was sucessful, false otherwise.
* @package
*/
paste() {
paste(copyData, workspace) {
let isHandled = false;
Blockly.Events.setGroup(true);
const block = Blockly.clipboard.paste();
const block = /** @type {Blockly.BlockSvg} */ (
Blockly.clipboard.paste(copyData, workspace)
);
if (block) {
isHandled = this.insertPastedBlock(block.workspace, block);
isHandled = this.insertPastedBlock(workspace, block);
}
Blockly.Events.setGroup(false);
return isHandled;
Expand Down
22 changes: 17 additions & 5 deletions plugins/keyboard-navigation/src/navigation_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ import {Navigation} from './navigation';
* Class for registering shortcuts for keyboard navigation.
*/
export class NavigationController {
/** Data copied by the copy or cut keyboard shortcuts. */
copyData = null;

/** The workspace a copy or cut keyboard shortcut happened in. */
copyWorkspace = null;

/**
* Constructor used for registering shortcuts.
* This will register any default shortcuts for keyboard navigation.
Expand Down Expand Up @@ -715,9 +721,12 @@ export class NavigationController {
return false;
},
callback: (workspace) => {
const sourceBlock = workspace.getCursor().getCurNode().getSourceBlock();
const sourceBlock =/** @type {Blockly.BlockSvg} */ (
workspace.getCursor().getCurNode().getSourceBlock());
workspace.hideChaff();
Blockly.clipboard.copy(sourceBlock);
this.copyData = sourceBlock.toCopyData();
this.copyWorkspace = sourceBlock.workspace;
return !!this.copyData;
},
};

Expand Down Expand Up @@ -752,7 +761,8 @@ export class NavigationController {
!workspace.options.readOnly && !Blockly.Gesture.inProgress();
},
callback: () => {
return this.navigation.paste();
if (!this.copyData || !this.copyWorkspace) return false;
return this.navigation.paste(this.copyData, this.copyWorkspace);
},
};

Expand Down Expand Up @@ -797,8 +807,10 @@ export class NavigationController {
return false;
},
callback: (workspace) => {
const sourceBlock = workspace.getCursor().getCurNode().getSourceBlock();
Blockly.clipboard.copy(sourceBlock);
const sourceBlock =/** @type {Blockly.BlockSvg} */ (
workspace.getCursor().getCurNode().getSourceBlock());
this.copyData = sourceBlock.toCopyData();
this.copyWorkspace = sourceBlock.workspace;
this.navigation.moveCursorOnBlockDelete(workspace, sourceBlock);
sourceBlock.checkAndDelete();
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ suite('Insert/Modify', function() {
setup(function() {
this.jsdomCleanup =
require('jsdom-global')('<!DOCTYPE html><div id="blocklyDiv"></div>');
// We are running these tests in node even thought they require a rendered
// workspace, which doesn't exactly work. The rendering system expects
// cancelAnimationFrame to be defined so we need to define it.
window.cancelAnimationFrame = function() {};

// NOTE: block positions chosen such that they aren't unintentionally
// bumped out of bounds during tests.
const xmlText = `<xml xmlns="https://developers.google.com/blockly/xml">
Expand Down Expand Up @@ -116,6 +121,7 @@ suite('Insert/Modify', function() {
delete Blockly.Blocks['stack_block'];
delete Blockly.Blocks['row_block'];
delete Blockly.Blocks['statement_block'];
window.cancelAnimationFrame = undefined;
this.jsdomCleanup();
});

Expand Down
5 changes: 5 additions & 0 deletions plugins/keyboard-navigation/test/navigation_test.mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ suite('Navigation', function() {
setup(function() {
this.jsdomCleanup =
require('jsdom-global')('<!DOCTYPE html><div id="blocklyDiv"></div>');
// We are running these tests in node even thought they require a rendered
// workspace, which doesn't exactly work. The rendering system expects
// cancelAnimationFrame to be defined so we need to define it.
window.cancelAnimationFrame = function() {};
this.controller = new NavigationController();
this.controller.init();
this.navigation = this.controller.navigation;
});

teardown(function() {
this.controller.dispose();
window.cancelAnimationFrame = undefined;
this.jsdomCleanup();
});

Expand Down
Loading

0 comments on commit 90bd48f

Please sign in to comment.