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.

Sign up for GitHub

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

Preview logic requires tweaking #14983

Merged
merged 4 commits into from
Sep 15, 2023
Merged

Preview logic requires tweaking #14983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5fca59c
Fix preview logic
illia-stv Sep 13, 2023
654813e
Code review fixes
illia-stv Sep 14, 2023
b92c701
Code review fixes
illia-stv Sep 14, 2023
c4a0245
Tests rename
illia-stv Sep 14, 2023
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
31 changes: 26 additions & 5 deletions packages/ckeditor5-clipboard/src/dragdropexperimental.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
createElement,
DomEmitterMixin,
delay,
Rect,
type DelayedFunc,
type ObservableChangeEvent,
type DomEmitter
Expand Down Expand Up @@ -293,7 +294,10 @@ export default class DragDropExperimental extends Plugin {
method: 'dragstart'
} );

this._updatePreview( data.dataTransfer );
const { dataTransfer, domTarget, domEvent } = data;
const { clientX } = domEvent;

this._updatePreview( { dataTransfer, domTarget, clientX } );

data.stopPropagation();

Expand Down Expand Up @@ -620,7 +624,15 @@ export default class DragDropExperimental extends Plugin {
/**
* Updates the dragged preview image.
*/
private _updatePreview( dataTransfer: DataTransfer ): void {
private _updatePreview( {
dataTransfer,
domTarget,
clientX
}: {
dataTransfer: DataTransfer;
domTarget: HTMLElement;
clientX: number;
} ): void {
const view = this.editor.editing.view;
const editable = view.document.selection.editableElement!;
const domEditable = view.domConverter.mapViewToDom( editable )!;
Expand All @@ -632,18 +644,27 @@ export default class DragDropExperimental extends Plugin {
} );

global.document.body.appendChild( this._previewContainer );
} else {
this._previewContainer.removeChild( this._previewContainer.firstElementChild! );
} else if ( this._previewContainer.firstElementChild ) {
this._previewContainer.removeChild( this._previewContainer.firstElementChild );
}

const domRect = new Rect( domEditable );

// If domTarget is inside the editable root, browsers will display the preview correctly by themselves.
if ( domEditable.contains( domTarget ) ) {
return;
}

const domEditablePaddingLeft = parseFloat( computedStyle.paddingLeft );
const preview = createElement( global.document, 'div' );

preview.className = 'ck ck-content';
preview.style.width = computedStyle.width;
preview.style.paddingLeft = `${ domRect.left - clientX + domEditablePaddingLeft }px`;

preview.innerHTML = dataTransfer.getData( 'text/html' );

dataTransfer.setDragImage( preview, 0, 0 );
// TODO set x to make dragged widget stick to the mouse cursor

this._previewContainer.appendChild( preview );
}
Expand Down
136 changes: 111 additions & 25 deletions packages/ckeditor5-clipboard/tests/dragdropexperimental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals document, Event */
/* globals window, document, Event */

import ClipboardPipeline from '../src/clipboardpipeline';
import DragDropExperimental from '../src/dragdropexperimental';
Expand All @@ -20,6 +20,7 @@ import ShiftEnter from '@ckeditor/ckeditor5-enter/src/shiftenter';
import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
import env from '@ckeditor/ckeditor5-utils/src/env';
import { Rect } from '@ckeditor/ckeditor5-utils';

import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
Expand Down Expand Up @@ -1420,7 +1421,7 @@ describe( 'Drag and Drop experimental', () => {
expect( editableElement.hasAttribute( 'draggable' ) ).to.be.false;
} );

