Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handleCut editor property #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jordandh
Copy link

Add a handleCut editor property to EditorView to allow for custom cut behavior as discussed in the forums https://discuss.prosemirror.net/t/handlecut-property/1135.

Rendered RFC

@jordandh jordandh changed the title Create 0000-handleCut.md Add handleCut editor property Jan 17, 2018

# Guide-level explanation

A new property would be added to EditorProps named handleCut. It would behave similarly to other handle properties (handlePaste, handleDrop, etc). If the handleCut function returns true then prosemirror's default behavior is prevented. The handleCut function can carry out its own transaction instead of prosemirror's default behavior of deleting the current selection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it also call Event#preventDefault() (I believe this is the standard behaviour for returning true from the other handleXXX methods)?

@bradleyayers
Copy link
Contributor

Are there any utilities baked into prosemirror-view that would be helpful to export as part of this RFC?


# Motivation

The handleCut property would allow others to apply custom behavior to cut events while leveraging prosemirror's built in cross browser support for the clipboard api. One use case is for not deleting the selected content on cut but instead adding a mark to the content. For example: tracking changes made to a document by striking a line through deleted content instead of removing it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that trying to track changes by overriding every possible way of deletion and replacing it with adding marks is not a good idea. And even if you wanted to do that, it'd be much better done at the filterTransaction level than at the UI level. (Not that that invalidates this whole RFC, just that this particular use case is probably a bad example.)

The recommended way to track changes is to store steps, use prosemirror-changeset to condense them to a set of insertions and deletions, and then use decorations to show those in the document.

