From 301438dfa46ac81e444ecbe19317acce8d07ffff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 8 Apr 2020 12:41:27 +0200 Subject: [PATCH 01/21] Add a test case for ckeditor/ckeditor#6502. --- tests/tableutils.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index ae1c4f56..0b07ce0f 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -810,7 +810,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should move rowspaned cells to row below removing it\'s row', () => { + it( 'should move rowspaned cells to a row below removing it\'s row', () => { setData( model, modelTable( [ [ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, '02' ], [ '12' ], @@ -826,6 +826,21 @@ describe( 'TableUtils', () => { [ '30', '31', '32' ] ] ) ); } ); + + it( 'should move rowspaned cells to a row below removing it\'s row (other cell is overlapping removed row)', () => { + setData( model, modelTable( [ + [ '00', { rowspan: 3, contents: '01' }, '02', '03', '04' ], + [ '10', { rowspan: 2, contents: '12' }, '13', '14' ], + [ '20', '23', '24' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', { rowspan: 2, contents: '01' }, '02', '03', '04' ], + [ '20', '12', '23', '24' ] + ] ) ); + } ); } ); describe( 'many rows', () => { From acbc8fc29bfcb2f04fcc16c65330fa3fb9b68f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 09:54:54 +0200 Subject: [PATCH 02/21] Refactor multiple rows removal algorithm. --- src/tableutils.js | 123 ++++++++++++++++++++++++++++------------------ 1 file changed, 74 insertions(+), 49 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index a1e1b17b..5e16213a 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -283,27 +283,47 @@ export default class TableUtils extends Plugin { */ removeRows( table, options ) { const model = this.editor.model; - const first = options.at; + const rowsToRemove = options.rows || 1; + const first = options.at; const last = first + rowsToRemove - 1; - model.change( writer => { - for ( let i = last; i >= first; i-- ) { - removeRow( table, i, writer ); + const cellsToMove = new Map(); + const cellsToTrim = []; + + for ( const { row, column, rowspan, cell } of [ ...new TableWalker( table, { endRow: last } ) ] ) { + // Get cells from removed row that are spanned over multiple rows. + if ( row >= first && row <= last && ( row + rowspan - 1 > last ) ) { + cellsToMove.set( column, { + cell, + rowspan: rowspan - ( row === first ? rowsToRemove : last - row + 1 ) + } ); } - const headingRows = table.getAttribute( 'headingRows' ) || 0; + // Get cells to reduce rowspan on cells that are above removed row and overlaps removed row. + if ( row < first && row + rowspan > last ) { + cellsToTrim.push( { + cell, + rowspan: row + rowspan - 1 >= last ? rowspan - rowsToRemove : rowspan - ( first - row ) + } ); + } + } - if ( headingRows && first < headingRows ) { - const newRows = getNewHeadingRowsValue( first, last, headingRows ); + model.change( writer => { + const nextRowIndex = last + 1; - // Must be done after the changes in table structure (removing rows). - // Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391. - model.enqueueChange( writer.batch, writer => { - updateNumericAttribute( 'headingRows', newRows, table, writer, 0 ); - } ); + moveCellsToNextRow( table, nextRowIndex, cellsToMove, writer ); + + for ( let i = last; i >= first; i-- ) { + writer.remove( table.getChild( i ) ); + } + + for ( const { rowspan, cell } of cellsToTrim ) { + updateNumericAttribute( 'rowspan', rowspan, cell, writer ); } + + updateHeadingRows( first, last, model, writer, table ); } ); } @@ -730,43 +750,6 @@ function breakSpanEvenly( span, numberOfCells ) { return { newCellsSpan, updatedSpan }; } -function removeRow( table, rowIndex, writer ) { - const cellsToMove = new Map(); - const tableRow = table.getChild( rowIndex ); - const tableMap = [ ...new TableWalker( table, { endRow: rowIndex } ) ]; - - // Get cells from removed row that are spanned over multiple rows. - tableMap - .filter( ( { row, rowspan } ) => row === rowIndex && rowspan > 1 ) - .forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) ); - - // Reduce rowspan on cells that are above removed row and overlaps removed row. - tableMap - .filter( ( { row, rowspan } ) => row <= rowIndex - 1 && row + rowspan > rowIndex ) - .forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) ); - - // Move cells to another row. - const targetRow = rowIndex + 1; - const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } ); - let previousCell; - - for ( const { row, column, cell } of [ ...tableWalker ] ) { - if ( cellsToMove.has( column ) ) { - const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column ); - const targetPosition = previousCell ? - writer.createPositionAfter( previousCell ) : - writer.createPositionAt( table.getChild( row ), 0 ); - writer.move( writer.createRangeOn( cellToMove ), targetPosition ); - updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer ); - previousCell = cellToMove; - } else { - previousCell = cell; - } - } - - writer.remove( tableRow ); -} - // Calculates a new heading rows value for removing rows from heading section. function getNewHeadingRowsValue( first, last, headingRows ) { if ( last < headingRows ) { @@ -787,3 +770,45 @@ function adjustHeadingColumns( table, removedColumnIndexes, writer ) { writer.setAttribute( 'headingColumns', headingColumns - headingsRemoved, table ); } } + +function updateHeadingRows( first, last, model, writer, table ) { + const headingRows = table.getAttribute( 'headingRows' ) || 0; + + if ( headingRows && first < headingRows ) { + const newRows = getNewHeadingRowsValue( first, last, headingRows ); + + // Must be done after the changes in table structure (removing rows). + // Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391. + model.enqueueChange( writer.batch, writer => { + updateNumericAttribute( 'headingRows', newRows, table, writer, 0 ); + } ); + } +} + +function moveCellsToNextRow( table, nextRowIndex, cellsToMove, writer ) { + const tableWalker = new TableWalker( table, { + includeSpanned: true, + startRow: nextRowIndex, + endRow: nextRowIndex + } ); + const row = table.getChild( nextRowIndex ); + + let previousCell; + + for ( const { column, cell, isSpanned } of tableWalker ) { + if ( cellsToMove.has( column ) ) { + const { cell: cellToMove, rowspan } = cellsToMove.get( column ); + + const targetPosition = previousCell ? + writer.createPositionAfter( previousCell ) : + writer.createPositionAt( row, 0 ); + + writer.move( writer.createRangeOn( cellToMove ), targetPosition ); + updateNumericAttribute( 'rowspan', rowspan, cellToMove, writer ); + + previousCell = cellToMove; + } else if ( !isSpanned ) { + previousCell = cell; + } + } +} From 741aaa71dfe4ae076e72d5dff263b063fd969706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 14:28:45 +0200 Subject: [PATCH 03/21] Add test case for truncating overlapping cells. --- tests/tableutils.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index 0b07ce0f..d112990a 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -798,7 +798,7 @@ describe( 'TableUtils', () => { [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ { rowspan: 2, contents: '13' }, '14' ], [ '22', '23', '24' ], - [ '30', '31', '32', '33', '34' ] + [ '31', '32', '33', '34' ] ] ) ); tableUtils.removeRows( root.getChild( 0 ), { at: 2 } ); @@ -806,7 +806,7 @@ describe( 'TableUtils', () => { assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ [ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ '13', '14' ], - [ '30', '31', '32', '33', '34' ] + [ '31', '32', '33', '34' ] ] ) ); } ); @@ -953,6 +953,22 @@ describe( 'TableUtils', () => { ] ) ); } ); + it( 'should decrease rowspan of table cells from rows before removed rows section', () => { + setData( model, modelTable( [ + [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], + [ '13', '14' ], + [ '22', '23', '24' ], + [ '31', '32', '33', '34' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ { rowspan: 2, contents: '00' }, '01', '02', '03', '04' ], + [ '31', '32', '33', '34' ] + ] ) ); + } ); + it( 'should create one undo step (1 batch)', () => { setData( model, modelTable( [ [ '00', '01' ], From ac998f7cbcf21f090132d43c870ba819662d4cca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 14:30:05 +0200 Subject: [PATCH 04/21] Reduce complexity of calculation notation of remove rows algorithm. --- src/tableutils.js | 76 ++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index 5e16213a..035a1c06 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -267,13 +267,13 @@ export default class TableUtils extends Plugin { * ┌───┬───┬───┐ `at` = 1 ┌───┬───┬───┐ * 0 │ a │ b │ c │ `rows` = 2 │ a │ b │ c │ 0 * │ ├───┼───┤ │ ├───┼───┤ - * 1 │ │ d │ e │ <-- remove from here │ │ h │ i │ 1 - * │ ├───┼───┤ will give: ├───┼───┼───┤ - * 2 │ │ f │ g │ │ j │ k │ l │ 2 - * │ ├───┼───┤ └───┴───┴───┘ - * 3 │ │ h │ i │ + * 1 │ │ d │ e │ <-- remove from here │ │ d │ i │ 1 + * │ │ ├───┤ will give: ├───┼───┼───┤ + * 2 │ │ │ f │ │ j │ k │ l │ 2 + * │ │ ├───┤ └───┴───┴───┘ + * 3 │ │ │ g │ * ├───┼───┼───┤ - * 4 │ j │ k │ l │ + * 4 │ h │ i │ j │ * └───┴───┴───┘ * * @param {module:engine/model/element~Element} table @@ -285,35 +285,38 @@ export default class TableUtils extends Plugin { const model = this.editor.model; const rowsToRemove = options.rows || 1; - const first = options.at; const last = first + rowsToRemove - 1; + // Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. const cellsToMove = new Map(); + // Similarly, if a cell is "above" removed rows sections we must trim their rowspan. const cellsToTrim = []; - for ( const { row, column, rowspan, cell } of [ ...new TableWalker( table, { endRow: last } ) ] ) { - // Get cells from removed row that are spanned over multiple rows. - if ( row >= first && row <= last && ( row + rowspan - 1 > last ) ) { - cellsToMove.set( column, { - cell, - rowspan: rowspan - ( row === first ? rowsToRemove : last - row + 1 ) - } ); + for ( const { row, column, rowspan, cell } of new TableWalker( table, { endRow: last } ) ) { + const rowspanInRemovedSection = last - row + 1; + const lastRowOfCell = row + rowspan - 1; + + const isCellStickingOutFromRemovedRows = row >= first && row <= last && lastRowOfCell > last; + + if ( isCellStickingOutFromRemovedRows ) { + const rowSpanToSet = rowspan - rowspanInRemovedSection; + cellsToMove.set( column, { cell, rowspan: rowSpanToSet } ); } - // Get cells to reduce rowspan on cells that are above removed row and overlaps removed row. - if ( row < first && row + rowspan > last ) { - cellsToTrim.push( { - cell, - rowspan: row + rowspan - 1 >= last ? rowspan - rowsToRemove : rowspan - ( first - row ) - } ); + const isCellOverlappingRemovedRows = row < first && lastRowOfCell >= first; + + if ( isCellOverlappingRemovedRows ) { + const rowspanAdjustment = lastRowOfCell >= last ? rowsToRemove : first - row; + const rowSpanToSet = rowspan - rowspanAdjustment; + cellsToTrim.push( { cell, rowspan: rowSpanToSet } ); } } model.change( writer => { - const nextRowIndex = last + 1; + const rowAfterRemovedSection = last + 1; - moveCellsToNextRow( table, nextRowIndex, cellsToMove, writer ); + moveCellsToRow( table, rowAfterRemovedSection, cellsToMove, writer ); for ( let i = last; i >= first; i-- ) { writer.remove( table.getChild( i ) ); @@ -323,7 +326,7 @@ export default class TableUtils extends Plugin { updateNumericAttribute( 'rowspan', rowspan, cell, writer ); } - updateHeadingRows( first, last, model, writer, table ); + updateHeadingRows( table, first, last, model, writer.batch ); } ); } @@ -750,15 +753,6 @@ function breakSpanEvenly( span, numberOfCells ) { return { newCellsSpan, updatedSpan }; } -// Calculates a new heading rows value for removing rows from heading section. -function getNewHeadingRowsValue( first, last, headingRows ) { - if ( last < headingRows ) { - return headingRows - ( last - first + 1 ); - } - - return first; -} - // Updates heading columns attribute if removing a row from head section. function adjustHeadingColumns( table, removedColumnIndexes, writer ) { const headingColumns = table.getAttribute( 'headingColumns' ) || 0; @@ -771,27 +765,28 @@ function adjustHeadingColumns( table, removedColumnIndexes, writer ) { } } -function updateHeadingRows( first, last, model, writer, table ) { +// Calculates a new heading rows value for removing rows from heading section. +function updateHeadingRows( table, first, last, model, batch ) { const headingRows = table.getAttribute( 'headingRows' ) || 0; - if ( headingRows && first < headingRows ) { - const newRows = getNewHeadingRowsValue( first, last, headingRows ); + if ( first < headingRows ) { + const newRows = last < headingRows ? headingRows - ( last - first + 1 ) : first; // Must be done after the changes in table structure (removing rows). // Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391. - model.enqueueChange( writer.batch, writer => { + model.enqueueChange( batch, writer => { updateNumericAttribute( 'headingRows', newRows, table, writer, 0 ); } ); } } -function moveCellsToNextRow( table, nextRowIndex, cellsToMove, writer ) { +function moveCellsToRow( table, targetRowIndex, cellsToMove, writer ) { const tableWalker = new TableWalker( table, { includeSpanned: true, - startRow: nextRowIndex, - endRow: nextRowIndex + startRow: targetRowIndex, + endRow: targetRowIndex } ); - const row = table.getChild( nextRowIndex ); + const row = table.getChild( targetRowIndex ); let previousCell; @@ -808,6 +803,7 @@ function moveCellsToNextRow( table, nextRowIndex, cellsToMove, writer ) { previousCell = cellToMove; } else if ( !isSpanned ) { + // If cell is spanned then `cell` holds reference to overlapping cell. See ckeditor/ckeditor5#6502. previousCell = cell; } } From e353a4abbce8035d3da117f2e97e3e589c185347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 14:39:30 +0200 Subject: [PATCH 05/21] Add comments to remove rows algorithm. --- src/tableutils.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index 035a1c06..cbb67103 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -288,9 +288,13 @@ export default class TableUtils extends Plugin { const first = options.at; const last = first + rowsToRemove - 1; - // Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. + // Removing rows from table requires most calculations to be done prior to changing table structure. + + // 1. Preparation + + // 1a. Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. const cellsToMove = new Map(); - // Similarly, if a cell is "above" removed rows sections we must trim their rowspan. + // 1b. Similarly, if a cell is "above" removed rows sections we must trim their rowspan. const cellsToTrim = []; for ( const { row, column, rowspan, cell } of new TableWalker( table, { endRow: last } ) ) { @@ -313,19 +317,24 @@ export default class TableUtils extends Plugin { } } + // 2. Execution model.change( writer => { + // 2a. Move cells from removed rows that extends over a removed section - must be done before removing rows. + // This will fill any gaps in a rows below that previously were empty because of row-spanned cells. const rowAfterRemovedSection = last + 1; - moveCellsToRow( table, rowAfterRemovedSection, cellsToMove, writer ); + // 2b. Remove all required rows. for ( let i = last; i >= first; i-- ) { writer.remove( table.getChild( i ) ); } + // 2c. Update cells from rows above that overlaps removed section. Similar to step 2 but does not involve moving cells. for ( const { rowspan, cell } of cellsToTrim ) { updateNumericAttribute( 'rowspan', rowspan, cell, writer ); } + // 2d. Adjust heading rows if removed rows were in a heading section. updateHeadingRows( table, first, last, model, writer.batch ); } ); } From 33b3cd0910e0b7c2f8b67e43c38d6dcb37ce864c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 14:51:56 +0200 Subject: [PATCH 06/21] Update remove rows test cases. --- tests/tableutils.js | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index d112990a..6a5c5d93 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -939,33 +939,37 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should properly calculate truncated rowspans', () => { + it( 'should move row-spaned cells to a row after removed rows section', () => { setData( model, modelTable( [ - [ '00', { contents: '01', rowspan: 3 } ], - [ '10' ], - [ '20' ] + [ '00', '01', '02', '03' ], + [ { rowspan: 4, contents: '10' }, { rowspan: 3, contents: '11' }, { rowspan: 2, contents: '12' }, '13' ], + [ '23' ], + [ '32', '33' ], + [ '41', '42', '43' ] ] ) ); - tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 2 } ); assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ - [ '20', '01' ] + [ '00', '01', '02', '03' ], + [ { rowspan: 2, contents: '10' }, '11', '32', '33' ], + [ '41', '42', '43' ] ] ) ); } ); it( 'should decrease rowspan of table cells from rows before removed rows section', () => { setData( model, modelTable( [ - [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], - [ '13', '14' ], - [ '22', '23', '24' ], - [ '31', '32', '33', '34' ] + [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03' ], + [ '13' ], + [ '22', '23' ], + [ '31', '32', '33' ] ] ) ); tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 2 } ); assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ - [ { rowspan: 2, contents: '00' }, '01', '02', '03', '04' ], - [ '31', '32', '33', '34' ] + [ { rowspan: 2, contents: '00' }, '01', '02', '03' ], + [ '31', '32', '33' ] ] ) ); } ); From 52d6f4238bc6cf9aa2cfd7479e32097566e5d1e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 15:19:01 +0200 Subject: [PATCH 07/21] Add special case for handling spanned cells over removed rows. --- tests/tableutils.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index 6a5c5d93..1c211239 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -943,17 +943,17 @@ describe( 'TableUtils', () => { setData( model, modelTable( [ [ '00', '01', '02', '03' ], [ { rowspan: 4, contents: '10' }, { rowspan: 3, contents: '11' }, { rowspan: 2, contents: '12' }, '13' ], - [ '23' ], - [ '32', '33' ], - [ '41', '42', '43' ] + [ { rowspan: 3, contents: '23' } ], + [ '32' ], + [ '41', '42' ] ] ) ); tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 2 } ); assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ [ '00', '01', '02', '03' ], - [ { rowspan: 2, contents: '10' }, '11', '32', '33' ], - [ '41', '42', '43' ] + [ { rowspan: 2, contents: '10' }, '11', '32', { rowspan: 2, contents: '23' } ], + [ '41', '42' ] ] ) ); } ); From 91b8ba65923b0c31729218bfb79b62f03207b020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 15:34:28 +0200 Subject: [PATCH 08/21] Use TableEditing in some of table utils tests. See ckeditor/ckeditor5#6574. --- tests/tableutils.js | 155 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 140 insertions(+), 15 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index 1c211239..946a34c0 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -8,37 +8,53 @@ import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model import { defaultConversion, defaultSchema, modelTable } from './_utils/utils'; +import TableEditing from '../src/tableediting'; import TableUtils from '../src/tableutils'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; describe( 'TableUtils', () => { let editor, model, root, tableUtils; - beforeEach( () => { - return ModelTestEditor.create( { - plugins: [ TableUtils ] - } ).then( newEditor => { - editor = newEditor; - model = editor.model; - root = model.document.getRoot( 'main' ); - tableUtils = editor.plugins.get( TableUtils ); - - defaultSchema( model.schema ); - defaultConversion( editor.conversion ); - } ); - } ); - afterEach( () => { return editor.destroy(); } ); describe( '#pluginName', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should provide plugin name', () => { expect( TableUtils.pluginName ).to.equal( 'TableUtils' ); } ); } ); describe( 'getCellLocation()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should return proper table cell location', () => { setData( model, modelTable( [ [ { rowspan: 2, colspan: 2, contents: '00[]' }, '02' ], @@ -52,6 +68,20 @@ describe( 'TableUtils', () => { } ); describe( 'insertRows()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should insert row in given table at given index', () => { setData( model, modelTable( [ [ '11[]', '12' ], @@ -192,6 +222,20 @@ describe( 'TableUtils', () => { } ); describe( 'insertColumns()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should insert column in given table at given index', () => { setData( model, modelTable( [ [ '11[]', '12' ], @@ -390,6 +434,20 @@ describe( 'TableUtils', () => { } ); describe( 'splitCellVertically()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should split table cell to given table cells number', () => { setData( model, modelTable( [ [ '00', '01', '02' ], @@ -534,6 +592,20 @@ describe( 'TableUtils', () => { } ); describe( 'splitCellHorizontally()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should split table cell to default table cells number', () => { setData( model, modelTable( [ [ '00', '01', '02' ], @@ -709,6 +781,20 @@ describe( 'TableUtils', () => { } ); describe( 'getColumns()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should return proper number of columns', () => { setData( model, modelTable( [ [ '00', { colspan: 3, contents: '01' }, '04' ] @@ -719,6 +805,20 @@ describe( 'TableUtils', () => { } ); describe( 'getRows()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should return proper number of columns for simple table', () => { setData( model, modelTable( [ [ '00', '01' ], @@ -749,6 +849,17 @@ describe( 'TableUtils', () => { } ); describe( 'removeRows()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ Paragraph, TableEditing, TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + } ); + } ); + describe( 'single row', () => { it( 'should remove a given row from a table start', () => { setData( model, modelTable( [ @@ -797,7 +908,7 @@ describe( 'TableUtils', () => { setData( model, modelTable( [ [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ { rowspan: 2, contents: '13' }, '14' ], - [ '22', '23', '24' ], + [ '22', '24' ], [ '31', '32', '33', '34' ] ] ) ); @@ -997,6 +1108,20 @@ describe( 'TableUtils', () => { } ); describe( 'removeColumns()', () => { + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( TableUtils ); + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + describe( 'single row', () => { it( 'should remove a given column', () => { setData( model, modelTable( [ From bbe2740013bca15593b5fa0020f1431d6be131be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 10 Apr 2020 15:40:05 +0200 Subject: [PATCH 09/21] Use cached table map because modifying table during TableWalker breaks the logic. --- src/tableutils.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tableutils.js b/src/tableutils.js index cbb67103..f2259ab4 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -795,11 +795,13 @@ function moveCellsToRow( table, targetRowIndex, cellsToMove, writer ) { startRow: targetRowIndex, endRow: targetRowIndex } ); + + const tableRowMap = [ ...tableWalker ]; const row = table.getChild( targetRowIndex ); let previousCell; - for ( const { column, cell, isSpanned } of tableWalker ) { + for ( const { column, cell, isSpanned } of tableRowMap ) { if ( cellsToMove.has( column ) ) { const { cell: cellToMove, rowspan } = cellsToMove.get( column ); From 71c6334a1aabcebd9a134bf91afab8fa9f1114e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 14:21:53 +0200 Subject: [PATCH 10/21] Fix sample in codeblock. --- src/tableutils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index f2259ab4..d242ecd4 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -267,9 +267,9 @@ export default class TableUtils extends Plugin { * ┌───┬───┬───┐ `at` = 1 ┌───┬───┬───┐ * 0 │ a │ b │ c │ `rows` = 2 │ a │ b │ c │ 0 * │ ├───┼───┤ │ ├───┼───┤ - * 1 │ │ d │ e │ <-- remove from here │ │ d │ i │ 1 + * 1 │ │ d │ e │ <-- remove from here │ │ d │ g │ 1 * │ │ ├───┤ will give: ├───┼───┼───┤ - * 2 │ │ │ f │ │ j │ k │ l │ 2 + * 2 │ │ │ f │ │ h │ i │ j │ 2 * │ │ ├───┤ └───┴───┴───┘ * 3 │ │ │ g │ * ├───┼───┼───┤ From ebb7ff4b7b41e676d1f23c543c1b9074758d1b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 15:19:27 +0200 Subject: [PATCH 11/21] Update TableUtils.removeRow test cases. --- tests/tableutils.js | 88 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/tableutils.js b/tests/tableutils.js index 946a34c0..4d35def3 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -905,6 +905,52 @@ describe( 'TableUtils', () => { } ); it( 'should decrease rowspan of table cells from previous rows', () => { + // +----+----+----+----+----+ + // | 00 | 01 | 02 | 03 | 04 | + // +----+ + + + + + // | 10 | | | | | + // +----+----+ + + + + // | 20 | 21 | | | | + // +----+----+----+ + + + // | 30 | 31 | 32 | | | + // +----+----+----+----+ + + // | 40 | 41 | 42 | 43 | | + // +----+----+----+----+----+ + // | 50 | 51 | 52 | 53 | 44 | + // +----+----+----+----+----+ + setData( model, modelTable( [ + [ '00', { contents: '01', rowspan: 2 }, { contents: '02', rowspan: 3 }, { contents: '03', rowspan: 4 }, + { contents: '04', rowspan: 5 } ], + [ '10' ], + [ '20', '21' ], + [ '30', '31', '32' ], + [ '40', '41', '42', '43' ], + [ '50', '51', '52', '53', '54' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 1 } ); + + // +----+----+----+----+----+ + // | 00 | 01 | 02 | 03 | 04 | + // +----+----+ + + + + // | 20 | 21 | | | | + // +----+----+----+ + + + // | 30 | 31 | 32 | | | + // +----+----+----+----+ + + // | 40 | 41 | 42 | 43 | | + // +----+----+----+----+----+ + // | 50 | 51 | 52 | 53 | 44 | + // +----+----+----+----+----+ + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01', { contents: '02', rowspan: 2 }, { contents: '03', rowspan: 3 }, { contents: '04', rowspan: 4 } ], + [ '20', '21' ], + [ '30', '31', '32' ], + [ '40', '41', '42', '43' ], + [ '50', '51', '52', '53', '54' ] + ] ) ); + } ); + + it( 'should decrease rowspan of table cells from previous rows (rowspaned cells on different rows)', () => { setData( model, modelTable( [ [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ { rowspan: 2, contents: '13' }, '14' ], @@ -1084,6 +1130,48 @@ describe( 'TableUtils', () => { ] ) ); } ); + it( 'should decrease rowspan of table cells from previous rows', () => { + // +----+----+----+----+----+ + // | 00 | 01 | 02 | 03 | 04 | + // +----+ + + + + + // | 10 | | | | | + // +----+----+ + + + + // | 20 | 21 | | | | + // +----+----+----+ + + + // | 30 | 31 | 32 | | | + // +----+----+----+----+ + + // | 40 | 41 | 42 | 43 | | + // +----+----+----+----+----+ + // | 50 | 51 | 52 | 53 | 44 | + // +----+----+----+----+----+ + setData( model, modelTable( [ + [ '00', { contents: '01', rowspan: 2 }, { contents: '02', rowspan: 3 }, { contents: '03', rowspan: 4 }, + { contents: '04', rowspan: 5 } ], + [ '10' ], + [ '20', '21' ], + [ '30', '31', '32' ], + [ '40', '41', '42', '43' ], + [ '50', '51', '52', '53', '54' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 2, rows: 2 } ); + + // +----+----+----+----+----+ + // | 00 | 01 | 02 | 03 | 04 | + // +----+ + + + + + // | 10 | | | | | + // +----+----+----+----+ + + // | 40 | 41 | 42 | 43 | | + // +----+----+----+----+----+ + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', { contents: '01', rowspan: 2 }, { contents: '02', rowspan: 2 }, { contents: '03', rowspan: 2 }, + { contents: '04', rowspan: 3 } ], + [ '10' ], + [ '40', '41', '42', '43' ], + [ '50', '51', '52', '53', '54' ] + ] ) ); + } ); + it( 'should create one undo step (1 batch)', () => { setData( model, modelTable( [ [ '00', '01' ], From c4bfddb6ff535863a57c20ffa20a9c435ee9c045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 15:24:37 +0200 Subject: [PATCH 12/21] Fix the logic behind adjusting cell's rowspan on removing row. --- src/tableutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tableutils.js b/src/tableutils.js index d242ecd4..e643b216 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -311,7 +311,7 @@ export default class TableUtils extends Plugin { const isCellOverlappingRemovedRows = row < first && lastRowOfCell >= first; if ( isCellOverlappingRemovedRows ) { - const rowspanAdjustment = lastRowOfCell >= last ? rowsToRemove : first - row; + const rowspanAdjustment = lastRowOfCell >= last ? rowsToRemove : lastRowOfCell - first + 1; const rowSpanToSet = rowspan - rowspanAdjustment; cellsToTrim.push( { cell, rowspan: rowSpanToSet } ); } From 2a3ff7a0eb386a2788cd12bb83945e6edd6e1143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 17:50:52 +0200 Subject: [PATCH 13/21] Refactor TableUtils.removeRow() to make algorithm less mangled. --- src/tableutils.js | 91 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index e643b216..74411450 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -290,32 +290,8 @@ export default class TableUtils extends Plugin { // Removing rows from table requires most calculations to be done prior to changing table structure. - // 1. Preparation - - // 1a. Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. - const cellsToMove = new Map(); - // 1b. Similarly, if a cell is "above" removed rows sections we must trim their rowspan. - const cellsToTrim = []; - - for ( const { row, column, rowspan, cell } of new TableWalker( table, { endRow: last } ) ) { - const rowspanInRemovedSection = last - row + 1; - const lastRowOfCell = row + rowspan - 1; - - const isCellStickingOutFromRemovedRows = row >= first && row <= last && lastRowOfCell > last; - - if ( isCellStickingOutFromRemovedRows ) { - const rowSpanToSet = rowspan - rowspanInRemovedSection; - cellsToMove.set( column, { cell, rowspan: rowSpanToSet } ); - } - - const isCellOverlappingRemovedRows = row < first && lastRowOfCell >= first; - - if ( isCellOverlappingRemovedRows ) { - const rowspanAdjustment = lastRowOfCell >= last ? rowsToRemove : lastRowOfCell - first + 1; - const rowSpanToSet = rowspan - rowspanAdjustment; - cellsToTrim.push( { cell, rowspan: rowSpanToSet } ); - } - } + // 1. Preparation - get row-spanned cells that have to be modified after removing rows. + const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, last, first, rowsToRemove ); // 2. Execution model.change( writer => { @@ -789,6 +765,69 @@ function updateHeadingRows( table, first, last, model, batch ) { } } +// Finds cells that will be: +// - trimmed - Cells that are "above" removed rows sections and overlaps removed section - their rowspan must be trimmed. +// - moved - Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. +// +// Sample table with overlapping & sticking out cells: +// +// +----+----+----+----+----+ +// | 00 | 01 | 02 | 03 | 04 | +// +----+ + + + + +// | 10 | | | | | +// +----+----+ + + + +// | 20 | 21 | | | | <-- removed row +// + + +----+ + + +// | | | 32 | | | <-- removed row +// +----+ + +----+ + +// | 40 | | | 43 | | +// +----+----+----+----+----+ +// +// In a table above: +// - cells to trim: '02', '03' & '04'. +// - cells to move: '21' & '32'. +function getCellsToMoveAndTrimOnRemoveRow( table, last, first, rowsToRemove ) { + const cellsToMove = new Map(); + const cellsToTrim = []; + + for ( const { row, column, rowspan, cell } of new TableWalker( table, { endRow: last } ) ) { + const lastRowOfCell = row + rowspan - 1; + + const isCellStickingOutFromRemovedRows = row >= first && row <= last && lastRowOfCell > last; + + if ( isCellStickingOutFromRemovedRows ) { + const rowspanInRemovedSection = last - row + 1; + const rowSpanToSet = rowspan - rowspanInRemovedSection; + + cellsToMove.set( column, { + cell, + rowspan: rowSpanToSet + } ); + } + + const isCellOverlappingRemovedRows = row < first && lastRowOfCell >= first; + + if ( isCellOverlappingRemovedRows ) { + let rowspanAdjustment; + + // Cell fully covers removed section - trim it by removed rows count. + if ( lastRowOfCell >= last ) { + rowspanAdjustment = rowsToRemove; + } + // Cell partially overlaps removed section - calculate cell's span that is in removed section. + else { + rowspanAdjustment = lastRowOfCell - first + 1; + } + + cellsToTrim.push( { + cell, + rowspan: rowspan - rowspanAdjustment + } ); + } + } + return { cellsToMove, cellsToTrim }; +} + function moveCellsToRow( table, targetRowIndex, cellsToMove, writer ) { const tableWalker = new TableWalker( table, { includeSpanned: true, From a4a048c7f3a7ad350471d981c62e1b2a389af963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 17:52:06 +0200 Subject: [PATCH 14/21] Fix code comments in table utils tests. --- tests/tableutils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index 4d35def3..f7b2d81a 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -916,7 +916,7 @@ describe( 'TableUtils', () => { // +----+----+----+----+ + // | 40 | 41 | 42 | 43 | | // +----+----+----+----+----+ - // | 50 | 51 | 52 | 53 | 44 | + // | 50 | 51 | 52 | 53 | 54 | // +----+----+----+----+----+ setData( model, modelTable( [ [ '00', { contents: '01', rowspan: 2 }, { contents: '02', rowspan: 3 }, { contents: '03', rowspan: 4 }, @@ -939,7 +939,7 @@ describe( 'TableUtils', () => { // +----+----+----+----+ + // | 40 | 41 | 42 | 43 | | // +----+----+----+----+----+ - // | 50 | 51 | 52 | 53 | 44 | + // | 50 | 51 | 52 | 53 | 54 | // +----+----+----+----+----+ assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ [ '00', '01', { contents: '02', rowspan: 2 }, { contents: '03', rowspan: 3 }, { contents: '04', rowspan: 4 } ], From 6e934c12e4c05310f13bab58d7c68cb1c428d3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 17:54:24 +0200 Subject: [PATCH 15/21] Typo fix. --- tests/tableutils.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index f7b2d81a..6f27c828 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -743,7 +743,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should split rowspanned & colspaned cell', () => { + it( 'should split rowspanned & colspanned cell', () => { setData( model, modelTable( [ [ '00', { colspan: 2, contents: '01[]' } ], [ '10', '11' ] @@ -950,7 +950,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should decrease rowspan of table cells from previous rows (rowspaned cells on different rows)', () => { + it( 'should decrease rowspan of table cells from previous rows (row-spanned cells on different rows)', () => { setData( model, modelTable( [ [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ { rowspan: 2, contents: '13' }, '14' ], @@ -967,7 +967,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should move rowspaned cells to a row below removing it\'s row', () => { + it( 'should move row-spanned cells to a row below removing it\'s row', () => { setData( model, modelTable( [ [ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, '02' ], [ '12' ], @@ -984,7 +984,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should move rowspaned cells to a row below removing it\'s row (other cell is overlapping removed row)', () => { + it( 'should move row-spanned cells to a row below removing it\'s row (other cell is overlapping removed row)', () => { setData( model, modelTable( [ [ '00', { rowspan: 3, contents: '01' }, '02', '03', '04' ], [ '10', { rowspan: 2, contents: '12' }, '13', '14' ], @@ -1096,7 +1096,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should move row-spaned cells to a row after removed rows section', () => { + it( 'should move row-spanned cells to a row after removed rows section', () => { setData( model, modelTable( [ [ '00', '01', '02', '03' ], [ { rowspan: 4, contents: '10' }, { rowspan: 3, contents: '11' }, { rowspan: 2, contents: '12' }, '13' ], From d04641fb9edf4ab5c6d05cfe0eda8d544e8f84ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 15 Apr 2020 17:56:06 +0200 Subject: [PATCH 16/21] Use same row-/col-spanned naming in tests. --- tests/tableutils.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index 6f27c828..d6c831ea 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -414,7 +414,7 @@ describe( 'TableUtils', () => { ], { headingColumns: 4 } ) ); } ); - it( 'should properly insert column while table has rowspanned cells', () => { + it( 'should properly insert column while table has row-spanned cells', () => { setData( model, modelTable( [ [ { rowspan: 4, contents: '00[]' }, { rowspan: 2, contents: '01' }, '02' ], [ '12' ], @@ -642,7 +642,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should properly update rowspanned cells overlapping selected cell', () => { + it( 'should properly update row-spanned cells overlapping selected cell', () => { setData( model, modelTable( [ [ { rowspan: 2, contents: '00' }, '01', { rowspan: 3, contents: '02' } ], [ '[]11' ], @@ -660,7 +660,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should split rowspanned cell', () => { + it( 'should split row-spanned cell', () => { setData( model, modelTable( [ [ '00', { rowspan: 2, contents: '01[]' } ], [ '10' ], @@ -678,7 +678,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should copy colspan while splitting rowspanned cell', () => { + it( 'should copy colspan while splitting row-spanned cell', () => { setData( model, modelTable( [ [ '00', { rowspan: 2, colspan: 2, contents: '01[]' } ], [ '10' ], @@ -724,7 +724,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should split rowspanned cell and updated other cells rowspan when splitting to bigger number of cells', () => { + it( 'should split row-spanned cell and updated other cells rowspan when splitting to bigger number of cells', () => { setData( model, modelTable( [ [ '00', { rowspan: 2, contents: '01[]' } ], [ '10' ], @@ -743,7 +743,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should split rowspanned & colspanned cell', () => { + it( 'should split row-spanned & col-spanned cell', () => { setData( model, modelTable( [ [ '00', { colspan: 2, contents: '01[]' } ], [ '10', '11' ] @@ -1324,7 +1324,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should remove column if other column is rowspanned (last column)', () => { + it( 'should remove column if other column is row-spanned (last column)', () => { setData( model, modelTable( [ [ '00', { rowspan: 2, contents: '01' } ], [ '10' ] @@ -1337,7 +1337,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should remove column if other column is rowspanned (first column)', () => { + it( 'should remove column if other column is row-spanned (first column)', () => { setData( model, modelTable( [ [ { rowspan: 2, contents: '00' }, '01' ], [ '11' ] From 3fb72312aea56b423a80b2037813f96b976201ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 16 Apr 2020 09:58:07 +0200 Subject: [PATCH 17/21] Fix table example in comments. --- tests/tableutils.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/tableutils.js b/tests/tableutils.js index d6c831ea..b02d8c4d 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -1142,7 +1142,7 @@ describe( 'TableUtils', () => { // +----+----+----+----+ + // | 40 | 41 | 42 | 43 | | // +----+----+----+----+----+ - // | 50 | 51 | 52 | 53 | 44 | + // | 50 | 51 | 52 | 53 | 54 | // +----+----+----+----+----+ setData( model, modelTable( [ [ '00', { contents: '01', rowspan: 2 }, { contents: '02', rowspan: 3 }, { contents: '03', rowspan: 4 }, @@ -1163,6 +1163,8 @@ describe( 'TableUtils', () => { // +----+----+----+----+ + // | 40 | 41 | 42 | 43 | | // +----+----+----+----+----+ + // | 50 | 51 | 52 | 53 | 54 | + // +----+----+----+----+----+ assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ [ '00', { contents: '01', rowspan: 2 }, { contents: '02', rowspan: 2 }, { contents: '03', rowspan: 2 }, { contents: '04', rowspan: 3 } ], From 75f6f35e439be9cef414aef03ad15c454a5083e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 16 Apr 2020 10:24:53 +0200 Subject: [PATCH 18/21] Refactor internal getCellsToMoveAndTrimOnRemoveRow() function. --- src/tableutils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index 74411450..b062f9b7 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -291,7 +291,7 @@ export default class TableUtils extends Plugin { // Removing rows from table requires most calculations to be done prior to changing table structure. // 1. Preparation - get row-spanned cells that have to be modified after removing rows. - const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, last, first, rowsToRemove ); + const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, first, last ); // 2. Execution model.change( writer => { @@ -786,7 +786,7 @@ function updateHeadingRows( table, first, last, model, batch ) { // In a table above: // - cells to trim: '02', '03' & '04'. // - cells to move: '21' & '32'. -function getCellsToMoveAndTrimOnRemoveRow( table, last, first, rowsToRemove ) { +function getCellsToMoveAndTrimOnRemoveRow( table, first, last ) { const cellsToMove = new Map(); const cellsToTrim = []; @@ -812,7 +812,7 @@ function getCellsToMoveAndTrimOnRemoveRow( table, last, first, rowsToRemove ) { // Cell fully covers removed section - trim it by removed rows count. if ( lastRowOfCell >= last ) { - rowspanAdjustment = rowsToRemove; + rowspanAdjustment = last - first + 1; } // Cell partially overlaps removed section - calculate cell's span that is in removed section. else { From 1df89fb25b5f7233a2b5d186755300c3dc297cd4 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 16 Apr 2020 12:12:13 +0200 Subject: [PATCH 19/21] Update src/tableutils.js --- src/tableutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tableutils.js b/src/tableutils.js index b062f9b7..9e391af9 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -305,7 +305,7 @@ export default class TableUtils extends Plugin { writer.remove( table.getChild( i ) ); } - // 2c. Update cells from rows above that overlaps removed section. Similar to step 2 but does not involve moving cells. + // 2c. Update cells from rows above that overlap removed section. Similar to step 2 but does not involve moving cells. for ( const { rowspan, cell } of cellsToTrim ) { updateNumericAttribute( 'rowspan', rowspan, cell, writer ); } From b0bd21a8ce1635adab1b9865285981c06fffe4c3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 16 Apr 2020 12:13:19 +0200 Subject: [PATCH 20/21] Update src/tableutils.js --- src/tableutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tableutils.js b/src/tableutils.js index 9e391af9..e80d1e64 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -766,7 +766,7 @@ function updateHeadingRows( table, first, last, model, batch ) { } // Finds cells that will be: -// - trimmed - Cells that are "above" removed rows sections and overlaps removed section - their rowspan must be trimmed. +// - trimmed - Cells that are "above" removed rows sections and overlap the removed section - their rowspan must be trimmed. // - moved - Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. // // Sample table with overlapping & sticking out cells: From b04996dfa57f8e50221e8e38da004efc9e560188 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 16 Apr 2020 12:14:01 +0200 Subject: [PATCH 21/21] Update src/tableutils.js --- src/tableutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tableutils.js b/src/tableutils.js index e80d1e64..dad1ab2b 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -767,7 +767,7 @@ function updateHeadingRows( table, first, last, model, batch ) { // Finds cells that will be: // - trimmed - Cells that are "above" removed rows sections and overlap the removed section - their rowspan must be trimmed. -// - moved - Cells from removed rows section might stick out of. Such cells are moved to a next row after a removed section. +// - moved - Cells from removed rows section might stick out of. These cells are moved to the next row after a removed section. // // Sample table with overlapping & sticking out cells: //