T/29: Support for drop positions #30
Conversation
|
|
||
| doc.fire( 'clipboardInput', { | ||
| dataTransfer: data.dataTransfer, | ||
| targetRanges |
There was a problem hiding this comment.
Hm... I'm more and more not sure about it. After paste, the position should where the selection.focus is. It means that ranges might be not enough. Also, what is more important, after paste, the selection should be collapsed, after drop not. It means that maybe these events should be handled separately and the common event does not make much sense?
There was a problem hiding this comment.
Note that there is still common inputTransformation for the input transformation, so I don't think we need clipboardInput.
There was a problem hiding this comment.
Hm... good points. I'll think on this.
There was a problem hiding this comment.
I looked into the code and I still think that clipboardInput is quite useful. It's where all the handling should be. Even the image upload feature uses this event. If not for it, you'd need to add 2 listeners. The same in the clipboard feature itself – it'd need two listeners.
The only thing is that we want to select the whole content after drop happened. But this is far down inside inputTransformation which was meant to be that "single pipe" anyway, so getting rid of clipboardInput won't help with that at all.
One thing which I don't like here is that inputTransformation does content insertion which is wrong. So I think that this requires a bigger refactoring anyway so it's not a part of this ticket anyway.
There was a problem hiding this comment.
|
I tested it manually and it works very well. |
src/clipboardobserver.js
Outdated
| * Fired with a `dataTransfer` which comes from the clipboard (was {@link module:engine/view/document~Document#event:paste pasted} | ||
| * or {@link module:engine/view/document~Document#event:drop dropped}) and | ||
| * should be processed in order to be inserted into the editor. | ||
| * Fired as a continuation of {@link #event:paste} amd {@link #event:drop} events. |
Not exactly – the selection should be after the pasted content. So it's the initial position modified by |
|
If we'll agree that we can stay with |
Suggested merge commit message (convention)
Feature: Added
dropRangeto thedropevent andtargetRangesto theclipboardInputevent. Closes ckeditor/ckeditor5#2668.Additional information
As you may notice the full d&d support misses just two things now – using the actual drop position (and not the selection) at the end of clipboard input pipeline. Right now the selection is used... so you don't see any changes when d&ding within the editor. The second thing is deleting the dragged content from its original position. But I didn't want to make this PR any bigger now, so this will come some other day.
Also, there's Edge support which I didn't check at all after @Mgsy showed me that Edge doesn't even try supporting dropping files and displays a "no go" icon.
One more thing – I've been thinking about renaming
drop'sdropRangetodropPositionbecause it seemed to me that it will always be a position, but it's not necessarily true. E.g. we may want to support dropping on object type elements (like image widgets) in a way that it will be replacing the image and not inserting the content before/after it. This will come up once we'll start working on the magicline stuff.