Skip to content

Commit b0b685a

Browse files
authored
refactor(shortcuts): Factor copy-eligibility out of cut/copy preconditionFn (#9102)
* refactor(shortcuts): Rename import isDeletable -> isIDeletable etc. Some of the existing code is confusing to read because e.g. isDeletable doesn't check if an item .isDeletable(), but only whether it is an IDeletable. By renaming these imports the shortcut precondition functions are easier to understand, and allows a subsequent to commit to add an isCopyable function that actually checks copyability. * refactor(shortcuts): Introduce isCopyable Create a function, isCopyable, that encapsulates the criteria we currently use to determine whether an item can be copied. This facilitate future modification of the copyability criteria (but is not intended to modify them at all at the present time). * chore(shortcuts): Add TODO re: copying shadow blocks
1 parent d5a4522 commit b0b685a

File tree

1 file changed

+54
-19
lines changed

1 file changed

+54
-19
lines changed

core/shortcut_items.ts

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,17 @@ import {BlockSvg} from './block_svg.js';
1010
import * as clipboard from './clipboard.js';
1111
import * as eventUtils from './events/utils.js';
1212
import {Gesture} from './gesture.js';
13-
import {ICopyData, isCopyable} from './interfaces/i_copyable.js';
14-
import {isDeletable} from './interfaces/i_deletable.js';
15-
import {isDraggable} from './interfaces/i_draggable.js';
13+
import {
14+
ICopyable,
15+
ICopyData,
16+
isCopyable as isICopyable,
17+
} from './interfaces/i_copyable.js';
18+
import {
19+
IDeletable,
20+
isDeletable as isIDeletable,
21+
} from './interfaces/i_deletable.js';
22+
import {IDraggable, isDraggable} from './interfaces/i_draggable.js';
23+
import {IFocusableNode} from './interfaces/i_focusable_node.js';
1624
import {KeyboardShortcut, ShortcutRegistry} from './shortcut_registry.js';
1725
import {Coordinate} from './utils/coordinate.js';
1826
import {KeyCodes} from './utils/keycodes.js';
@@ -62,7 +70,7 @@ export function registerDelete() {
6270
return (
6371
!workspace.isReadOnly() &&
6472
focused != null &&
65-
isDeletable(focused) &&
73+
isIDeletable(focused) &&
6674
focused.isDeletable() &&
6775
!Gesture.inProgress()
6876
);
@@ -76,7 +84,7 @@ export function registerDelete() {
7684
const focused = scope.focusedNode;
7785
if (focused instanceof BlockSvg) {
7886
focused.checkAndDelete();
79-
} else if (isDeletable(focused) && focused.isDeletable()) {
87+
} else if (isIDeletable(focused) && focused.isDeletable()) {
8088
eventUtils.setGroup(true);
8189
focused.dispose();
8290
eventUtils.setGroup(false);
@@ -92,6 +100,39 @@ let copyData: ICopyData | null = null;
92100
let copyWorkspace: WorkspaceSvg | null = null;
93101
let copyCoords: Coordinate | null = null;
94102

103+
/**
104+
* Determine if a focusable node can be copied using cut or copy.
105+
*
106+
* Unfortunately the ICopyable interface doesn't include an isCopyable
107+
* method, so we must use some other criteria to make the decision.
108+
* Specifically,
109+
*
110+
* - It must be an ICopyable.
111+
* - So that a pasted copy can be manipluated and/or disposed of, it
112+
* must be both an IDraggable and an IDeletable.
113+
* - Additionally, both .isMovable() and .isDeletable() must return
114+
* true (i.e., can currently be moved and deleted).
115+
*
116+
* TODO(#9098): Revise these criteria. The latter criteria prevents
117+
* shadow blocks from being copied; additionally, there are likely to
118+
* be other circumstances were it is desirable to allow movable /
119+
* copyable copies of a currently-unmovable / -copyable block to be
120+
* made.
121+
*
122+
* @param focused The focused object.
123+
*/
124+
function isCopyable(
125+
focused: IFocusableNode,
126+
): focused is ICopyable<ICopyData> & IDeletable & IDraggable {
127+
return (
128+
isICopyable(focused) &&
129+
isIDeletable(focused) &&
130+
focused.isDeletable() &&
131+
isDraggable(focused) &&
132+
focused.isMovable()
133+
);
134+
}
135+
95136
/**
96137
* Keyboard shortcut to copy a block on ctrl+c, cmd+c, or alt+c.
97138
*/
@@ -110,11 +151,7 @@ export function registerCopy() {
110151
return (
111152
!workspace.isReadOnly() &&
112153
!Gesture.inProgress() &&
113-
focused != null &&
114-
isDeletable(focused) &&
115-
focused.isDeletable() &&
116-
isDraggable(focused) &&
117-
focused.isMovable() &&
154+
!!focused &&
118155
isCopyable(focused)
119156
);
120157
},
@@ -124,7 +161,7 @@ export function registerCopy() {
124161
e.preventDefault();
125162
workspace.hideChaff();
126163
const focused = scope.focusedNode;
127-
if (!focused || !isCopyable(focused)) return false;
164+
if (!focused || !isICopyable(focused)) return false;
128165
copyData = focused.toCopyData();
129166
copyWorkspace =
130167
focused.workspace instanceof WorkspaceSvg
@@ -158,13 +195,11 @@ export function registerCut() {
158195
return (
159196
!workspace.isReadOnly() &&
160197
!Gesture.inProgress() &&
161-
focused != null &&
162-
isDeletable(focused) &&
163-
focused.isDeletable() &&
164-
isDraggable(focused) &&
165-
focused.isMovable() &&
198+
!!focused &&
166199
isCopyable(focused) &&
167-
!focused.workspace.isFlyout
200+
// Extra criteria for cut (not just copy):
201+
!focused.workspace.isFlyout &&
202+
focused.isDeletable()
168203
);
169204
},
170205
callback(workspace, e, shortcut, scope) {
@@ -177,9 +212,9 @@ export function registerCut() {
177212
focused.checkAndDelete();
178213
return true;
179214
} else if (
180-
isDeletable(focused) &&
215+
isIDeletable(focused) &&
181216
focused.isDeletable() &&
182-
isCopyable(focused)
217+
isICopyable(focused)
183218
) {
184219
copyData = focused.toCopyData();
185220
copyWorkspace = workspace;

0 commit comments

Comments
 (0)