Skip to content

Improve cut/copy/paste flexibility #9098

@cpcallen

Description

@cpcallen

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:

  1. Be an ICopyable, so that it is possible to get it's copy data.
  2. Be anIDraggable and IDeletable. This is reasonable as we want to ensure that any pasted copies can be manipulated/deleted.
  3. 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:

  1. 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.
  2. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    issue: feature requestDescribes a new feature and why it should be added

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions