Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/29: Support for drop positions #30

Merged
merged 5 commits into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,8 @@
"@ckeditor/ckeditor5-block-quote": "^0.1.1",
"@ckeditor/ckeditor5-editor-classic": "^0.7.3",
"@ckeditor/ckeditor5-paragraph": "^0.8.0",
"@ckeditor/ckeditor5-enter": "^0.9.1",
"@ckeditor/ckeditor5-heading": "^0.9.1",
"@ckeditor/ckeditor5-presets": "^0.2.2",
"@ckeditor/ckeditor5-link": "^0.7.0",
"@ckeditor/ckeditor5-list": "^0.6.1",
"@ckeditor/ckeditor5-typing": "^0.9.1",
"@ckeditor/ckeditor5-undo": "^0.8.1",
"eslint-config-ckeditor5": "^1.0.0",
"gulp": "^3.9.1",
"guppy-pre-commit": "^0.4.0"
Expand Down
61 changes: 53 additions & 8 deletions src/clipboardobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,72 @@ export default class ClipboardObserver extends DomEventObserver {
function handleInput( evt, data ) {
data.preventDefault();

doc.fire( 'clipboardInput', { dataTransfer: data.dataTransfer } );
const targetRanges = data.dropRange ? [ data.dropRange ] : Array.from( doc.selection.getRanges() );

doc.fire( 'clipboardInput', {
dataTransfer: data.dataTransfer,
targetRanges
Copy link

Choose a reason for hiding this comment

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

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?

Copy link

Choose a reason for hiding this comment

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

Note that there is still common inputTransformation for the input transformation, so I don't think we need clipboardInput.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm... good points. I'll think on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

} );
}
}

onDomEvent( domEvent ) {
this.fire( domEvent.type, domEvent, {
const evtData = {
dataTransfer: new DataTransfer( domEvent.clipboardData ? domEvent.clipboardData : domEvent.dataTransfer )
} );
};

if ( domEvent.type == 'drop' ) {
evtData.dropRange = getDropViewRange( this.document, domEvent );
}

this.fire( domEvent.type, domEvent, evtData );
}
}

function getDropViewRange( doc, domEvent ) {
const domDoc = domEvent.target.ownerDocument;
const x = domEvent.clientX;
const y = domEvent.clientY;
let domRange;

// Webkit & Blink.
if ( domDoc.caretRangeFromPoint && domDoc.caretRangeFromPoint( x, y ) ) {
domRange = domDoc.caretRangeFromPoint( x, y );
}
// FF.
else if ( domEvent.rangeParent ) {
domRange = domDoc.createRange();
domRange.setStart( domEvent.rangeParent, domEvent.rangeOffset );
domRange.collapse( true );
}

if ( domRange ) {
return doc.domConverter.domRangeToView( domRange );
} else {
return doc.selection.getFirstRange();
}
}

/**
* 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} and {@link #event:drop} events.
* It's part of the {@link module:clipboard/clipboard~Clipboard "clipboard pipeline"}.
*
* Fired with a `dataTransfer` which comes from the clipboard and which content should be processed
* and inserted into the editor.
*
* Note that this event is not available by default. To make it available {@link module:clipboard/clipboardobserver~ClipboardObserver}
* needs to be added to {@link module:engine/view/document~Document} by the {@link module:engine/view/document~Document#addObserver} method.
* It's done by the {@link module:clipboard/clipboard~Clipboard} feature. If it's not loaded, it must be done manually.
*
* @see module:clipboard/clipboardobserver~ClipboardObserver
* @see module:clipboard/clipboard~Clipboard
* @event module:engine/view/document~Document#event:clipboardInput
* @param {Object} data Event data.
* @param {module:clipboard/datatransfer~DataTransfer} data.dataTransfer Data transfer instance.
* @param {Array.<module:engine/view/range~Range>} data.targetRanges Ranges which are the target of the operation
* (usually – into which the content should be inserted).
* If clipboard input was triggered by a paste operation, then these are the selection ranges. If by a drop operation,
* then it's the drop position (which can be different than the selection at the moment of drop).
*/

/**
Expand All @@ -63,9 +107,10 @@ export default class ClipboardObserver extends DomEventObserver {
* needs to be added to {@link module:engine/view/document~Document} by the {@link module:engine/view/document~Document#addObserver} method.
* It's done by the {@link module:clipboard/clipboard~Clipboard} feature. If it's not loaded, it must be done manually.
*
* @see module:clipboard/clipboardobserver~ClipboardObserver
* @see module:engine/view/document~Document#event:clipboardInput
* @event module:engine/view/document~Document#event:drop
* @param {module:clipboard/clipboardobserver~ClipboardEventData} data Event data.
* @param {module:engine/view/range~Range} dropRange The position into which the content is dropped.
*/

