From 81694cd1d8a3f6cd624171c8917131bc1d3832f4 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 6 May 2020 20:22:12 +0200 Subject: [PATCH 1/2] Fixed insert table row/column commands when a widget is selected inside the table cell. --- .../src/commands/insertcolumncommand.js | 7 +++++-- .../src/commands/insertrowcommand.js | 5 ++++- .../tests/commands/insertcolumncommand.js | 19 +++++++++++++++++- .../tests/commands/insertrowcommand.js | 20 ++++++++++++++++++- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-table/src/commands/insertcolumncommand.js b/packages/ckeditor5-table/src/commands/insertcolumncommand.js index 3690c6c1fae..8f79dc03023 100644 --- a/packages/ckeditor5-table/src/commands/insertcolumncommand.js +++ b/packages/ckeditor5-table/src/commands/insertcolumncommand.js @@ -75,8 +75,11 @@ export default class InsertColumnCommand extends Command { const referencePosition = insertBefore ? selection.getFirstPosition() : selection.getLastPosition(); const referenceRange = insertBefore ? selection.getFirstRange() : selection.getLastRange(); - const tableCell = referenceRange.getContainedElement() || findAncestor( 'tableCell', referencePosition ); - const table = tableCell.parent.parent; + const containedElement = referenceRange.getContainedElement(); + const isTableCell = containedElement && containedElement.is( 'tableCell' ); + + const tableCell = isTableCell ? containedElement : findAncestor( 'tableCell', referencePosition ); + const table = findAncestor( 'table', tableCell ); const { column } = tableUtils.getCellLocation( tableCell ); diff --git a/packages/ckeditor5-table/src/commands/insertrowcommand.js b/packages/ckeditor5-table/src/commands/insertrowcommand.js index ebdd839c827..130e958b446 100644 --- a/packages/ckeditor5-table/src/commands/insertrowcommand.js +++ b/packages/ckeditor5-table/src/commands/insertrowcommand.js @@ -74,7 +74,10 @@ export default class InsertRowCommand extends Command { const referencePosition = insertAbove ? selection.getFirstPosition() : selection.getLastPosition(); const referenceRange = insertAbove ? selection.getFirstRange() : selection.getLastRange(); - const tableCell = referenceRange.getContainedElement() || findAncestor( 'tableCell', referencePosition ); + const containedElement = referenceRange.getContainedElement(); + const isTableCell = containedElement && containedElement.is( 'tableCell' ); + const tableCell = isTableCell ? containedElement : findAncestor( 'tableCell', referencePosition ); + const tableRow = tableCell.parent; const table = tableRow.parent; diff --git a/packages/ckeditor5-table/tests/commands/insertcolumncommand.js b/packages/ckeditor5-table/tests/commands/insertcolumncommand.js index 4e44e7b3255..d49a6661dd2 100644 --- a/packages/ckeditor5-table/tests/commands/insertcolumncommand.js +++ b/packages/ckeditor5-table/tests/commands/insertcolumncommand.js @@ -4,6 +4,7 @@ */ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; +import HorizontalLineEditing from '@ckeditor/ckeditor5-horizontal-line/src/horizontallineediting'; import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import InsertColumnCommand from '../../src/commands/insertcolumncommand'; @@ -18,7 +19,7 @@ describe( 'InsertColumnCommand', () => { beforeEach( () => { return ModelTestEditor .create( { - plugins: [ TableUtils, TableSelection ] + plugins: [ TableUtils, TableSelection, HorizontalLineEditing ] } ) .then( newEditor => { editor = newEditor; @@ -181,6 +182,22 @@ describe( 'InsertColumnCommand', () => { [ { colspan: 5, contents: '31' }, { colspan: 2, contents: '34' } ] ], { headingColumns: 5 } ) ); } ); + + it( 'should insert a column when a widget in the table cell is selected', () => { + setData( model, modelTable( [ + [ '11', '12' ], + [ '21', '22' ], + [ '31', '[]' ] + ] ) ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '11', '12', '' ], + [ '21', '22', '' ], + [ '31', '', '' ] + ] ) ); + } ); } ); } ); diff --git a/packages/ckeditor5-table/tests/commands/insertrowcommand.js b/packages/ckeditor5-table/tests/commands/insertrowcommand.js index d4a95111848..c69646fe999 100644 --- a/packages/ckeditor5-table/tests/commands/insertrowcommand.js +++ b/packages/ckeditor5-table/tests/commands/insertrowcommand.js @@ -4,6 +4,7 @@ */ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; +import HorizontalLineEditing from '@ckeditor/ckeditor5-horizontal-line/src/horizontallineediting'; import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import InsertRowCommand from '../../src/commands/insertrowcommand'; @@ -18,7 +19,7 @@ describe( 'InsertRowCommand', () => { beforeEach( () => { return ModelTestEditor .create( { - plugins: [ TableUtils, TableSelection ] + plugins: [ TableUtils, TableSelection, HorizontalLineEditing ] } ) .then( newEditor => { editor = newEditor; @@ -214,6 +215,23 @@ describe( 'InsertRowCommand', () => { [ 0, 0 ] ] ); } ); + + it( 'should insert a row when a widget in the table cell is selected', () => { + setData( model, modelTable( [ + [ '11', '12' ], + [ '21', '22' ], + [ '31', '[]' ] + ] ) ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '11', '12' ], + [ '21', '22' ], + [ '31', '' ], + [ '', '' ] + ] ) ); + } ); } ); } ); From 542c77baea33179e8267ce11dba48e34c2a78e98 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 7 May 2020 10:48:56 +0200 Subject: [PATCH 2/2] Insert row above/below refactored to use utils. --- .../src/commands/insertcolumncommand.js | 14 +++++--------- .../src/commands/insertrowcommand.js | 17 ++++++----------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/packages/ckeditor5-table/src/commands/insertcolumncommand.js b/packages/ckeditor5-table/src/commands/insertcolumncommand.js index 8f79dc03023..ab27c7cdbb8 100644 --- a/packages/ckeditor5-table/src/commands/insertcolumncommand.js +++ b/packages/ckeditor5-table/src/commands/insertcolumncommand.js @@ -9,6 +9,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import { findAncestor } from './utils'; +import { getColumnIndexes, getSelectionAffectedTableCells } from '../utils'; /** * The insert column command. @@ -72,16 +73,11 @@ export default class InsertColumnCommand extends Command { const tableUtils = editor.plugins.get( 'TableUtils' ); const insertBefore = this.order === 'left'; - const referencePosition = insertBefore ? selection.getFirstPosition() : selection.getLastPosition(); - const referenceRange = insertBefore ? selection.getFirstRange() : selection.getLastRange(); + const affectedTableCells = getSelectionAffectedTableCells( selection ); + const columnIndexes = getColumnIndexes( affectedTableCells ); - const containedElement = referenceRange.getContainedElement(); - const isTableCell = containedElement && containedElement.is( 'tableCell' ); - - const tableCell = isTableCell ? containedElement : findAncestor( 'tableCell', referencePosition ); - const table = findAncestor( 'table', tableCell ); - - const { column } = tableUtils.getCellLocation( tableCell ); + const column = insertBefore ? columnIndexes.first : columnIndexes.last; + const table = findAncestor( 'table', affectedTableCells[ 0 ] ); tableUtils.insertColumns( table, { columns: 1, at: insertBefore ? column : column + 1 } ); } diff --git a/packages/ckeditor5-table/src/commands/insertrowcommand.js b/packages/ckeditor5-table/src/commands/insertrowcommand.js index 130e958b446..623ece4da5f 100644 --- a/packages/ckeditor5-table/src/commands/insertrowcommand.js +++ b/packages/ckeditor5-table/src/commands/insertrowcommand.js @@ -9,6 +9,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import { findAncestor } from './utils'; +import { getRowIndexes, getSelectionAffectedTableCells } from '../utils'; /** * The insert row command. @@ -71,18 +72,12 @@ export default class InsertRowCommand extends Command { const tableUtils = editor.plugins.get( 'TableUtils' ); const insertAbove = this.order === 'above'; - const referencePosition = insertAbove ? selection.getFirstPosition() : selection.getLastPosition(); - const referenceRange = insertAbove ? selection.getFirstRange() : selection.getLastRange(); + const affectedTableCells = getSelectionAffectedTableCells( selection ); + const rowIndexes = getRowIndexes( affectedTableCells ); - const containedElement = referenceRange.getContainedElement(); - const isTableCell = containedElement && containedElement.is( 'tableCell' ); - const tableCell = isTableCell ? containedElement : findAncestor( 'tableCell', referencePosition ); + const row = insertAbove ? rowIndexes.first : rowIndexes.last; + const table = findAncestor( 'table', affectedTableCells[ 0 ] ); - const tableRow = tableCell.parent; - const table = tableRow.parent; - - const row = table.getChildIndex( tableRow ); - - tableUtils.insertRows( table, { rows: 1, at: this.order === 'below' ? row + 1 : row } ); + tableUtils.insertRows( table, { rows: 1, at: insertAbove ? row : row + 1 } ); } }