it( 'should only show one preview element', () => {
it( 'should only show one preview element when you drag element outside the editing root', () => {
setModelData( model,
'<blockQuote>' +
'[<paragraph>foo</paragraph>' +
Expand All @@ -1430,10 +1431,13 @@ describe( 'Drag and Drop experimental', () => {
'<horizontalLine></horizontalLine>'
);

const pilcrow = document.createElement( 'div' );
pilcrow.setAttribute( 'class', 'pilcrow' );

const dataTransferMock = createDataTransfer();

fireDragStart( dataTransferMock );
fireDragStart( dataTransferMock );
fireDragStart( dataTransferMock, () => {}, pilcrow );
fireDragStart( dataTransferMock, () => {}, pilcrow );

const numberOfCkContentElements = Object
.keys( document.getElementsByClassName( 'ck-content' ) )
Expand All @@ -1442,6 +1446,66 @@ describe( 'Drag and Drop experimental', () => {
// There should be two elements with the `.ck-content` class - editor and drag-and-drop preview.
expect( numberOfCkContentElements ).to.equal( 2 );
} );

it( 'should show preview with custom implementation if drag element outside the editing root', () => {
setModelData( editor.model, '<paragraph>[Foo.]</paragraph><horizontalLine></horizontalLine>' );

const dataTransfer = createDataTransfer( {} );

const spy = sinon.spy( dataTransfer, 'setDragImage' );
const clientX = 10;

viewDocument.fire( 'dragstart', {
dataTransfer,
preventDefault: sinon.spy(),
stopPropagation: sinon.spy(),
domEvent: {
clientX
}
} );

const editable = editor.editing.view.document.selection.editableElement;
const domEditable = editor.editing.view.domConverter.mapViewToDom( editable );
const computedStyle = window.getComputedStyle( domEditable );
const paddingLeftString = computedStyle.paddingLeft;
const paddingLeft = parseFloat( paddingLeftString );

const domRect = new Rect( domEditable );

sinon.assert.calledWith( spy, sinon.match( {
style: {
'padding-left': `${ domRect.left - clientX + paddingLeft }px`
},
className: 'ck ck-content',
firstChild: sinon.match( {
tagName: 'P',
innerHTML: 'Foo.'
} )
} ), 0, 0 );
sinon.assert.calledOnce( spy );
} );

it( 'should show preview with browser implementation if drag element inside the editing root', () => {
setModelData( editor.model, '<paragraph>[Foo.]</paragraph><horizontalLine></horizontalLine>' );

const dataTransfer = createDataTransfer( {} );

const spy = sinon.spy( dataTransfer, 'setDragImage' );

const modelElement = root.getNodeByPath( [ 0 ] );
const viewElement = mapper.toViewElement( modelElement );
const domElement = domConverter.mapViewToDom( viewElement );

viewDocument.fire( 'dragstart', {
dataTransfer,
preventDefault: sinon.spy(),
stopPropagation: sinon.spy(),
domEvent: getMockedMousePosition( domElement ),
domTarget: domElement
} );

sinon.assert.notCalled( spy );
} );
} );

describe( 'dragenter', () => {
Expand Down Expand Up @@ -1945,10 +2009,16 @@ describe( 'Drag and Drop experimental', () => {
it( 'is enabled when starts dragging the text node', () => {
setModelData( editor.model, '<paragraph>[Foo.]</paragraph><horizontalLine></horizontalLine>' );

const nodeModel = root.getNodeByPath( [ 0 ] );
const nodeView = mapper.toViewElement( nodeModel );
const nodeDOM = domConverter.mapViewToDom( nodeView );
const dataTransfer = createDataTransfer( {} );

viewDocument.fire( 'dragstart', {
dataTransfer,
preventDefault: sinon.spy(),
dataTransfer: createDataTransfer( {} ),
stopPropagation: sinon.spy()
stopPropagation: sinon.spy(),
domEvent: getMockedMousePosition( nodeDOM )
} );

expect( widgetToolbarRepository.isEnabled ).to.be.true;
Expand All @@ -1957,14 +2027,19 @@ describe( 'Drag and Drop experimental', () => {
it( 'is disabled when plugin is disabled', () => {
setModelData( editor.model, '<paragraph>Foo.</paragraph>[<horizontalLine></horizontalLine>]' );

const nodeModel = root.getNodeByPath( [ 0 ] );
const nodeView = mapper.toViewElement( nodeModel );
const nodeDOM = domConverter.mapViewToDom( nodeView );

const plugin = editor.plugins.get( 'DragDropExperimental' );
plugin.isEnabled = false;

viewDocument.fire( 'dragstart', {
preventDefault: sinon.spy(),
target: viewDocument.getRoot().getChild( 1 ),
dataTransfer: createDataTransfer( {} ),
stopPropagation: sinon.spy()
stopPropagation: sinon.spy(),
domEvent: getMockedMousePosition( nodeDOM )
} );

expect( widgetToolbarRepository.isEnabled ).to.be.false;
Expand All @@ -1973,11 +2048,16 @@ describe( 'Drag and Drop experimental', () => {
it( 'is disabled when starts dragging the widget', () => {
setModelData( editor.model, '<paragraph>Foo.</paragraph>[<horizontalLine></horizontalLine>]' );

const nodeModel = root.getNodeByPath( [ 0 ] );
const nodeView = mapper.toViewElement( nodeModel );
const nodeDOM = domConverter.mapViewToDom( nodeView );

viewDocument.fire( 'dragstart', {
preventDefault: sinon.spy(),
target: viewDocument.getRoot().getChild( 1 ),
dataTransfer: createDataTransfer( {} ),
stopPropagation: sinon.spy()
stopPropagation: sinon.spy(),
domEvent: getMockedMousePosition( nodeDOM )
} );

expect( widgetToolbarRepository.isEnabled ).to.be.false;
Expand All @@ -1988,11 +2068,16 @@ describe( 'Drag and Drop experimental', () => {

const dataTransfer = createDataTransfer( {} );

const nodeModel = root.getNodeByPath( [ 0 ] );
const nodeView = mapper.toViewElement( nodeModel );
const nodeDOM = domConverter.mapViewToDom( nodeView );

viewDocument.fire( 'dragstart', {
preventDefault: sinon.spy(),
target: viewDocument.getRoot().getChild( 0 ),
dataTransfer,
stopPropagation: sinon.spy()
stopPropagation: sinon.spy(),
domEvent: getMockedMousePosition( nodeDOM )
} );

expect( widgetToolbarRepository.isEnabled ).to.be.false;
Expand All @@ -2017,11 +2102,16 @@ describe( 'Drag and Drop experimental', () => {

const dataTransfer = createDataTransfer( {} );

const nodeModel = root.getNodeByPath( [ 0 ] );
const nodeView = mapper.toViewElement( nodeModel );
const nodeDOM = domConverter.mapViewToDom( nodeView );

viewDocument.fire( 'dragstart', {
preventDefault: sinon.spy(),
target: viewDocument.getRoot().getChild( 0 ),
dataTransfer,
stopPropagation() {}
stopPropagation() {},
domEvent: getMockedMousePosition( nodeDOM )
} );

expect( widgetToolbarRepository.isEnabled ).to.be.false;
Expand Down Expand Up @@ -2098,8 +2188,8 @@ describe( 'Drag and Drop experimental', () => {
} );
} );

function fireDragStart( dataTransferMock, preventDefault = () => {} ) {
const eventData = prepareEventData( model.document.selection.getLastPosition() );
function fireDragStart( dataTransferMock, preventDefault = () => {}, domTarget ) {
const eventData = prepareEventData( model.document.selection.getLastPosition(), domTarget );

viewDocument.fire( 'mousedown', {
...eventData
Expand All @@ -2113,8 +2203,6 @@ describe( 'Drag and Drop experimental', () => {
} );
}

// -----------------------------------

function fireDragging( dataTransferMock, modelPositionOrRange ) {
viewDocument.fire( 'dragging', {
...prepareEventData( modelPositionOrRange ),
Expand All @@ -2125,8 +2213,6 @@ describe( 'Drag and Drop experimental', () => {
} );
}

// -----------------------------------

function fireDrop( dataTransferMock, modelPosition ) {
viewDocument.fire( 'clipboardInput', {
...prepareEventData( modelPosition ),
Expand All @@ -2145,7 +2231,7 @@ describe( 'Drag and Drop experimental', () => {
} );
}

function prepareEventData( modelPositionOrRange ) {
function prepareEventData( modelPositionOrRange, domTarget ) {
let domNode, viewElement, viewRange;

if ( modelPositionOrRange.is( 'position' ) ) {
Expand All @@ -2154,13 +2240,17 @@ describe( 'Drag and Drop experimental', () => {
viewRange = view.createRange( viewPosition );
viewElement = mapper.findMappedViewAncestor( viewPosition );

domNode = viewPosition.parent.is( '$text' ) ?
domConverter.findCorrespondingDomText( viewPosition.parent ).parentNode :
domConverter.mapViewToDom( viewElement );
if ( !domTarget ) {
domNode = viewPosition.parent.is( '$text' ) ?
domConverter.findCorrespondingDomText( viewPosition.parent ).parentNode :
domConverter.mapViewToDom( viewElement );
} else {
domNode = domTarget;
}
} else {
viewRange = mapper.toViewRange( modelPositionOrRange );
viewElement = viewRange.getContainedElement();
domNode = domConverter.mapViewToDom( viewElement );
domNode = domTarget || domConverter.mapViewToDom( viewElement );
}

return {
Expand All @@ -2185,8 +2275,6 @@ describe( 'Drag and Drop experimental', () => {
}
}

// -----------------------------------

function expectDraggingMarker( targetPositionOrRange ) {
expect( model.markers.has( 'drop-target' ) ).to.be.true;

Expand All @@ -2198,8 +2286,6 @@ describe( 'Drag and Drop experimental', () => {
}
}

// -----------------------------------

function expectFinalized() {
expect( viewDocument.getRoot().hasAttribute( 'draggable' ) ).to.be.false;

Expand Down