/**
Expand All @@ -77,7 +122,7 @@ export default class ClipboardObserver extends DomEventObserver {
* needs to be added to {@link module:engine/view/document~Document} by the {@link module:engine/view/document~Document#addObserver} method.
* It's done by the {@link module:clipboard/clipboard~Clipboard} feature. If it's not loaded, it must be done manually.
*
* @see module:clipboard/clipboardobserver~ClipboardObserver
* @see module:engine/view/document~Document#event:clipboardInput
* @event module:engine/view/document~Document#event:paste
* @param {module:clipboard/clipboardobserver~ClipboardEventData} data Event data.
*/
Expand Down
229 changes: 195 additions & 34 deletions tests/clipboardobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,77 +6,238 @@
/* globals document */

import ClipboardObserver from '../src/clipboardobserver';
import ViewDocument from '@ckeditor/ckeditor5-engine/src/view/document';
import Document from '@ckeditor/ckeditor5-engine/src/view/document';
import Element from '@ckeditor/ckeditor5-engine/src/view/element';
import Range from '@ckeditor/ckeditor5-engine/src/view/range';
import Position from '@ckeditor/ckeditor5-engine/src/view/position';
import DataTransfer from '../src/datatransfer';

describe( 'ClipboardObserver', () => {
let viewDocument, observer;
let doc, observer, root, el, range, eventSpy, preventDefaultSpy;

beforeEach( () => {
viewDocument = new ViewDocument();
observer = viewDocument.addObserver( ClipboardObserver );
doc = new Document();
root = doc.createRoot( 'div' );

// Create view and DOM structures.
el = new Element( 'p' );
root.appendChildren( el );
doc.domConverter.viewToDom( root, document, { withChildren: true, bind: true } );

doc.selection.collapse( el );
range = new Range( new Position( root, 1 ) );
// Just making sure that the following tests will check anything.
expect( range.isEqual( doc.selection.getFirstRange() ) ).to.be.false;

observer = doc.addObserver( ClipboardObserver );

eventSpy = sinon.spy();
preventDefaultSpy = sinon.spy();
} );

it( 'should define domEventType', () => {
expect( observer.domEventType ).to.deep.equal( [ 'paste', 'copy', 'cut', 'drop' ] );
} );

describe( 'onDomEvent', () => {
let pasteSpy, preventDefaultSpy;
describe( 'paste event', () => {
it( 'should be fired with the right event data', () => {
const dataTransfer = mockDomDataTransfer();
const targetElement = mockDomTargetElement( {} );

function getDataTransfer() {
return {
getData( type ) {
return 'foo:' + type;
}
};
}

beforeEach( () => {
pasteSpy = sinon.spy();
preventDefaultSpy = sinon.spy();
} );

it( 'should fire paste with the right event data - clipboardData', () => {
const dataTransfer = getDataTransfer();

viewDocument.on( 'paste', pasteSpy );
doc.on( 'paste', eventSpy );

observer.onDomEvent( {
type: 'paste',
target: document.body,
target: targetElement,
clipboardData: dataTransfer,
preventDefault: preventDefaultSpy
} );

expect( pasteSpy.calledOnce ).to.be.true;
expect( eventSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];

expect( data.domTarget ).to.equal( targetElement );

const data = pasteSpy.args[ 0 ][ 1 ];
expect( data.domTarget ).to.equal( document.body );
expect( data.dataTransfer ).to.be.instanceOf( DataTransfer );
expect( data.dataTransfer.getData( 'x/y' ) ).to.equal( 'foo:x/y' );

expect( preventDefaultSpy.calledOnce ).to.be.true;
} );
} );

it( 'should fire paste with the right event data - dataTransfer', () => {
const dataTransfer = getDataTransfer();
describe( 'drop event', () => {
it( 'should be fired with the right event data - basics', () => {
const dataTransfer = mockDomDataTransfer();
const targetElement = mockDomTargetElement( {} );

viewDocument.on( 'drop', pasteSpy );
doc.on( 'drop', eventSpy );

observer.onDomEvent( {
type: 'drop',
target: document.body,
target: targetElement,
dataTransfer,
preventDefault: preventDefaultSpy
} );

expect( pasteSpy.calledOnce ).to.be.true;
expect( eventSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];

expect( data.domTarget ).to.equal( targetElement );

const data = pasteSpy.args[ 0 ][ 1 ];
expect( data.domTarget ).to.equal( document.body );
expect( data.dataTransfer ).to.be.instanceOf( DataTransfer );
expect( data.dataTransfer.getData( 'x/y' ) ).to.equal( 'foo:x/y' );

expect( data.dropRange.isEqual( doc.selection.getFirstRange() ) ).to.be.true;

expect( preventDefaultSpy.calledOnce ).to.be.true;
} );

it( 'should be fired with the right event data – dropRange (when no info about it in the drop event)', () => {
const dataTransfer = mockDomDataTransfer();
const targetElement = mockDomTargetElement( {} );

doc.on( 'drop', eventSpy );

observer.onDomEvent( {
type: 'drop',
target: targetElement,
dataTransfer,
preventDefault() {}
} );

expect( eventSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];

expect( data.dropRange.isEqual( doc.selection.getFirstRange() ) ).to.be.true;
} );

it( 'should be fired with the right event data – dropRange (when document.caretRangeFromPoint present)', () => {
let caretRangeFromPointCalledWith;

const domRange = doc.domConverter.viewRangeToDom( range );
const dataTransfer = mockDomDataTransfer();
const targetElement = mockDomTargetElement( {
caretRangeFromPoint( x, y ) {
caretRangeFromPointCalledWith = [ x, y ];

return domRange;
}
} );

doc.on( 'drop', eventSpy );

observer.onDomEvent( {
type: 'drop',
target: targetElement,
dataTransfer,
clientX: 10,
clientY: 20,
preventDefault() {}
} );

expect( eventSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];

expect( data.dropRange.isEqual( range ) ).to.be.true;
expect( caretRangeFromPointCalledWith ).to.deep.equal( [ 10, 20 ] );
} );

it( 'should be fired with the right event data – dropRange (when evt.rangeParent|Offset present)', () => {
const domRange = doc.domConverter.viewRangeToDom( range );
const dataTransfer = mockDomDataTransfer();
const targetElement = mockDomTargetElement( {
createRange() {
return document.createRange();
}
} );

doc.on( 'drop', eventSpy );

observer.onDomEvent( {
type: 'drop',
target: targetElement,
dataTransfer,
rangeParent: domRange.startContainer,
rangeOffset: domRange.startOffset,
preventDefault() {}
} );

expect( eventSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];

expect( data.dropRange.isEqual( range ) ).to.be.true;
} );
} );

describe( 'clipboardInput event', () => {
it( 'should be fired on paste', () => {
const dataTransfer = new DataTransfer( mockDomDataTransfer() );
const normalPrioritySpy = sinon.spy();

doc.on( 'clipboardInput', eventSpy );
doc.on( 'paste', normalPrioritySpy );

doc.fire( 'paste', {
dataTransfer,
preventDefault: preventDefaultSpy
} );

expect( eventSpy.calledOnce ).to.be.true;
expect( preventDefaultSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];
expect( data.dataTransfer ).to.equal( dataTransfer );

expect( data.targetRanges ).to.have.length( 1 );
expect( data.targetRanges[ 0 ].isEqual( doc.selection.getFirstRange() ) ).to.be.true;

expect( sinon.assert.callOrder( normalPrioritySpy, eventSpy ) );
} );

it( 'should be fired on drop', () => {
const dataTransfer = new DataTransfer( mockDomDataTransfer() );
const normalPrioritySpy = sinon.spy();

doc.on( 'clipboardInput', eventSpy );
doc.on( 'drop', normalPrioritySpy );

doc.fire( 'drop', {
dataTransfer,
preventDefault: preventDefaultSpy,
dropRange: range
} );

expect( eventSpy.calledOnce ).to.be.true;
expect( preventDefaultSpy.calledOnce ).to.be.true;

const data = eventSpy.args[ 0 ][ 1 ];
expect( data.dataTransfer ).to.equal( dataTransfer );

expect( data.targetRanges ).to.have.length( 1 );
expect( data.targetRanges[ 0 ].isEqual( range ) ).to.be.true;

expect( sinon.assert.callOrder( normalPrioritySpy, eventSpy ) );
} );
} );
} );

// Returns a super simple mock of HTMLElement (we use only ownerDocument from it).
function mockDomTargetElement( documentMock ) {
return {
ownerDocument: documentMock
};
}

function mockDomDataTransfer() {
return {
files: [],
getData( type ) {
return 'foo:' + type;
}
};
}
Loading