Skip to content

Commit

Permalink
Faster algorithm.
Browse files Browse the repository at this point in the history
  • Loading branch information
Mati365 committed Dec 30, 2024
1 parent 6d58de0 commit 34b199d
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 109 deletions.
93 changes: 9 additions & 84 deletions packages/ckeditor5-table/src/tableselection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,18 +360,24 @@ 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 table = anchorCell.findAncestor( 'table' )!;
const endColumn = Math.max( startLocation.column, endLocation.column + targetColumnExtraColspan );

// First collect cells based on initial selection
let selectionMap: Array<Array<Element>> = createSelectionMap( endRow - startRow + 1 );
const selectionMap: Array<Array<Element>> = new Array( endRow - startRow + 1 ).fill( null ).map( () => [] );

const walkerOptions = {
startRow,
Expand All @@ -384,48 +390,6 @@ export default class TableSelection extends Plugin {
selectionMap[ row - startRow ].push( cell );
}

// If the entire last row is selected, extend the selection to include all columns in the rows above for better UX.
// This prevents a scenario where selecting the entire last row (which may contain colspans) results in only one column
// being selected in the row above, instead of all columns (when colspan is equal to the total amount of columns in the table).
// This adjustment is only active for top-left to bottom-right selections, as it mimics the behavior of colspan in tables.
//
// Example:
// +---+---+---+---+
// | A | B | C | D |
// +---+---+---+---+
// | E |
// +---+---+---+---+
//
// The selection begins at `A` and ends at `E`. The entire last row is selected, without this condition the selection would
// include only `A` and `E` cells, but with this condition, the selection will include `A`, `B`, `C`, `D` and `E` cells.
//
// See: https://github.com/ckeditor/ckeditor5/issues/17538
if ( !startColumn && startLocation.row <= endLocation.row ) {
// Pick total width of the last selection row. It includes colspan values and not-fully selected cells.
const totalRowWidth = getTotalColumnsInRow( table, endRow );

// Pick width of all selected cells in the last selection row.
const selectedCellsWidth = getTotalCellsWidth( selectionMap[ selectionMap.length - 1 ] );

// If last row is fully selected, adjust selection to include all columns in all rows above
if ( selectedCellsWidth === totalRowWidth ) {
const totalTableColumns = tableUtils.getColumns( table );
const fullSelectorWalker = new TableWalker( table, {
startRow,
endRow,
startColumn: 0,
endColumn: totalTableColumns
} );

// Let's reset the selection map and fill it with the full row selection
selectionMap = createSelectionMap( endRow - startRow + 1 );

for ( const { row, cell } of fullSelectorWalker ) {
selectionMap[ row - startRow ].push( cell );
}
}
}

const flipVertically = endLocation.row < startLocation.row;
const flipHorizontally = endLocation.column < startLocation.column;

Expand All @@ -443,42 +407,3 @@ export default class TableSelection extends Plugin {
};
}
}

/**
* Creates a 2D array of selections based on the given size.
*
* @param size The size of the map
* @returns An array of arrays of elements
*/
function createSelectionMap( size: number ): Array<Array<Element>> {
return new Array( size ).fill( null ).map( () => [] );
}

/**
* Calculates the total width of columns in a table row by getting the sum of colspan values
* of all cells in that row.
*
* @param table The table element to calculate the row width for
* @param rowIndex The index of the row to process
* @returns The total width of the row (sum of colspan values)
*/
function getTotalColumnsInRow( table: Element, rowIndex: number ): number {
const cells = Array
.from( new TableWalker( table, { row: rowIndex } ) )
.map( ( { cell } ) => cell );

return getTotalCellsWidth( cells );
}

/**
* Calculates the total width of the given cells by summing up their colspan values.
*
* @param cells An array of table cell elements to process
* @returns The total width (sum of colspan values) of all provided cells
*/
function getTotalCellsWidth( cells: Array<Element> ): number {
return cells.reduce(
( width, cell ) => width + ( parseInt( cell.getAttribute( 'colspan' ) as string ) || 1 ),
0
);
}
5 changes: 3 additions & 2 deletions packages/ckeditor5-table/tests/commands/mergecellscommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -745,9 +745,10 @@ describe( 'MergeCellsCommand', () => {

expect( getData( model ) ).to.equalMarkup( modelTable( [
[
'<paragraph>[00</paragraph><paragraph>01</paragraph><paragraph>02</paragraph>' +
'<paragraph>[00</paragraph><paragraph>01</paragraph>' +
'<paragraph>10</paragraph><paragraph>11</paragraph>' +
'<paragraph>20</paragraph><paragraph>21]</paragraph>'
'<paragraph>20</paragraph><paragraph>21]</paragraph>',
'02'
]
] ) );
} );
Expand Down
46 changes: 23 additions & 23 deletions packages/ckeditor5-table/tests/tableclipboard-paste.js
Original file line number Diff line number Diff line change
Expand Up @@ -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( [
[
Expand All @@ -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' ]
] ) );
} );
Expand Down Expand Up @@ -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' ]
] ) );
} );
Expand Down

0 comments on commit 34b199d

Please sign in to comment.