diff --git a/core/focus_manager.ts b/core/focus_manager.ts index 01be4813f..3d0a9347f 100644 --- a/core/focus_manager.ts +++ b/core/focus_manager.ts @@ -458,6 +458,13 @@ export class FocusManager { }; } + /** + * @returns whether something is currently holding ephemeral focus + */ + ephemeralFocusTaken(): boolean { + return this.currentlyHoldsEphemeralFocus; + } + /** * Ensures that the manager is currently allowing operations that change its * internal focus state (such as via focusNode()). diff --git a/core/layer_manager.ts b/core/layer_manager.ts index 1d5afdd74..fd7d8fe23 100644 --- a/core/layer_manager.ts +++ b/core/layer_manager.ts @@ -104,9 +104,11 @@ export class LayerManager { moveToDragLayer(elem: IRenderedElement & IFocusableNode) { this.dragLayer?.appendChild(elem.getSvgRoot()); - // Since moving the element to the drag layer will cause it to lose focus, - // ensure it regains focus (to ensure proper highlights & sent events). - getFocusManager().focusNode(elem); + if (elem.canBeFocused()) { + // Since moving the element to the drag layer will cause it to lose focus, + // ensure it regains focus (to ensure proper highlights & sent events). + getFocusManager().focusNode(elem); + } } /** @@ -117,9 +119,11 @@ export class LayerManager { moveOffDragLayer(elem: IRenderedElement & IFocusableNode, layerNum: number) { this.append(elem, layerNum); - // Since moving the element off the drag layer will cause it to lose focus, - // ensure it regains focus (to ensure proper highlights & sent events). - getFocusManager().focusNode(elem); + if (elem.canBeFocused()) { + // Since moving the element off the drag layer will cause it to lose focus, + // ensure it regains focus (to ensure proper highlights & sent events). + getFocusManager().focusNode(elem); + } } /** diff --git a/core/shortcut_items.ts b/core/shortcut_items.ts index c175637c6..161d5fceb 100644 --- a/core/shortcut_items.ts +++ b/core/shortcut_items.ts @@ -9,6 +9,7 @@ import {BlockSvg} from './block_svg.js'; import * as clipboard from './clipboard.js'; import * as eventUtils from './events/utils.js'; +import {getFocusManager} from './focus_manager.js'; import {Gesture} from './gesture.js'; import { ICopyable, @@ -72,7 +73,9 @@ export function registerDelete() { focused != null && isIDeletable(focused) && focused.isDeletable() && - !Gesture.inProgress() + !Gesture.inProgress() && + // Don't delete the block if a field editor is open + !getFocusManager().ephemeralFocusTaken() ); }, callback(workspace, e, shortcut, scope) { @@ -101,7 +104,7 @@ let copyWorkspace: WorkspaceSvg | null = null; let copyCoords: Coordinate | null = null; /** - * Determine if a focusable node can be copied using cut or copy. + * Determine if a focusable node can be copied. * * Unfortunately the ICopyable interface doesn't include an isCopyable * method, so we must use some other criteria to make the decision. @@ -110,8 +113,8 @@ let copyCoords: Coordinate | null = null; * - It must be an ICopyable. * - So that a pasted copy can be manipluated and/or disposed of, it * must be both an IDraggable and an IDeletable. - * - Additionally, both .isMovable() and .isDeletable() must return - * true (i.e., can currently be moved and deleted). + * - Additionally, both .isOwnMovable() and .isOwnDeletable() must return + * true (i.e., the copy could be moved and deleted). * * TODO(#9098): Revise these criteria. The latter criteria prevents * shadow blocks from being copied; additionally, there are likely to @@ -124,6 +127,40 @@ let copyCoords: Coordinate | null = null; function isCopyable( focused: IFocusableNode, ): focused is ICopyable & IDeletable & IDraggable { + if (!(focused instanceof BlockSvg)) return false; + return ( + isICopyable(focused) && + isIDeletable(focused) && + focused.isOwnDeletable() && + isDraggable(focused) && + focused.isOwnMovable() + ); +} + +/** + * Determine if a focusable node can be cut. + * + * Unfortunately the ICopyable interface doesn't include an isCuttable + * method, so we must use some other criteria to make the decision. + * Specifically, + * + * - It must be an ICopyable. + * - So that a pasted copy can be manipluated and/or disposed of, it + * must be both an IDraggable and an IDeletable. + * - Additionally, both .isMovable() and .isDeletable() must return + * true (i.e., can currently be moved and deleted). This is the main + * difference with isCopyable. + * + * TODO(#9098): Revise these criteria. The latter criteria prevents + * shadow blocks from being copied; additionally, there are likely to + * be other circumstances were it is desirable to allow movable / + * copyable copies of a currently-unmovable / -copyable block to be + * made. + * + * @param focused The focused object. + */ +function isCuttable(focused: IFocusableNode): boolean { + if (!(focused instanceof BlockSvg)) return false; return ( isICopyable(focused) && isIDeletable(focused) && @@ -148,10 +185,17 @@ export function registerCopy() { name: names.COPY, preconditionFn(workspace, scope) { const focused = scope.focusedNode; + if (!(focused instanceof BlockSvg)) return false; + + const targetWorkspace = workspace.isFlyout + ? workspace.targetWorkspace + : workspace; return ( - !workspace.isReadOnly() && - !Gesture.inProgress() && !!focused && + !!targetWorkspace && + !targetWorkspace.isReadOnly() && + !targetWorkspace.isDragging() && + !getFocusManager().ephemeralFocusTaken() && isCopyable(focused) ); }, @@ -159,17 +203,27 @@ export function registerCopy() { // Prevent the default copy behavior, which may beep or otherwise indicate // an error due to the lack of a selection. e.preventDefault(); - workspace.hideChaff(); + const focused = scope.focusedNode; - if (!focused || !isICopyable(focused)) return false; - copyData = focused.toCopyData(); - copyWorkspace = + if (!focused || !isCopyable(focused)) return false; + let targetWorkspace: WorkspaceSvg | null = focused.workspace instanceof WorkspaceSvg ? focused.workspace : workspace; - copyCoords = isDraggable(focused) - ? focused.getRelativeToSurfaceXY() - : null; + targetWorkspace = targetWorkspace.isFlyout + ? targetWorkspace.targetWorkspace + : targetWorkspace; + if (!targetWorkspace) return false; + + if (!focused.workspace.isFlyout) { + targetWorkspace.hideChaff(); + } + copyData = focused.toCopyData(); + copyWorkspace = targetWorkspace; + copyCoords = + isDraggable(focused) && focused.workspace == targetWorkspace + ? focused.getRelativeToSurfaceXY() + : null; return !!copyData; }, keyCodes: [ctrlC, metaC], @@ -193,13 +247,11 @@ export function registerCut() { preconditionFn(workspace, scope) { const focused = scope.focusedNode; return ( - !workspace.isReadOnly() && - !Gesture.inProgress() && !!focused && - isCopyable(focused) && - // Extra criteria for cut (not just copy): - !focused.workspace.isFlyout && - focused.isDeletable() + !workspace.isReadOnly() && + !workspace.isDragging() && + !getFocusManager().ephemeralFocusTaken() && + isCuttable(focused) ); }, callback(workspace, e, shortcut, scope) { @@ -246,7 +298,16 @@ export function registerPaste() { const pasteShortcut: KeyboardShortcut = { name: names.PASTE, preconditionFn(workspace) { - return !workspace.isReadOnly() && !Gesture.inProgress(); + const targetWorkspace = workspace.isFlyout + ? workspace.targetWorkspace + : workspace; + return ( + !!copyData && + !!targetWorkspace && + !targetWorkspace.isReadOnly() && + !targetWorkspace.isDragging() && + !getFocusManager().ephemeralFocusTaken() + ); }, callback(workspace: WorkspaceSvg, e: Event) { if (!copyData || !copyWorkspace) return false; @@ -305,7 +366,11 @@ export function registerUndo() { const undoShortcut: KeyboardShortcut = { name: names.UNDO, preconditionFn(workspace) { - return !workspace.isReadOnly() && !Gesture.inProgress(); + return ( + !workspace.isReadOnly() && + !Gesture.inProgress() && + !getFocusManager().ephemeralFocusTaken() + ); }, callback(workspace, e) { // 'z' for undo 'Z' is for redo. @@ -340,7 +405,11 @@ export function registerRedo() { const redoShortcut: KeyboardShortcut = { name: names.REDO, preconditionFn(workspace) { - return !Gesture.inProgress() && !workspace.isReadOnly(); + return ( + !Gesture.inProgress() && + !workspace.isReadOnly() && + !getFocusManager().ephemeralFocusTaken() + ); }, callback(workspace, e) { // 'z' for undo 'Z' is for redo. diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 5d5a40ccc..9eb5ea545 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2806,10 +2806,11 @@ export class WorkspaceSvg /** See IFocusableTree.onTreeBlur. */ onTreeBlur(nextTree: IFocusableTree | null): void { // If the flyout loses focus, make sure to close it unless focus is being - // lost to the toolbox. + // lost to the toolbox or ephemeral focus. if (this.isFlyout && this.targetWorkspace) { // Only hide the flyout if the flyout's workspace is losing focus and that - // focus isn't returning to the flyout itself or the toolbox. + // focus isn't returning to the flyout itself, the toolbox, or ephemeral. + if (getFocusManager().ephemeralFocusTaken()) return; const flyout = this.targetWorkspace.getFlyout(); const toolbox = this.targetWorkspace.getToolbox(); if (toolbox && nextTree === toolbox) return; diff --git a/tests/mocha/shortcut_items_test.js b/tests/mocha/shortcut_items_test.js index 29487ec58..622df9efc 100644 --- a/tests/mocha/shortcut_items_test.js +++ b/tests/mocha/shortcut_items_test.js @@ -181,13 +181,13 @@ suite('Keyboard Shortcut Items', function () { runReadOnlyTest(keyEvent, testCaseName); }); }); - // Do not copy a block if a gesture is in progress. - suite('Gesture in progress', function () { + // Do not copy 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(Blockly.Gesture, 'inProgress').returns(true); + sinon.stub(this.workspace, 'isDragging').returns(true); this.injectionDiv.dispatchEvent(keyEvent); sinon.assert.notCalled(this.copySpy); sinon.assert.notCalled(this.hideChaffSpy); @@ -201,7 +201,7 @@ suite('Keyboard Shortcut Items', function () { const keyEvent = testCase[1]; test(testCaseName, function () { sinon - .stub(Blockly.common.getSelected(), 'isDeletable') + .stub(Blockly.common.getSelected(), 'isOwnDeletable') .returns(false); this.injectionDiv.dispatchEvent(keyEvent); sinon.assert.notCalled(this.copySpy); @@ -215,7 +215,9 @@ suite('Keyboard Shortcut Items', function () { const testCaseName = testCase[0]; const keyEvent = testCase[1]; test(testCaseName, function () { - sinon.stub(Blockly.common.getSelected(), 'isMovable').returns(false); + sinon + .stub(Blockly.common.getSelected(), 'isOwnMovable') + .returns(false); this.injectionDiv.dispatchEvent(keyEvent); sinon.assert.notCalled(this.copySpy); sinon.assert.notCalled(this.hideChaffSpy);