ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

How do we react to drag events if they don't bubble up?

Open dKustura opened this issue 1 year ago • 3 comments

If we want custom logic that will check data that is being dragged over the CK Editor, and perform custom actions based on that data, how do we do that now when the event bubbling is stopped? This worked before v40.0.0 before this piece of code was introduced:

https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-clipboard/src/dragdrop.ts#L372

The problem I see here is that the logic is based on an assumption that all data will either be handled completely by the CK Editor or completely by external code. There is no in-between.

Related issues:

  • https://github.com/ckeditor/ckeditor5/issues/2837
  • https://github.com/ckeditor/ckeditor5/issues/6464

What I'm asking is actually a way to control the editor to behave completely opposite of what https://github.com/ckeditor/ckeditor5/issues/6464 requested.

dKustura avatar Aug 09 '24 15:08 dKustura

This was introduced while working on the drag and drop of blocks in the editor (https://github.com/ckeditor/ckeditor5/pull/14000 first as an experimental feature and then promoted to the base feature).

https://github.com/ckeditor/ckeditor5/blob/5c870eb3a3df4de878f5faf53c6edba938056ef0/packages/ckeditor5-clipboard/src/dragdrop.ts#L372

The above line is not code commented and not covered with tests. This might be related to some intermediate state of the feature PoC but I'm not sure now. It could also be related to some optimization, but I am guessing now. I briefly checked how the editor behaves if that line is removed and It looks like all works correctly and all tests are passing.

Let's remove the evt.stop() call in the dragover event handling.

niegowski avatar Aug 14 '24 14:08 niegowski

When can we expect this in a new version?

dKustura avatar Aug 14 '24 15:08 dKustura

To give some more context to this problem, the line of code calling evt.stop() in dragdrop.ts works in conjunction with the following piece of code in clipboardobserver.ts:

  • https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-clipboard/src/clipboardobserver.ts#L59
  • https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-clipboard/src/clipboardobserver.ts#L80 So when the evt.stop() is called it marks the event.stop.called as true, and the handleInput function stops event propagation.

The event.stopPropagation() call was initially added because of https://github.com/ckeditor/ckeditor5-upload/issues/92. It's debatable if the reporter should have solved the image upload problem inside their own code, because now all clipboard events are stopped from propagating. Actually, if I understand correctly, all clipboard dragover events will be stopped except for some file types that are not handled by the CKEditor.

To add more confusion to this problem, if add this piece of code into my CKEditor setup

editor.editing.view.document.on('dragover', (event) => {
    event.stop()
})

It will actually not stop event propagation because handleInput will never get called, so I will be able to handle the event outside of CKEditor, but then the dragdrop plugin won't receive the event so it won't behave properly. 😅

Whatever you decide to do (leave the code as is or change it) I'd say it's a breaking change.

dKustura avatar Aug 16 '24 09:08 dKustura

Also ran into this issue today as I wondered why my drag & drop functionality in Neos CMS didn't trigger various drag events when moving content elements.

The inline editor also reacts to my dragged element by showing the drop line, which it shouldn't as it is something that should not and cannot be dragged into the text.

So it would be great if I could tell what CK ignores and it shouldn't stop the bubble up, when not even handling the event.

So for now, I will remove the evt.stop via patch and see if things work fine without.

Sebobo avatar Aug 07 '25 11:08 Sebobo