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

Fix for: Element/Image drag to move within table is no longer available #3244

Closed
wants to merge 4 commits into from
Closed
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
47 changes: 34 additions & 13 deletions plugins/tableselection/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
fakeSelectedEditorClass = fakeSelectedClass + '-editor',
fakeSelectedTableDataAttribute = 'cke-table-faked-selection-table',
ignoredTableAttribute = 'data-cke-tableselection-ignored',
draggingActiveAttribute = 'cke-table-dragging-active',
fakeSelection = { active: false },
tabletools,
getSelectedCells,
Expand Down Expand Up @@ -231,10 +232,10 @@
var editor = evt.editor || evt.sender.editor,
selection = editor && editor.getSelection(),
ranges = selection && selection.getRanges() || [],
table = ranges[ 0 ]._getTableElement( { table: 1 } ),
enclosedNode = ranges && ranges[ 0 ].getEnclosedNode(),
isEnclosedNodeAnImage = enclosedNode && ( enclosedNode.type == CKEDITOR.NODE_ELEMENT ) && enclosedNode.is( 'img' ),
cells,
table,
iterator;

if ( !selection ) {
Expand All @@ -253,8 +254,8 @@
return;
}

// (#2945)
if ( ranges[ 0 ]._getTableElement( { table: 1 } ).hasAttribute( ignoredTableAttribute ) ) {
// (#2945 and #547)
if ( table.hasAttribute( ignoredTableAttribute ) || table.hasAttribute( draggingActiveAttribute ) ) {
return;
}

Expand Down Expand Up @@ -304,8 +305,8 @@
table = target && target.getAscendant( 'table', true ),
tableElements = { table: 1, thead: 1, tbody: 1, tfoot: 1, tr: 1, td: 1, th: 1 };

// (#2945)
if ( table && table.hasAttribute( ignoredTableAttribute ) ) {
// (#2945 and #547)
if ( table && ( table.hasAttribute( ignoredTableAttribute ) || table.hasAttribute( draggingActiveAttribute ) ) ) {
return;
}

Expand Down Expand Up @@ -379,7 +380,8 @@
}

function fakeSelectionDragHandler( evt ) {
var table = evt.data.getTarget().getAscendant( 'table', true );
var table = evt.data.getTarget().getAscendant( 'table', true ),
editor = evt.editor || evt.sender.editor;

// (#2945)
if ( table && table.hasAttribute( ignoredTableAttribute ) ) {
Expand All @@ -392,9 +394,20 @@
return;
}

// We're not supporting dragging in our table selection for the time being.
evt.cancel();
evt.data.preventDefault();
// On 'dragstart' add a temporal attribute cancelling tableselection features, just like
// for #2945. The reason why ignoredTableAttribute can't be reused is that after drag we
// don't know if the attribute was added temporary or by design. (#547)
if ( evt.name == 'dragstart' ) {
editor.fire( 'lockSnapshot', { dontUpdate: 1 } );
table.setAttribute( draggingActiveAttribute, '' );
return;
}

// If it's 'drop' event, reset fake selection which appears otherwise (no conditional needed though
// as 'dragstart' is covered before and the fakeSelectionDragHandler() function only runs for 'dragstart'
// and 'drop' events).
editor.getSelection().reset();
clearFakeCellSelection( editor, true );
}

function copyTable( editor, isCut ) {
Expand Down Expand Up @@ -483,18 +496,18 @@

function fakeSelectionCopyCutHandler( evt ) {
var editor = evt.editor || evt.sender.editor,
selection = editor.getSelection();
selection = editor.getSelection(),
table = selection.getRanges()[ 0 ]._getTableElement( { table: 1 } );

if ( !selection.isInTable() ) {
return;
}

// (#2945)
if ( selection.getRanges()[ 0 ]._getTableElement( { table: 1 } ).hasAttribute( ignoredTableAttribute ) ) {
// (#2945 and #547)
if ( table.hasAttribute( ignoredTableAttribute ) || table.hasAttribute( draggingActiveAttribute ) ) {
return;
}


copyTable( editor, evt.name === 'cut' );
}

Expand Down Expand Up @@ -785,6 +798,13 @@
return;
}

// (#547)
if ( table && table.hasAttribute( draggingActiveAttribute ) ) {
editor.fire( 'unlockSnapshot' );
table.removeAttribute( draggingActiveAttribute );
return;
}

var selectedCells = getSelectedCells( selection ),
pastedTable = this.findTableInPastedContent( editor, evt.data.dataValue ),
boundarySelection = selection.isInTable( true ) && this.isBoundarySelection( selection ),
Expand Down Expand Up @@ -1192,6 +1212,7 @@
editable.attachListener( mouseHost, 'mouseup', fakeSelectionMouseHandler, null, evtInfo );

editable.attachListener( editable, 'dragstart', fakeSelectionDragHandler );
editable.attachListener( editable, 'drop', fakeSelectionDragHandler );
editable.attachListener( editor, 'selectionCheck', fakeSelectionChangeHandler );

CKEDITOR.plugins.tableselection.keyboardIntegration( editor );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<div id="editor1">
<table border="1" cellpadding="1" cellspacing="1" style="width:100%">
<tbody>
<tr>
<td>&nbsp;</td>
</tr>
</tbody>
</table>
<p><img alt="" src="https://cdn.emojics.com/v1.0.0/emojis/wow/symbols/1f636-color.png" style="height:50px; width:50px" /></p>
<table border="1" cellpadding="1" cellspacing="1" style="width:500px">
<tbody>
<tr>
<td>
<p>1.1</p>
</td>
<td>
<p>1.2</p>
</td>
</tr>
<tr>
<td><img alt="" src="https://cdn.emojics.com/v1.0.0/emojis/like/symbols/1f60a-color.png" style="height:50px; width:50px" /></td>
<td>
<p>2.2</p>
</td>
</tr>
<tr>
<td>
<p>3.1</p>
</td>
<td>
<table border="1" cellpadding="1" cellspacing="1" style="width:100%">
<tbody>
<tr>
<td><img alt="" src="https://cdn.emojics.com/v1.0.0/emojis/angry/symbols/2639-color.png" style="height:50px; width:50px" /></td>
<td>&nbsp;</td>
</tr>
<tr>
<td>&nbsp;</td>
<td>&nbsp;</td>
</tr>
</tbody>
</table>
</td>
</tr>
</tbody>
</table>
<p><img alt="" src="https://cdn.emojics.com/v1.0.0/emojis/sad/symbols/1f641-color.png" style="height:50px; width:50px" /></p>
<table border="1" cellpadding="1" cellspacing="1" style="width:100%">
<tbody>
<tr>
<td>&nbsp;</td>
</tr>
</tbody>
</table>
</div>
<div id="editor2">
<table border="1" cellpadding="1" cellspacing="1" style="width:100%">
<tbody>
<tr>
<td>&nbsp;</td>
</tr>
</tbody>
</table>
</div>

<script>
if ( bender.tools.env.mobile ) {
bender.ignore();
}

bender.tools.ignoreUnsupportedEnvironment( 'tableselection' );

CKEDITOR.replace( 'editor1', { height: 500 } );
CKEDITOR.replace( 'editor2', { height: 100 } );
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
@bender-ui: collapsed
@bender-tags: bug, 547, 4.14.0
@bender-ckeditor-plugins: wysiwygarea, toolbar, tableselection, clipboard, sourcearea, undo, elementspath, image, link

## Important:
**After each scenario use `undo` button to revert editor to the basic state.** Observe if undoing works correctly.

For each scenario **Expected** and **Unexpected** results are the same:

**Expected:**
Emoji was dropped in the new location without creating additional elements (like nested table, additional row or whitespaces).

**Unexpected:**
Drop target was not droppable or some elements (mentioned above) appeared from nowhere.

## Scenarios:

1. Drag <span style="color:#59bb34">green (happy)</span> emoji to the cell 1.1.

1. Drag <span style="color:#e74433">red (angry)</span> emoji to each of the cells in the table nested in cell 3.2, then drag it to the cell 2.2.

1. Drag <span style="color:#59bb34">green (happy)</span> emoji to the table at the beginning of WYSIWYG area and back to the cell 2.1.

1. Drag <span style="color:#e74433">red (angry)</span> emoji to the table at the end of WYSIWYG area and back to the nested table.

1. Drag <span style="color:#f2c041">yellow (neutral)</span> emoji to the cell 1.1.

1. Drag <span style="color:#f2c041">yellow (neutral)</span> emoji to any cell in the nested table.

1. Drag <span style="color:#e47a3b">orange (sad)</span> emoji to the cell 1.1.

1. Drag <span style="color:#e47a3b">orange (sad)</span> emoji to any cell in the nested table.

1. Drag <span style="color:#e47a3b">orange (sad)</span> emoji to the table in second editor instance.

1. Drag <span style="color:#e74433">red (angry)</span> emoji to the table in second editor instance.

1. Last one is a little bit tricky:
* Select text in cell 1.1.
* Still holding mouse button move coursor to another cell and go back to cell 1.1
(the point is it should become fake selected).
* Drag text to another cell.

If you need there is a recording of potential bug we try to catch in this scenario:
https://github.com/ckeditor/ckeditor-dev/pull/3244#pullrequestreview-258421144.