An example of using the handleCut property:
```
new EditorView(dom, {
handleCut: function (view, event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (view, event) signature doesn't provide access to the cut slice, or a way to modify it. I can imagine user code needing a way to influence the thing that'll be put on the clipboard. Which, I suppose, means we'd also need a handleCopy prop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I guess we already sort of have clipboardSerializer for that, though that may not always be flexible enough.

@marijnh
Copy link
Member

marijnh commented Jan 18, 2018

Are there any utilities baked into prosemirror-view that would be helpful to export as part of this RFC?

Ideally, we'd just export the tools that client code would need to fully override cut or copy events. Unfortunately, due to browser compatibility (hacks to get around lack of working clipboard access on IE and early Edge) and some subtleties in the way ProseMirror builds up clipboard HTML and stores some metdata in an attribute, doing this cleanly is tricky, and when I looked into this for handlePaste I decided to just provide a hook that's called at a point where it can have some influence on the handling without dealing with all this complexity.

I suspect the issues are the same here.

@marijnh
Copy link
Member

marijnh commented Jan 18, 2018

I'd argue that handleCut is a bit of a misnomer for a callback that can only replace the default 'remove the selection' behavior, and I'm still not 100% convinced this has a solid use case.

I'm going to let this sit a bit until I have time to properly think about it — further feedback from others would also be very welcome.

@jordandh
Copy link
Author

It seems like there are two cases here. Overriding the behavior of the clipboard data created from a cut/copy and overriding the behavior of modifying the document from a cut. Perhaps two handlers can be used. One handler (handleCopy) can be passed the clipboard slice or html and return a modified version to be put in the clipboard. A second handler (handleCut) can override just the document modification behavior. handleCopy and handleCut may or may not be the best names.

@g1xb17
Copy link

g1xb17 commented Jan 24, 2018

This is definitely not our use case to highlight stuff. I simply want to ensure for example nodes that are deleted by the selection if they leave behind the empty element that is deleted. And if it's an all selection that it ensures the schema default thats left behind is currently correct for our cases.

You could also attach some prosemirror meta like history$, you could attach cut$. We put paste on our paste transactions.

Anyway, I was more inline with thinking that the use case that we were advocating for was changing how remove selection is handled, not outright canceling it with marks.

With that said, It makes a lot more sense to do the marks at the point of origin than to try to handle generic functionality constantly on each transaction filtering through bunches of trs. Every single thing you add as a handler for 1 case is kind of annoying. Especially considering that without doing something with handleDOMEvents to try to indicate the next transaction may or may not actually be a cut event without any guarantee, going to the filter you will have to invert if you need that and check that out plus you would have to assume that was a cut. Since just "deleteSelection()" says nothing about what this delete was from, especially if your intention is to differentiate between types of deletion for some reason like @jordandh.

I want to be fair to your case because, I can see the benefit to having access in any case.

But, my case is far simpler because I have no intentions of caring about anything else with the deletion portion of cut events other than to maybe extend it to the Node instead of it's content only, and also ensure attributes on our base doc. Also, please consider we keep a node at the bottom of our doc called metadata. An AllSelection selects that and deletes it. I won't explain why we have that or what its for, just that deleting that is really really bad, and that is another reason why we need access. I'd rather not use filters to check every single tr in existence ever again if we deleted the bottom of the doc, because, it's possible some action comes from collab that does change it on purpose.

I wanted to have my 2 cents on this before it was too late since I was the other guy who wanted this.

@bradleyayers
Copy link
Contributor

handleCopy and handleCut may or may not be the best names.

Something including Clipboard might be a good idea, to avoid any confusion with existing concepts in ProseMirror.

One use case is for not deleting the selected content on cut but instead adding a mark to the content. For example: tracking changes made to a document by striking a line through deleted content instead of removing it.

I might be confused here, but I thought that in @jordandh's case he actually wanted cut and delete to do the same thing, i.e. add a mark (strike-out) to the selection and leave it in the document. If that's true, that scenario has no need to differentiate between types of deletion.

I'd rather not use filters to check every single tr in existence ever again if we deleted the bottom of the doc, because, it's possible some action comes from collab that does change it on purpose.

Interesting consideration. Could you elaborate on the problem you imagine happening with collab?

@g1xb17
Copy link

g1xb17 commented Jan 24, 2018

I cant really get into that without explaining our setup but, essentially with an all selection, it touches the node we use in the model to sync settings and other content related config type stuff between users in collab and in general people using our editor.

We are using this editor as a replacement for another editor we have using different software and it has reverse compatibility. One issue with this is that the meta node we use is something we serialize back with reverse compatibility so, if it's touched by this version of our editor with prosemirror, thats gone forever if they dont undo it, which is something thats not easily visible to the user.

It's not even specific to collab, I was just saying, we use this node to sync settings with our collab stuff. Because we have a very deeply complicated setup. These things are part of various other things that need to be preserved. There's probably someone out there itching to say something like "Why would you do things this way?" to which I say "Ever have software that's existed for 7 years before today? Thats why." Since we use this as a replacement, it dictates a lot of how we do things as well.

In general though, Never assume the solution that works for you would also work for everyone. Thats something that I would hope most people in general could appreciate about solutions in development environments. I dont mean that to be rude or anything. But, It just seems like sometimes people are dismissive of use cases for others because they can't see why anyone would ever want to do something the way they are describing it.

@jordandh
Copy link
Author

@JCHollett we actually use a similar node of meta data in our documents that is not visible to the user in the editor. I didn't even realize we had the select all delete problem until you mentioned it. I'll have to solve the same problem as well.

@g1xb17
Copy link

g1xb17 commented Jan 25, 2018

Happy to help indirectly. My original point in the main thread was for this exact problem. I thought I mentioned it. Anyway.

@marijnh
Copy link
Member

marijnh commented Jan 29, 2018

The problems caused by keeping metadata in top-level nodes that aren't rendered may be best avoided by just not doing that. Is there any reason why that data has to be in the actual document, rather than stored alongside it?

@jordandh Are you still using the approach where you mark, rather than delete, things that the user deletes? Have you figured out a way to handle typing over content, deleting through IME (i.e. virtual backspace on Android), and all the other obscure cases that can cause content to be deleted?

@jordandh
Copy link
Author

While I would like to keep the metadata out of the nodes its not an option. I am stuck with using the document format of a different system. I hoped that setting the meta data node spec selectable property to false would prevent it from being selected and therefore from being deleted. But I guess selectable doesn't work that way for a select all operation.

@marijnh I am still marking deletions instead of actually deleting them. I switched to using appendTransaction and the prosemirror-changeset module to figure out what was deleted and insert it back into the document with marks. This method is working for typing over content. I'm not sure about virtual backspace as I haven't tested it yet. As for other obscure cases, that is something I still need to investigate. I think you are already aware of my problem with tracking a change where the last item (which is empty) of a list is removed from hitting enter to exit the list. Things like paste, drag and drop, cutting, regular text input, delete/backspace at a cursor or selection are working well. Even for selections over node boundaries. Although there might be some cases where that doesn't work that I haven't run into yet.

@marijnh
Copy link
Member

marijnh commented Jan 29, 2018

I am stuck with using the document format of a different system.

I don't see how the format that some external system uses would force you to shape your ProseMirror document on the client a certain way. Surely you can feed only the relevant parts to the editor view and add back the metadata when you serialize.

But I guess selectable doesn't work that way for a select all operation.

No. As documented, this controls whether the node is node-selectable.

I switched to using appendTransaction and the prosemirror-changeset module to figure out what was deleted and insert it back into the document with marks.

Interesting. I guess this approach also covers the cut situation, maybe we don't need a special prop after all?

@jordandh
Copy link
Author

You are right that it is possible to strip the meta data out of the doc while used in a prosemirror editor. The main reason I chose not to do that is that prosemirror will merge in meta data changes from different collaborators. That means I didn't have to write extra code to do something prosemirror can already do for me. And it had worked great for me up to this point. So its certainly an option but one I'd rather avoid if I can make it work the way it is.

I don't think we need a special prop for my purposes anymore. But there may still be uses cases out there.

@jordandh
Copy link
Author

Moving the meta data outside of the document would require me to add some pre-processing to documents when collaborative editing begins. I could actually solve some other problems in a pre-processing phase. @marijnh can you let me know if you think there is no other solution to the select-all-delete problem? It would certainly help my development knowing this now. Thanks.

@marijnh
Copy link
Member

marijnh commented Jan 29, 2018

@jordandh Could you open a new thread on the forum about the metadata issue?

@Nantris
Copy link

Nantris commented May 13, 2023

Adding a handleCut would be good I think and this is currently extremely problematic for us. I can't see any way right now to shortcircuit a context-menu-initiated cut operation.

Alternatively, a way to navigate the editor while locking the content would work for us. I'm trying to implement the ability to still navigate the editor using keyboard commands without allowing document modification, and this inability to intercept cut seems to be the only case that can't be worked around.

In Slate, which we're coming from, there's an onCommand hook and when a cut occurred Slate would actually fire delete and we would just prevent that.

Is there a chance this use case might be accomodated @marijnh?

Edit: Whoops I forgot I could try cut inside of handleDOMEvents and that seems to work so far.

@Nantris
Copy link

Nantris commented Feb 20, 2024

I still think this would be worthwhile to pursue. We now have two cut handlers and without being able to return true/false to shortcircuit logic, it's quite difficult to deconflict them without needing to lump code together which shouldn't be from a logical organization standpoint.

Edit: Actually it seems like only one handler can work for cut and for copy so we seemingly have no choice but to Frankenstein them together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants