From 76a82e31fdd9c50c3651e0df8f62abda9051c31d Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 30 Dec 2024 13:23:45 +0100 Subject: [PATCH] Select all columns in previous table rows when the last row is fully selected. --- .../ckeditor5-table/src/tableselection.ts | 13 +++- .../tests/tableclipboard-paste.js | 64 +++++++++---------- .../ckeditor5-table/tests/tableselection.js | 22 +++++++ 3 files changed, 64 insertions(+), 35 deletions(-) diff --git a/packages/ckeditor5-table/src/tableselection.ts b/packages/ckeditor5-table/src/tableselection.ts index fa8ee4ee3ab..d534bd4ef2e 100644 --- a/packages/ckeditor5-table/src/tableselection.ts +++ b/packages/ckeditor5-table/src/tableselection.ts @@ -360,16 +360,23 @@ export default class TableSelection extends Plugin { */ private _getCellsToSelect( anchorCell: Element, targetCell: Element ) { const tableUtils: TableUtils = this.editor.plugins.get( 'TableUtils' ); + const table = anchorCell.findAncestor( 'table' )!; + const startLocation = tableUtils.getCellLocation( anchorCell ); const endLocation = tableUtils.getCellLocation( targetCell ); const startRow = Math.min( startLocation.row, endLocation.row ); const endRow = Math.max( startLocation.row, endLocation.row ); + // If users selects the colspan cell, and the previous row contains selection, the selection should be + // expanded in the previous row to accommodate the size of the colspan cell. + // See: https://github.com/ckeditor/ckeditor5/issues/17538 + const targetColumnExtraColspan = ( parseInt( targetCell.getAttribute( 'colspan' ) as string || '1' ) - 1 ); + const startColumn = Math.min( startLocation.column, endLocation.column ); - const endColumn = Math.max( startLocation.column, endLocation.column ); + const endColumn = Math.max( startLocation.column, endLocation.column + targetColumnExtraColspan ); - // 2-dimensional array of the selected cells to ease flipping the order of cells for backward selections. + // First collect cells based on initial selection const selectionMap: Array> = new Array( endRow - startRow + 1 ).fill( null ).map( () => [] ); const walkerOptions = { @@ -379,7 +386,7 @@ export default class TableSelection extends Plugin { endColumn }; - for ( const { row, cell } of new TableWalker( anchorCell.findAncestor( 'table' )!, walkerOptions ) ) { + for ( const { row, cell } of new TableWalker( table, walkerOptions ) ) { selectionMap[ row - startRow ].push( cell ); } diff --git a/packages/ckeditor5-table/tests/tableclipboard-paste.js b/packages/ckeditor5-table/tests/tableclipboard-paste.js index d943350691a..60ce0918c3d 100644 --- a/packages/ckeditor5-table/tests/tableclipboard-paste.js +++ b/packages/ckeditor5-table/tests/tableclipboard-paste.js @@ -2527,15 +2527,15 @@ describe( 'table clipboard', () => { // + + + +----+----+----+ // | | | | 14 | 15 | 16 | // + +----+----+----+----+----+ - // | | aa | ab | ac | | + // | | aa | ab | ac | aa | ab | // +----+----+----+----+----+----+----+ - // | 30 | 31 | ba | bb | bc | 35 | 36 | + // | 30 | 31 | ba | bb | bc | ba | bb | + // + +----+----+----+----+----+----+ + // | | 41 | ca | cb | cc | ca | cb | // + +----+----+----+----+----+----+ - // | | 41 | ca | cb | cc | 45 | - // + +----+----+----+----+ + // | | 51 | 52 | | 54 | | - // +----+----+----+ +----+ + - // | 60 | 61 | 62 | | 64 | | + // + +----+----+ +----+ + + // | | 61 | 62 | | 64 | | // +----+----+----+----+----+----+----+ expect( getModelData( model, { withoutSelection: true } ) ).to.equalMarkup( modelTable( [ [ @@ -2545,10 +2545,10 @@ describe( 'table clipboard', () => { { contents: '04', colspan: 3 } ], [ '14', '15', '16' ], - [ 'aa', 'ab', 'ac', { contents: '', colspan: 2 } ], - [ { contents: '30', rowspan: 3 }, '31', 'ba', 'bb', 'bc', '35', '36' ], - [ '41', 'ca', 'cb', 'cc', { contents: '45', colspan: 2, rowspan: 3 } ], - [ '51', '52', { contents: '', rowspan: 2 }, '54' ], + [ 'aa', 'ab', 'ac', 'aa', 'ab' ], + [ { contents: '30', rowspan: 3 }, '31', 'ba', 'bb', 'bc', 'ba', 'bb' ], + [ '41', 'ca', 'cb', 'cc', 'ca', 'cb' ], + [ '51', '52', { contents: '', rowspan: 2 }, '54', { contents: '', colspan: 2, rowspan: 2 } ], [ '60', '61', '62', '64' ] ] ) ); } ); @@ -3082,23 +3082,23 @@ describe( 'table clipboard', () => { // +----+----+----+ +----+----+ // | 10 | 11 | 12 | | 14 | 15 | // +----+----+----+----+----+----+ - // | aa | ab | aa | ab | aa | 25 | + // | aa | ab | aa | ab | aa | ab | // +----+----+----+----+----+----+ - // | ba | bb | ba | bb | ba | 35 | + // | ba | bb | ba | bb | ba | bb | // +----+----+----+----+----+----+ - // | aa | ab | aa | ab | aa | | - // +----+----+----+----+----+ + - // | 50 | 51 | 52 | | | + // | aa | ab | aa | ab | aa | ab | + // +----+----+----+----+----+----+ + // | 50 | 51 | 52 | | // +----+----+----+----+----+----+ // | 60 | 61 | 62 | 63 | 64 | 65 | // +----+----+----+----+----+----+ expect( getModelData( model, { withoutSelection: true } ) ).to.equalMarkup( modelTable( [ [ '00', '01', '02', { contents: '03', rowspan: 2 }, '04', '05' ], [ '10', '11', '12', '14', '15' ], - [ 'aa', 'ab', 'aa', 'ab', 'aa', '25' ], - [ 'ba', 'bb', 'ba', 'bb', 'ba', '35' ], - [ 'aa', 'ab', 'aa', 'ab', 'aa', { contents: '', rowspan: 2 } ], - [ '50', '51', { contents: '52', colspan: 2 }, '' ], + [ 'aa', 'ab', 'aa', 'ab', 'aa', 'ab' ], + [ 'ba', 'bb', 'ba', 'bb', 'ba', 'bb' ], + [ 'aa', 'ab', 'aa', 'ab', 'aa', 'ab' ], + [ '50', '51', { contents: '52', colspan: 2 }, { contents: '', colspan: 2 } ], [ '60', '61', '62', '63', '64', '65' ] ] ) ); } ); @@ -3115,27 +3115,27 @@ describe( 'table clipboard', () => { ] ); // +----+----+----+----+----+----+ - // | 00 | 01 | aa | ab | aa | 05 | + // | 00 | 01 | aa | ab | aa | ab | // +----+----+----+----+----+----+ - // | 10 | 11 | ba | bb | ba | 15 | + // | 10 | 11 | ba | bb | ba | bb | // +----+----+----+----+----+----+ - // | 20 | 21 | aa | ab | aa | 25 | + // | 20 | 21 | aa | ab | aa | ab | // +----+----+----+----+----+----+ - // | 30 | 31 | ba | bb | ba | 35 | + // | 30 | 31 | ba | bb | ba | bb | // +----+----+----+----+----+----+ - // | 40 | aa | ab | aa | | - // +----+----+----+----+----+ + - // | 50 | 51 | 52 | | | + // | 40 | aa | ab | aa | ab | + // +----+----+----+----+----+----+ + // | 50 | 51 | 52 | | // +----+----+----+----+----+----+ // | 60 | 61 | 62 | 63 | 64 | 65 | // +----+----+----+----+----+----+ expect( getModelData( model, { withoutSelection: true } ) ).to.equalMarkup( modelTable( [ - [ '00', '01', 'aa', 'ab', 'aa', '05' ], - [ '10', '11', 'ba', 'bb', 'ba', '15' ], - [ '20', '21', 'aa', 'ab', 'aa', '25' ], - [ '30', '31', 'ba', 'bb', 'ba', '35' ], - [ { contents: '40', colspan: 2 }, 'aa', 'ab', 'aa', { contents: '', rowspan: 2 } ], - [ '50', '51', { contents: '52', colspan: 2 }, '' ], + [ '00', '01', 'aa', 'ab', 'aa', 'ab' ], + [ '10', '11', 'ba', 'bb', 'ba', 'bb' ], + [ '20', '21', 'aa', 'ab', 'aa', 'ab' ], + [ '30', '31', 'ba', 'bb', 'ba', 'bb' ], + [ { contents: '40', colspan: 2 }, 'aa', 'ab', 'aa', 'ab' ], + [ '50', '51', { contents: '52', colspan: 2 }, { contents: '', colspan: 2 } ], [ '60', '61', '62', '63', '64', '65' ] ] ) ); } ); diff --git a/packages/ckeditor5-table/tests/tableselection.js b/packages/ckeditor5-table/tests/tableselection.js index 762fa26a92f..095f08ab2e3 100644 --- a/packages/ckeditor5-table/tests/tableselection.js +++ b/packages/ckeditor5-table/tests/tableselection.js @@ -570,6 +570,28 @@ describe( 'TableSelection', () => { expect( selection.isBackward ).to.be.true; } ); + it( 'should select all cells when selecting from a regular row to a row with colspan', () => { + setModelData( model, modelTable( [ + [ '00', '01', '02' ], + [ { contents: '11', colspan: 3 } ] + ] ) ); + + table = modelRoot.getChild( 0 ); + + const anchorCell = table.getChild( 0 ).getChild( 0 ); + const targetCell = table.getChild( 1 ).getChild( 0 ); + + tableSelection.setCellSelection( anchorCell, targetCell ); + + const selectedCells = tableSelection.getSelectedTableCells(); + + expect( selectedCells ).to.have.length( 4 ); + expect( selectedCells[ 0 ] ).to.equal( table.getChild( 0 ).getChild( 0 ) ); + expect( selectedCells[ 1 ] ).to.equal( table.getChild( 0 ).getChild( 1 ) ); + expect( selectedCells[ 2 ] ).to.equal( table.getChild( 0 ).getChild( 2 ) ); + expect( selectedCells[ 3 ] ).to.equal( table.getChild( 1 ).getChild( 0 ) ); + } ); + function assertSelection( anchorCell, focusCell, count ) { const cells = [ ...selection.getRanges() ].map( range => range.getContainedElement() );