From 32bb84ec8fbb9a23295ce5bb6328548ce55e3db2 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Fri, 13 Jun 2025 11:57:03 -0700 Subject: [PATCH] Allow copying from readonly workspace and add cut tests Also cleans up logic a bit --- core/shortcut_items.ts | 22 +++--- tests/mocha/shortcut_items_test.js | 123 ++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/core/shortcut_items.ts b/core/shortcut_items.ts index 615f1edc2..dca2d7366 100644 --- a/core/shortcut_items.ts +++ b/core/shortcut_items.ts @@ -94,7 +94,6 @@ export function registerDelete() { } let copyData: ICopyData | null = null; -let copyWorkspace: WorkspaceSvg | null = null; let copyCoords: Coordinate | null = null; /** @@ -156,7 +155,6 @@ export function registerCopy() { return ( !!focused && !!targetWorkspace && - !targetWorkspace.isReadOnly() && !targetWorkspace.isDragging() && !getFocusManager().ephemeralFocusTaken() && isCopyable(focused) @@ -179,7 +177,6 @@ export function registerCopy() { targetWorkspace.hideChaff(); } copyData = focused.toCopyData(); - copyWorkspace = targetWorkspace; copyCoords = isDraggable(focused) && focused.workspace == targetWorkspace ? focused.getRelativeToSurfaceXY() @@ -220,7 +217,6 @@ export function registerCut() { return false; } copyData = focused.toCopyData(); - copyWorkspace = workspace; copyCoords = isDraggable(focused) ? focused.getRelativeToSurfaceXY() : null; @@ -264,7 +260,11 @@ export function registerPaste() { ); }, callback(workspace: WorkspaceSvg, e: Event) { - if (!copyData || !copyWorkspace) return false; + if (!copyData) return false; + const targetWorkspace = workspace.isFlyout + ? workspace.targetWorkspace + : workspace; + if (!targetWorkspace || targetWorkspace.isReadOnly()) return false; if (e instanceof PointerEvent) { // The event that triggers a shortcut would conventionally be a KeyboardEvent. @@ -273,19 +273,19 @@ export function registerPaste() { // at the mouse coordinates where the menu was opened, and this PointerEvent // is where the menu was opened. const mouseCoords = svgMath.screenToWsCoordinates( - copyWorkspace, + targetWorkspace, new Coordinate(e.clientX, e.clientY), ); - return !!clipboard.paste(copyData, copyWorkspace, mouseCoords); + return !!clipboard.paste(copyData, targetWorkspace, mouseCoords); } if (!copyCoords) { // If we don't have location data about the original copyable, let the // paster determine position. - return !!clipboard.paste(copyData, copyWorkspace); + return !!clipboard.paste(copyData, targetWorkspace); } - const {left, top, width, height} = copyWorkspace + const {left, top, width, height} = targetWorkspace .getMetricsManager() .getViewMetrics(true); const viewportRect = new Rect(top, top + height, left, left + width); @@ -293,12 +293,12 @@ export function registerPaste() { if (viewportRect.contains(copyCoords.x, copyCoords.y)) { // If the original copyable is inside the viewport, let the paster // determine position. - return !!clipboard.paste(copyData, copyWorkspace); + return !!clipboard.paste(copyData, targetWorkspace); } // Otherwise, paste in the middle of the viewport. const centerCoords = new Coordinate(left + width / 2, top + height / 2); - return !!clipboard.paste(copyData, copyWorkspace, centerCoords); + return !!clipboard.paste(copyData, targetWorkspace, centerCoords); }, keyCodes: [ctrlV, metaV], }; diff --git a/tests/mocha/shortcut_items_test.js b/tests/mocha/shortcut_items_test.js index 622df9efc..7667ba387 100644 --- a/tests/mocha/shortcut_items_test.js +++ b/tests/mocha/shortcut_items_test.js @@ -173,12 +173,17 @@ suite('Keyboard Shortcut Items', function () { }); }); }); - // Do not copy a block if a workspace is in readonly mode. - suite('Not called when readOnly is true', function () { + // Allow copying a block if a workspace is in readonly mode. + suite('Called when readOnly is true', function () { testCases.forEach(function (testCase) { const testCaseName = testCase[0]; const keyEvent = testCase[1]; - runReadOnlyTest(keyEvent, testCaseName); + test(testCaseName, function () { + this.workspace.setIsReadOnly(true); + this.injectionDiv.dispatchEvent(keyEvent); + sinon.assert.calledOnce(this.copySpy); + sinon.assert.calledOnce(this.hideChaffSpy); + }); }); }); // Do not copy a block if a drag is in progress. @@ -238,6 +243,118 @@ suite('Keyboard Shortcut Items', function () { }); }); + suite('Cut', function () { + setup(function () { + this.block = setSelectedBlock(this.workspace); + this.copySpy = sinon.spy(this.block, 'toCopyData'); + this.disposeSpy = sinon.spy(this.block, 'dispose'); + this.hideChaffSpy = sinon.spy( + Blockly.WorkspaceSvg.prototype, + 'hideChaff', + ); + }); + const testCases = [ + [ + 'Control X', + createKeyDownEvent(Blockly.utils.KeyCodes.X, [ + Blockly.utils.KeyCodes.CTRL, + ]), + ], + [ + 'Meta X', + createKeyDownEvent(Blockly.utils.KeyCodes.X, [ + Blockly.utils.KeyCodes.META, + ]), + ], + ]; + // Cut a block. + suite('Simple', function () { + testCases.forEach(function (testCase) { + const testCaseName = testCase[0]; + const keyEvent = testCase[1]; + test(testCaseName, function () { + this.injectionDiv.dispatchEvent(keyEvent); + sinon.assert.calledOnce(this.copySpy); + sinon.assert.calledOnce(this.disposeSpy); + sinon.assert.calledOnce(this.hideChaffSpy); + }); + }); + }); + // Do not cut a block if a workspace is in readonly mode. + suite('Not called when readOnly is true', function () { + testCases.forEach(function (testCase) { + const testCaseName = testCase[0]; + const keyEvent = testCase[1]; + test(testCaseName, function () { + this.workspace.setIsReadOnly(true); + this.injectionDiv.dispatchEvent(keyEvent); + sinon.assert.notCalled(this.copySpy); + sinon.assert.notCalled(this.disposeSpy); + sinon.assert.notCalled(this.hideChaffSpy); + }); + }); + }); + // Do not cut a block if a drag is in progress. + suite('Drag in progress', function () { + testCases.forEach(function (testCase) { + const testCaseName = testCase[0]; + const keyEvent = testCase[1]; + test(testCaseName, function () { + sinon.stub(this.workspace, 'isDragging').returns(true); + this.injectionDiv.dispatchEvent(keyEvent); + sinon.assert.notCalled(this.copySpy); + sinon.assert.notCalled(this.disposeSpy); + sinon.assert.notCalled(this.hideChaffSpy); + }); + }); + }); + // Do not cut a block if is is not deletable. + suite('Block is not deletable', function () { + testCases.forEach(function (testCase) { + const testCaseName = testCase[0]; + const keyEvent = testCase[1]; + test(testCaseName, function () { + sinon + .stub(Blockly.common.getSelected(), 'isOwnDeletable') + .returns(false); + this.injectionDiv.dispatchEvent(keyEvent); + sinon.assert.notCalled(this.copySpy); + sinon.assert.notCalled(this.disposeSpy); + sinon.assert.notCalled(this.hideChaffSpy); + }); + }); + }); + // Do not cut a block if it is not movable. + suite('Block is not movable', function () { + testCases.forEach(function (testCase) { + const testCaseName = testCase[0]; + const keyEvent = testCase[1]; + test(testCaseName, function () { + sinon + .stub(Blockly.common.getSelected(), 'isOwnMovable') + .returns(false); + this.injectionDiv.dispatchEvent(keyEvent); + sinon.assert.notCalled(this.copySpy); + sinon.assert.notCalled(this.disposeSpy); + sinon.assert.notCalled(this.hideChaffSpy); + }); + }); + }); + test('Not called when connection is focused', function () { + // Restore the stub behavior called during setup + Blockly.getFocusManager().getFocusedNode.restore(); + + setSelectedConnection(this.workspace); + const event = createKeyDownEvent(Blockly.utils.KeyCodes.C, [ + Blockly.utils.KeyCodes.CTRL, + ]); + this.injectionDiv.dispatchEvent(event); + sinon.assert.notCalled(this.copySpy); + sinon.assert.notCalled(this.disposeSpy); + sinon.assert.notCalled(this.hideChaffSpy); + }); + }); + suite('Undo', function () { setup(function () { this.undoSpy = sinon.spy(this.workspace, 'undo');