-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Description
Check for duplicates
- I have searched for similar issues before opening a new one.
Problem
At the moment the cut and copy shortcuts are fairly inflexible. In order to cut or copy a focusable, it must:
- Be an
ICopyable, so that it is possible to get it's copy data. - Be an
IDraggableandIDeletable. This is reasonable as we want to ensure that any pasted copies can be manipulated/deleted. - Return
truefrom both.isMovableand.isDeletable, for the same reason.
These criteria are currently hard-coded into the preconditionFn of the cut and copy shortcuts.
Criteria 3 is necessary because the copy will get the same .movable/.deletable bits as the source item, so if the source isn't currently movable/deletable the pasted copy won't be either.
Relatedly, because shadow blocks' .isMovable and .isDeletable always return false, making shadows non-copyable.
We know that developers would like to allow their users to copy shadow blocks (see e.g. #9084). It's possible to do so via a kludgy workaround in the shortcuts themselves, but some relatively small changes to the Blockly API allow a more general approach would be beneficial not only for this scenario but for others as well. For example:
- It should be possible to have fixed starting blocks that can be copied (e.g. Blockly Games's Maze's initial "move forward" block).
- It might be desirable that pasted copies of disabled blocks be enabled.
- It would be useful if it were possible for custom copyable items to have other attributes that differ on the copy than on the original—e.g., a unique identifier or, in a multi-user scenario, a created-by-user attribute.
Request
I propose the following changes:
- Add an
.isCopyable()method toICopyable.- To avoid this being a breaking change, the method could be made an optional member of the interface in v12, possibly being promoted to non-optional in a future major version. The cut/copy shortcuts would fall back to the current criteria for any
ICopyablenot implementing this new method.
- To avoid this being a breaking change, the method could be made an optional member of the interface in v12, possibly being promoted to non-optional in a future major version. The cut/copy shortcuts would fall back to the current criteria for any
- Provide implementations of
isCopyableonBlockandWorkspaceComment:- For backwards compatibility, the default implementation should mostly replicate the current copyability criteria, so that e.g. fixed start blocks do not become copyable unexpectedly.
- Shadow blocks's
.isCopyable()should either return a value based on.isOwnDeletable()/.isOwnMovable, or based on the copyability of their parent block.
- Modify the copy shortcut to allow any item returning
truefromisCopyableto be copied. - Modify the cut shortcut to allow any item returning
truefromisCopyableandisDeletableto be cut. - Specify that the
.toCopyData()method must ensure that the serialised version of the item is suitable for pasting—i.e., that it is movable, deletableand not a shadow block[edit: turns out that due to the way shadow state is serialised, this is already the case.]- This would not be a breaking change, as only developers providing a custom
isCopyablemethod would need to ensure the correspondingtoCopyDatamethod conforms to this rule. - But it should also be fully general, so that developers of custom
ICopyables that have.shadow-like attributes (ones that should differ on the pasted copy) are guaranteed to have a suitable place to handle that.
- This would not be a breaking change, as only developers providing a custom
Alternatives considered
The status quo is an option, though not a great one given that we know that developers want to allow users to copy shadow blocks (#9084) at least.
An alternative to including an .isCopyable() method in ICopyable would be to provide a way for developers to set a custom isCopyable(focusable: IFocusable): boolean function, to be called from the cut/copy preconditionFns. This is arguably a more general approach but would make it harder for plugin IFocusable developers to ensure that any custom copyability criteria are adhered to.
Two possible alternatives to the rule that toCopyData must return "good" pastable data (e.g. movable, deleteable, non-shadow blocks) would be:
Modify theCopyDatareturned bytoCopyDatabefore saving it.- There does not appear to be a general way to do this correctly for all types of
CopyData, especially that from customICopyablesthat are neitherBlocks norWorkspaceComments.
- There does not appear to be a general way to do this correctly for all types of
- Introduce an additional post-paste cleanup method on
ICopyableto handle fixup.- As with
isCopyable, this method could be optional to make the change non-breaking. - For blocks, the default implementation would call
.setMovable(true),.setDeletable(true)andon the pasted item. [Edit: copied blocks are serialised in a way that means the rootmost one will never be a shadow block.].setShadow(false)- Or directly set/clear the
.movable,.deletableand.shadowproperties, if we want to route around any customsetMovable/setDeletablemethods included in a block definition.
- Or directly set/clear the
- As with
Additional context
No response