8
0
Просмотр исходного кода

Merge pull request #260 from ckeditor/i/6356

Internal: Dropped `DeleteCommand` hooks in favor of the `Model#deleteContent()` hook in the `TableSelection` plugin. Improved `deleteContent()` integration tests. Closes ckeditor/ckeditor5#6356.
Piotrek Koszuliński 5 лет назад
Родитель
Сommit
8ab7fd2043

+ 19 - 53
packages/ckeditor5-table/src/tableselection.js

@@ -14,7 +14,7 @@ import TableUtils from './tableutils';
 import { setupTableSelectionHighlighting } from './tableselection/converters';
 import MouseSelectionHandler from './tableselection/mouseselectionhandler';
 import { findAncestor } from './commands/utils';
-import { clearTableCellsContents } from './tableselection/utils';
+import { getTableCellsInSelection, clearTableCellsContents } from './tableselection/utils';
 import cropTable from './tableselection/croptable';
 
 import '../theme/tableselection.css';
@@ -107,29 +107,6 @@ export default class TableSelection extends Plugin {
 		this.listenTo( model, 'deleteContent', ( evt, args ) => this._handleDeleteContent( evt, args ), { priority: 'high' } );
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	afterInit() {
-		const editor = this.editor;
-
-		const deleteCommand = editor.commands.get( 'delete' );
-
-		if ( deleteCommand ) {
-			this.listenTo( deleteCommand, 'execute', event => {
-				this._handleDeleteCommand( event, { isForward: false } );
-			}, { priority: 'high' } );
-		}
-
-		const forwardDeleteCommand = editor.commands.get( 'forwardDelete' );
-
-		if ( forwardDeleteCommand ) {
-			this.listenTo( forwardDeleteCommand, 'execute', event => {
-				this._handleDeleteCommand( event, { isForward: true } );
-			}, { priority: 'high' } );
-		}
-	}
-
 	/**
 	 * @inheritDoc
 	 */
@@ -315,41 +292,30 @@ export default class TableSelection extends Plugin {
 	 * @param {Array.<*>} args Delete content method arguments.
 	 */
 	_handleDeleteContent( event, args ) {
-		const [ selection ] = args;
+		const [ selection, options ] = args;
 		const model = this.editor.model;
+		const isBackward = !options || options.direction == 'backward';
+		const selectedTableCells = getTableCellsInSelection( selection );
 
-		if ( this.hasMultiCellSelection && selection.is( 'documentSelection' ) ) {
-			event.stop();
-
-			clearTableCellsContents( model, this.getSelectedTableCells() );
-
-			model.change( writer => {
-				writer.setSelection( Array.from( this.getSelectedTableCells() ).pop(), 0 );
-			} );
+		if ( !selectedTableCells.length ) {
+			return;
 		}
-	}
-
-	/**
-	 * It overrides default `DeleteCommand` behavior over a selected table fragment.
-	 *
-	 * @private
-	 * @param {module:utils/eventinfo~EventInfo} event
-	 * @param {Object} options
-	 * @param {Boolean} options.isForward Whether it handles forward or backward delete.
-	 */
-	_handleDeleteCommand( event, options ) {
-		const model = this.editor.model;
 
-		if ( this.hasMultiCellSelection ) {
-			event.stop();
+		event.stop();
 
-			clearTableCellsContents( model, this.getSelectedTableCells() );
+		model.change( writer => {
+			const tableCellToSelect = selectedTableCells[ isBackward ? selectedTableCells.length - 1 : 0 ];
 
-			const tableCell = options.isForward ? this._startElement : this._endElement;
+			clearTableCellsContents( model, selectedTableCells );
 
-			model.change( writer => {
-				writer.setSelection( tableCell, 0 );
-			} );
-		}
+			// The insertContent() helper passes the actual DocumentSelection,
+			// while the deleteContent() helper always operates on the abstract clones.
+			if ( selection.is( 'documentSelection' ) ) {
+				writer.setSelection( tableCellToSelect, 'in' );
+			} else {
+				selection.setTo( tableCellToSelect, 'in' );
+			}
+		} );
 	}
 }
+

+ 22 - 2
packages/ckeditor5-table/src/tableselection/utils.js

@@ -7,6 +7,8 @@
  * @module table/tableselection/utils
  */
 
+import { getRangeContainedElement } from '../commands/utils';
+
 /**
  * Clears contents of the passed table cells.
  *
@@ -17,8 +19,6 @@
  *
  *		clearTableCellsContents( editor.model, tableSelection.getSelectedTableCells() );
  *
- * **Note**: This function is used also by {@link module:table/tableselection~TableSelection#getSelectionAsFragment}
- *
  * @param {module:engine/model/model~Model} model
  * @param {Iterable.<module:engine/model/element~Element>} tableCells
  */
@@ -29,3 +29,23 @@ export function clearTableCellsContents( model, tableCells ) {
 		}
 	} );
 }
