Check for duplicates
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
IDraggable and IDeletable. This is reasonable as we want to ensure that any pasted copies can be manipulated/deleted.
- Return
true from both .isMovable and .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 to ICopyable.
- 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
ICopyable not implementing this new method.
- Provide implementations of
isCopyable on Block and WorkspaceComment:
- 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
true from isCopyable to be copied.
- Modify the cut shortcut to allow any item returning
true from isCopyable and isDeletable to 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, deletable and 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
isCopyable method would need to ensure the corresponding toCopyData method 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.
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 the CopyData returned by toCopyData before saving it.
- There does not appear to be a general way to do this correctly for all types of
CopyData, especially that from custom ICopyables that are neither Blocks nor WorkspaceComments.
- Introduce an additional post-paste cleanup method on
ICopyable to 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) and .setShadow(false) on the pasted item. [Edit: copied blocks are serialised in a way that means the rootmost one will never be a shadow block.]
- Or directly set/clear the
.movable, .deletable and .shadow properties, if we want to route around any custom setMovable/setDeletable methods included in a block definition.
Additional context
No response
Check for duplicates
Problem
At the moment the cut and copy shortcuts are fairly inflexible. In order to cut or copy a focusable, it must:
ICopyable, so that it is possible to get it's copy data.IDraggableandIDeletable. This is reasonable as we want to ensure that any pasted copies can be manipulated/deleted.truefrom both.isMovableand.isDeletable, for the same reason.These criteria are currently hard-coded into the
preconditionFnof the cut and copy shortcuts.Criteria 3 is necessary because the copy will get the same
.movable/.deletablebits as the source item, so if the source isn't currently movable/deletable the pasted copy won't be either.Relatedly, because shadow blocks'
.isMovableand.isDeletablealways 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:
Request
I propose the following changes:
.isCopyable()method toICopyable.ICopyablenot implementing this new method.isCopyableonBlockandWorkspaceComment:.isCopyable()should either return a value based on.isOwnDeletable()/.isOwnMovable, or based on the copyability of their parent block.truefromisCopyableto be copied.truefromisCopyableandisDeletableto be cut..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.]isCopyablemethod would need to ensure the correspondingtoCopyDatamethod conforms to this rule.ICopyables that have.shadow-like attributes (ones that should differ on the pasted copy) are guaranteed to have a suitable place to handle that.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 inICopyablewould be to provide a way for developers to set a customisCopyable(focusable: IFocusable): booleanfunction, to be called from the cut/copypreconditionFns. This is arguably a more general approach but would make it harder for pluginIFocusabledevelopers to ensure that any custom copyability criteria are adhered to.Two possible alternatives to the rule that
toCopyDatamust return "good" pastable data (e.g. movable, deleteable, non-shadow blocks) would be:Modify theCopyDatareturned bytoCopyDatabefore saving it.CopyData, especially that from customICopyablesthat are neitherBlocks norWorkspaceComments.ICopyableto handle fixup.isCopyable, this method could be optional to make the change non-breaking..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).movable,.deletableand.shadowproperties, if we want to route around any customsetMovable/setDeletablemethods included in a block definition.Additional context
No response