Fix Copy/pasting void elements is not working

[laufeyrut]

Description This PR makes it possible to copy/cut void elements.

Issue Fixes: https://github.com/ianstormtaylor/slate/issues/4808 and https://github.com/ianstormtaylor/slate/issues/4806

Example Before After:

Context Created a new function hasSelectableTarget that checks if hasEditableTarget and isTargetInsideNonReadonlyVoid. Using this function instead of hasEditableTarget so we are not excluding nonReadonlyVoids fx onCopy and onCut and onPaste

I did not add Cypress tests for the new logic since Cypress does not support copy/paste and the only solution I found was to use some package like https://www.npmjs.com/package/clipboardy. If we want to include the package or if there is a better solution for it I'm happy to add the tests.

Checks

• [x] The new code matches the existing patterns and styles.
• [x] The tests pass with yarn test.
• [x] The linter passes with yarn lint. (Fix errors with yarn fix.)
• [x] The relevant examples still work. (Run examples with yarn start.)
• [x] You've added a changeset if changing functionality. (Add one with yarn changeset add.)

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 03234e5ea7eaa2014911c83cd9b9a278830fdc34

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
slate-react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[laufeyrut]

@dylans After reading your comment I did some more testing and found one bug in my approach regarding editing the editable voids, I fixed that and added a cypress test for it.

But I see the issue with copy-pasting inside the editable voids and I'm not sure how it is best to solve that 😞 Do you have any ideas?

[SmilinBrian]

Looking forward to this one as it will solve some problems for our Slate use. We have a number of different void nodes that users expect to be able to Copy and Paste while the void is showing as 'selected'.

I'm not sure I understand what the 'blocking' issue is… is it that you might have an <input type=text> inside the void?

[dylans]

Looking forward to this one as it will solve some problems for our Slate use. We have a number of different void nodes that users expect to be able to Copy and Paste while the void is showing as 'selected'.

I'm not sure I understand what the 'blocking' issue is… is it that you might have an <input type=text> inside the void?

Yes, for example in Plate, we have a void that contains an Excalidraw drawing inside it. I need time to think through how to make this not break that scenario.

[SmilinBrian]

I'm not sure I understand what the 'blocking' issue is… is it that you might have an <input type=text> inside the void?

Yes, for example in Plate, we have a void that contains an Excalidraw drawing inside it. I need time to think through how to make this not break that scenario.

I see. I wonder if it would make sense for withReact(editor) to add hasSelectableTarget() and/or hasEditableTarget() to the editor, and that way customizers / plug-ins could override them to fine-tune the decision about whether or not "Slate" should try to deal with event X?

[evasteingrims]

Looking forward to this one as it will solve some problems for our Slate use. We have a number of different void nodes that users expect to be able to Copy and Paste while the void is showing as 'selected'. I'm not sure I understand what the 'blocking' issue is… is it that you might have an <input type=text> inside the void?

Yes, for example in Plate, we have a void that contains an Excalidraw drawing inside it. I need time to think through how to make this not break that scenario.

Hi @dylans. Have you had some time to think about this?

[dylans]

Looking forward to this one as it will solve some problems for our Slate use. We have a number of different void nodes that users expect to be able to Copy and Paste while the void is showing as 'selected'. I'm not sure I understand what the 'blocking' issue is… is it that you might have an <input type=text> inside the void?

Yes, for example in Plate, we have a void that contains an Excalidraw drawing inside it. I need time to think through how to make this not break that scenario.

Hi @dylans. Have you had some time to think about this?

I have but I don't have a good answer yet. Need a few more days to try out other examples and see the potential consequences of this.

[zbeyens]

I see. I wonder if it would make sense for withReact(editor) to add hasSelectableTarget() and/or hasEditableTarget() to the editor, and that way customizers / plug-ins could override them to fine-tune the decision about whether or not "Slate" should try to deal with event X?

Definitely this. In Plate I would have added a plugin field that handles copy/paste onKeyDown events so this behavior should be controllable.

The question here is, should we copy the void node by default? I'd say yes as it's the most common case, just note that would be a breaking change.

[dylans]

I see. I wonder if it would make sense for withReact(editor) to add hasSelectableTarget() and/or hasEditableTarget() to the editor, and that way customizers / plug-ins could override them to fine-tune the decision about whether or not "Slate" should try to deal with event X?

Definitely this. In Plate I would have added a plugin field that handles copy/paste onKeyDown events so this behavior should be controllable.

The question here is, should we copy the void node by default? I'd say yes as it's the most common case, just note that would be a breaking change.

This is one way I was roughly thinking could work. What do you think @laufeyrut?

[laufeyrut]

I see. I wonder if it would make sense for withReact(editor) to add hasSelectableTarget() and/or hasEditableTarget() to the editor, and that way customizers / plug-ins could override them to fine-tune the decision about whether or not "Slate" should try to deal with event X?

Definitely this. In Plate I would have added a plugin field that handles copy/paste onKeyDown events so this behavior should be controllable. The question here is, should we copy the void node by default? I'd say yes as it's the most common case, just note that would be a breaking change.

This is one way I was roughly thinking could work. What do you think @laufeyrut?

Yes, I think this could work! I don't know of a better idea. @dylans I'm not sure how that would be implemented though

I see. I wonder if it would make sense for withReact(editor) to add hasSelectableTarget() and/or hasEditableTarget() to the editor, and that way customizers / plug-ins could override them to fine-tune the decision about whether or not "Slate" should try to deal with event X?

Definitely this. In Plate I would have added a plugin field that handles copy/paste onKeyDown events so this behavior should be controllable.

The question here is, should we copy the void node by default? I'd say yes as it's the most common case, just note that would be a breaking change.

I think being able to copy the void nodes should be the default behavior 🙏

[alex-vladut]

Hey @laufeyrut great job on adding these fixes in place 👍 I looked at extracting those methods to be easily overridable from withReact and you can find the changes on this branch https://github.com/ianstormtaylor/slate/compare/main...alex-vladut:slate:fix-copy-editable-voids?expand=1. What I did was mainly to move the methods into the ReactEditor and reference them from there. Let me know if you think it makes sense and maybe you can incorporate those changes into this PR to get some feedback

[laufeyrut]

Hey @laufeyrut great job on adding these fixes in place 👍 I looked at extracting those methods to be easily overridable from withReact and you can find the changes on this branch https://github.com/ianstormtaylor/slate/compare/main...alex-vladut:slate:fix-copy-editable-voids?expand=1. What I did was mainly to move the methods into the ReactEditor and reference them from there. Let me know if you think it makes sense and maybe you can incorporate those changes into this PR to get some feedback

I really like this change, Thank you for the help @alex-vladut 🙏

@dylans I look forward to hearing what you think about this