Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Introduced support for removing multiple rows / columns using multi-cell selection #261

Merged
merged 42 commits into from
Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0e92c13
Tests: Added unit tests for integration row removal with table select…
mlewand Mar 2, 2020
90adf35
Internal: Enable row removal in case of multi-cell table selection.
mlewand Mar 2, 2020
1552f26
Tests: Added basic test cases for removing rows with multiple selecte…
mlewand Mar 2, 2020
9fa8890
Internal: Adjusted row removing command so that first of the new mult…
mlewand Mar 2, 2020
ce6c30b
Internal: Extracted _removeRow method.
mlewand Mar 2, 2020
fb16e69
Tests: Added more detailed unit tests for remove rows command and mul…
mlewand Mar 3, 2020
310a9e4
Internal: Introduced support for removing multiple rows using RemoveR…
mlewand Mar 3, 2020
5af4b85
Internal: Improved code coverage.
mlewand Mar 3, 2020
b4e04a5
Internal: Minor refactor to clean up the code. Remove stray parameters.
mlewand Mar 3, 2020
23d3a40
Tests: Added test coverage for removing rows with mixed header and re…
mlewand Mar 3, 2020
72bc3c3
Tests: Added unit test for column removal command availability in cas…
mlewand Mar 3, 2020
9baca00
Internal: Enable column removing for multi-cell selection.
mlewand Mar 3, 2020
1fc5458
Tests: Added unit tests for removing column when multiple cells in a …
mlewand Mar 3, 2020
f29e4ed
Internal: Introduced column removing for a single column multi-cell s…
mlewand Mar 3, 2020
e835295
Tests: Unit tests for multiple column removal.
mlewand Mar 3, 2020
c4ebf7a
Internal: Multiple columns can now be removed.
mlewand Mar 3, 2020
734a8bd
Internal: Removing table column also modifies heading columns accordi…
mlewand Mar 5, 2020
9859193
Internal: Column remove command should not be active if your selectio…
mlewand Mar 6, 2020
206ee4d
Merge branch 'master' into i/6126
mlewand Mar 6, 2020
25937a6
Internal: Adjusted the code to recent table selection changes on mast…
mlewand Mar 6, 2020
24aeae8
Internal: Adjusted the code to recent table selection changes on mast…
mlewand Mar 6, 2020
2a1c019
Internal: Added a helper to get number of rows in the table.
mlewand Mar 6, 2020
a1158c4
Internal: Remove row command enabled state for multiple cell selectio…
mlewand Mar 6, 2020
94b6a99
Docs: API docs for private _removeRow method.
mlewand Mar 6, 2020
fd15692
Internal: A workaround for converter issue (ckeditor/ckeditor5#6391).
mlewand Mar 9, 2020
f2cef8f
Internal: Fixed off by one error when removing row headers.
mlewand Mar 9, 2020
774b29a
Internal: Prereview suggestions addressed.
mlewand Mar 9, 2020
ce22fbb
Internal: Adding an explanation why multiple model.change() calls are…
mlewand Mar 9, 2020
7af267e
Internal: Worked around the problem that removing multiple rows resul…
mlewand Mar 9, 2020
6a5eb47
Other: Removed explicitly passed default argument.
mlewand Mar 9, 2020
e6a5be8
Internal: Fixed cases where inverted selections would not work correc…
mlewand Mar 9, 2020
a42ae23
Merge branch 'master' into i/6126
mlewand Mar 10, 2020
55aa630
Internal: Reused recently added `getSelectionAffectedTableCells()` fu…
mlewand Mar 10, 2020
bd4c3dc
Internal: Minor code simplification.
mlewand Mar 10, 2020
a110583
Internal: Some further removerowcommand reorganization.
mlewand Mar 10, 2020
da4fbf3
Internal: Reorganized code in remove column command.
mlewand Mar 10, 2020
42121d3
Merge branch 'master' into i/6126
mlewand Mar 11, 2020
01572f4
Tests: Added unit test for https://github.com/ckeditor/ckeditor5-tabl…
mlewand Mar 11, 2020
7db145f
Tests: Moved similar unit test to more appropriate descr group.
mlewand Mar 11, 2020
b2b364d
Tests: Added unit test for removing a row with a simple (collapsed) s…
mlewand Mar 11, 2020
af62ac7
Internal: Fixed a case where editor would crash when removing a row w…
mlewand Mar 11, 2020
9bb7439
Merge branch 'master' into i/6126
mlewand Mar 11, 2020
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
83 changes: 56 additions & 27 deletions src/commands/removecolumncommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,22 @@ export default class RemoveColumnCommand extends Command {
* @inheritDoc
*/
refresh() {
const editor = this.editor;
const selection = editor.model.document.selection;
const tableUtils = editor.plugins.get( 'TableUtils' );
const tableUtils = this.editor.plugins.get( 'TableUtils' );
const firstCell = this._getReferenceCells().next().value;

const tableCell = findAncestor( 'tableCell', selection.getFirstPosition() );

this.isEnabled = !!tableCell && tableUtils.getColumns( tableCell.parent.parent ) > 1;
this.isEnabled = !!firstCell && tableUtils.getColumns( firstCell.parent.parent ) > 1;
}

/**
* @inheritDoc
*/
execute() {
const model = this.editor.model;
const selection = model.document.selection;

const firstPosition = selection.getFirstPosition();

const tableCell = findAncestor( 'tableCell', firstPosition );
const tableRow = tableCell.parent;
const referenceCells = Array.from( this._getReferenceCells() );
const firstCell = referenceCells[ 0 ];
const lastCell = referenceCells[ referenceCells.length - 1 ];
const tableRow = firstCell.parent;
const table = tableRow.parent;

const headingColumns = table.getAttribute( 'headingColumns' ) || 0;
Expand All @@ -56,41 +52,74 @@ export default class RemoveColumnCommand extends Command {
const tableMap = [ ...new TableWalker( table ) ];

// Get column index of removed column.
const cellData = tableMap.find( value => value.cell === tableCell );
const removedColumn = cellData.column;
const selectionRow = cellData.row;
const cellToFocus = getCellToFocus( tableCell );
const firstCellData = tableMap.find( value => value.cell === firstCell );
const selectionRow = firstCellData.row;
const cellsToFocus = getCellToFocus( firstCell, lastCell );

const removedColumnIndexes = {
first: firstCellData.column,
last: tableMap.find( value => value.cell === lastCell ).column
};

model.change( writer => {
// A temporary workaround to avoid the "model-selection-range-intersects" error.
writer.setSelection( writer.createSelection( table, 'on' ) );

// Update heading columns attribute if removing a row from head section.
if ( headingColumns && selectionRow <= headingColumns ) {
writer.setAttribute( 'headingColumns', headingColumns - 1, table );
}

for ( const { cell, column, colspan } of tableMap ) {
// If colspaned cell overlaps removed column decrease it's span.
if ( column <= removedColumn && colspan > 1 && column + colspan > removedColumn ) {
updateNumericAttribute( 'colspan', colspan - 1, cell, writer );
} else if ( column === removedColumn ) {
// The cell in removed column has colspan of 1.
writer.remove( cell );
for (
let removedColumnIndex = removedColumnIndexes.last;
removedColumnIndex >= removedColumnIndexes.first;
removedColumnIndex--
) {
for ( const { cell, column, colspan } of tableMap ) {
// If colspaned cell overlaps removed column decrease it's span.
if ( column <= removedColumnIndex && colspan > 1 && column + colspan > removedColumnIndex ) {
updateNumericAttribute( 'colspan', colspan - 1, cell, writer );
} else if ( column === removedColumnIndex ) {
// The cell in removed column has colspan of 1.
writer.remove( cell );
}
}
}

writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
writer.setSelection( writer.createPositionAt( cellsToFocus.reverse().filter( item => item != null )[ 0 ], 0 ) );
} );
}

/**
* Returns cells that are selected and are a reference to removing rows.
*
* @private
* @returns {Iterable.<module:engine/model/element~Element>} Generates `tableCell` elements.
*/
* _getReferenceCells() {
const plugins = this.editor.plugins;
if ( plugins.has( 'TableSelection' ) && plugins.get( 'TableSelection' ).hasMultiCellSelection ) {
for ( const cell of plugins.get( 'TableSelection' ).getSelectedTableCells() ) {
yield cell;
}
} else {
const selection = this.editor.model.document.selection;

yield findAncestor( 'tableCell', selection.getFirstPosition() );
}
}
}

// Returns a proper table cell to focus after removing a column. It should be a next sibling to selection visually stay in place but:
// - selection is on last table cell it will return previous cell.
// - table cell is spanned over 2+ columns - it will be truncated so the selection should stay in that cell.
function getCellToFocus( tableCell ) {
const colspan = parseInt( tableCell.getAttribute( 'colspan' ) || 1 );
function getCellToFocus( firstCell, lastCell ) {
const colspan = parseInt( lastCell.getAttribute( 'colspan' ) || 1 );

if ( colspan > 1 ) {
return tableCell;
return [ firstCell, lastCell ];
}

return tableCell.nextSibling ? tableCell.nextSibling : tableCell.previousSibling;
// return lastCell.nextSibling ? lastCell.nextSibling : lastCell.previousSibling;
return [ firstCell.previousSibling, lastCell.nextSibling ];
}
143 changes: 84 additions & 59 deletions src/commands/removerowcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,86 +28,111 @@ export default class RemoveRowCommand extends Command {
* @inheritDoc
*/
refresh() {
const model = this.editor.model;
const doc = model.document;
const firstCell = this._getReferenceCells().next().value;

const tableCell = findAncestor( 'tableCell', doc.selection.getFirstPosition() );

this.isEnabled = !!tableCell && tableCell.parent.parent.childCount > 1;
this.isEnabled = !!firstCell && firstCell.parent.parent.childCount > 1;
}

/**
* @inheritDoc
*/
execute() {
const model = this.editor.model;
const selection = model.document.selection;

const firstPosition = selection.getFirstPosition();
const tableCell = findAncestor( 'tableCell', firstPosition );
const tableRow = tableCell.parent;
const table = tableRow.parent;

const removedRow = table.getChildIndex( tableRow );

const tableMap = [ ...new TableWalker( table, { endRow: removedRow } ) ];
const referenceCells = Array.from( this._getReferenceCells() );
const removedRowIndexes = {
first: referenceCells[ 0 ].parent.index,
last: referenceCells[ referenceCells.length - 1 ].parent.index
};
const firstCell = referenceCells[ 0 ];
const table = firstCell.parent.parent;
const tableMap = [ ...new TableWalker( table, { endRow: removedRowIndexes.last } ) ];

this.editor.model.change( writer => {
// Temporary workaround to avoid the "model-selection-range-intersects" error.
writer.setSelection( writer.createSelection( table, 'on' ) );

const firstCellData = tableMap.find( value => value.cell === firstCell );
const columnToFocus = firstCellData.column;
let cellToFocus;

for ( let i = removedRowIndexes.last; i >= removedRowIndexes.first; i-- ) {
const removedRowIndex = i;
this._removeRow( removedRowIndex, table, writer, tableMap );

cellToFocus = getCellToFocus( table, removedRowIndex, columnToFocus );
}

const cellData = tableMap.find( value => value.cell === tableCell );
writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
} );
}

/**
* @private
*/
_removeRow( removedRowIndex, table, writer, tableMap ) {
const cellsToMove = new Map();
const tableRow = table.getChild( removedRowIndex );
const headingRows = table.getAttribute( 'headingRows' ) || 0;

const columnToFocus = cellData.column;
if ( headingRows && removedRowIndex <= headingRows ) {
updateNumericAttribute( 'headingRows', headingRows - 1, table, writer, 0 );
}

model.change( writer => {
if ( headingRows && removedRow <= headingRows ) {
updateNumericAttribute( 'headingRows', headingRows - 1, table, writer, 0 );
// Get cells from removed row that are spanned over multiple rows.
tableMap
.filter( ( { row, rowspan } ) => row === removedRowIndex && 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 <= removedRowIndex - 1 && row + rowspan > removedRowIndex )
.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );

// Move cells to another row.
const targetRow = removedRowIndex + 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;
}
}

const cellsToMove = new Map();

// Get cells from removed row that are spanned over multiple rows.
tableMap
.filter( ( { row, rowspan } ) => row === removedRow && 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 <= removedRow - 1 && row + rowspan > removedRow )
.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );

// Move cells to another row.
const targetRow = removedRow + 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 );
writer.remove( tableRow );
}

previousCell = cellToMove;
} else {
previousCell = cell;
}
/**
* Returns cells that are selected and are a reference to removing rows.
*
* @private
* @returns {Iterable.<module:engine/model/element~Element>} Generates `tableCell` elements.
*/
* _getReferenceCells() {
const plugins = this.editor.plugins;
if ( plugins.has( 'TableSelection' ) && plugins.get( 'TableSelection' ).hasMultiCellSelection ) {
for ( const cell of plugins.get( 'TableSelection' ).getSelectedTableCells() ) {
yield cell;
}
} else {
const selection = this.editor.model.document.selection;

writer.remove( tableRow );

const cellToFocus = getCellToFocus( table, removedRow, columnToFocus );
writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
} );
yield findAncestor( 'tableCell', selection.getFirstPosition() );
}
}
}

// Returns a cell that should be focused before removing the row, belonging to the same column as the currently focused cell.
function getCellToFocus( table, removedRow, columnToFocus ) {
const row = table.getChild( removedRow );
function getCellToFocus( table, removedRowIndex, columnToFocus ) {
const row = table.getChild( removedRowIndex );

// Default to first table cell.
let cellToFocus = row.getChild( 0 );
Expand Down
Loading