+
+/**
+ * Returns all model cells within the provided model selection.
+ *
+ * @param {Iterable.<module:engine/model/selection~Selection>} selection
+ * @returns {Array.<module:engine/model/element~Element>}
+ */
+export function getTableCellsInSelection( selection ) {
+	const cells = [];
+
+	for ( const range of selection.getRanges() ) {
+		const element = getRangeContainedElement( range );
+
+		if ( element && element.is( 'tableCell' ) ) {
+			cells.push( element );
+		}
+	}
+
+	return cells;
+}

+ 58 - 6
packages/ckeditor5-table/tests/tableselection-integration.js

@@ -95,6 +95,56 @@ describe( 'table selection', () => {
 					[ '31', '32', '33' ]
 				] ) );
 			} );
+
+			it( 'should work with any arbitrary selection passed to Model#deleteContent() (delete backwards)', () => {
+				const selection = model.createSelection( [
+					model.createRange(
+						model.createPositionFromPath( modelRoot, [ 0, 0, 0 ] ),
+						model.createPositionFromPath( modelRoot, [ 0, 0, 1 ] )
+					),
+					model.createRange(
+						model.createPositionFromPath( modelRoot, [ 0, 0, 1 ] ),
+						model.createPositionFromPath( modelRoot, [ 0, 0, 2 ] )
+					)
+				] );
+
+				model.change( writer => {
+					model.deleteContent( selection );
+					writer.setSelection( selection );
+				} );
+
+				assertEqualMarkup( getModelData( model ), modelTable( [
+					[ '', '[]', '13' ],
+					[ '21', '22', '23' ],
+					[ '31', '32', '33' ]
+				] ) );
+			} );
+
+			it( 'should work with any arbitrary selection passed to Model#deleteContent() (delete forwards)', () => {
+				const selection = model.createSelection( [
+					model.createRange(
+						model.createPositionFromPath( modelRoot, [ 0, 0, 0 ] ),
+						model.createPositionFromPath( modelRoot, [ 0, 0, 1 ] )
+					),
+					model.createRange(
+						model.createPositionFromPath( modelRoot, [ 0, 0, 1 ] ),
+						model.createPositionFromPath( modelRoot, [ 0, 0, 2 ] )
+					)
+				] );
+
+				model.change( writer => {
+					model.deleteContent( selection, {
+						direction: 'forward'
+					} );
+					writer.setSelection( selection );
+				} );
+
+				assertEqualMarkup( getModelData( model ), modelTable( [
+					[ '[]', '', '13' ],
+					[ '21', '22', '23' ],
+					[ '31', '32', '33' ]
+				] ) );
+			} );
 		} );
 
 		describe( 'on user input', () => {
@@ -108,15 +158,16 @@ describe( 'table selection', () => {
 
 				viewDocument.fire( 'keydown', { keyCode: getCode( 'x' ) } );
 
-				//                                      figure       table         tbody         tr            td            span
-				const viewSpan = viewDocument.getRoot().getChild( 0 ).getChild( 1 ).getChild( 0 ).getChild( 1 ).getChild( 1 ).getChild( 0 );
+				// Mutate at the place where the document selection was put; it's more realistic
+				// than mutating at some arbitrary position.
+				const placeOfMutation = viewDocument.selection.getFirstRange().start.parent;
 
 				viewDocument.fire( 'mutations', [
 					{
 						type: 'children',
 						oldChildren: [],
 						newChildren: [ new ViewText( viewDocument, 'x' ) ],
-						node: viewSpan
+						node: placeOfMutation
 					}
 				] );
 
@@ -130,15 +181,16 @@ describe( 'table selection', () => {
 			it( 'should not interfere with default key handler if no table selection', () => {
 				viewDocument.fire( 'keydown', { keyCode: getCode( 'x' ) } );
 
-				//                                      figure       table         tbody         tr            td            span
-				const viewSpan = viewDocument.getRoot().getChild( 0 ).getChild( 1 ).getChild( 0 ).getChild( 0 ).getChild( 0 ).getChild( 0 );
+				// Mutate at the place where the document selection was put; it's more realistic
+				// than mutating at some arbitrary position.
+				const placeOfMutation = viewDocument.selection.getFirstRange().start.parent;
 
 				viewDocument.fire( 'mutations', [
 					{
 						type: 'children',
 						oldChildren: [],
 						newChildren: [ new ViewText( viewDocument, 'x' ) ],
-						node: viewSpan
+						node: placeOfMutation
 					}
 				] );