Skip to content

Commit

Permalink
Merge pull request #16733 from ckeditor/ck/14658
Browse files Browse the repository at this point in the history
Fix (table): Horizontal split button no longer splits larger table cell spans into more than two parts. Closes #14658.
  • Loading branch information
niegowski authored Jul 18, 2024
2 parents 248e072 + 5f58ffa commit 3c23755
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 4 deletions.
24 changes: 20 additions & 4 deletions packages/ckeditor5-table/src/tableutils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,12 @@ export default class TableUtils extends Plugin {
newCellsAttributes.colspan = colspan;
}

// Accumulator that stores distance from the last inserted cell span.
// It helps with evenly splitting larger cell spans (for example 10 cells collapsing into 3 cells).
// We split these cells into 3, 3, 4 cells and we have to call `createCells` only when distance between
// these cells is equal or greater than the new cells span size.
let distanceFromLastCellSpan = 0;

for ( const tableSlot of tableMap ) {
const { column, row } = tableSlot;

Expand All @@ -733,13 +739,23 @@ export default class TableUtils extends Plugin {
//
// 1. It's a row after split cell + it's height.
const isAfterSplitCell = row >= splitCellRow + updatedSpan;

// 2. Is on the same column.
const isOnSameColumn = column === cellColumn;
// 3. And it's row index is after previous cell height.
const isInEvenlySplitRow = ( row + splitCellRow + updatedSpan ) % newCellsSpan === 0;

if ( isAfterSplitCell && isOnSameColumn && isInEvenlySplitRow ) {
createCells( 1, writer, tableSlot.getPositionBefore(), newCellsAttributes );
// Reset distance from the last cell span if we are on the same column and we exceeded the new cells span size.
if ( distanceFromLastCellSpan >= newCellsSpan && isOnSameColumn ) {
distanceFromLastCellSpan = 0;
}

if ( isAfterSplitCell && isOnSameColumn ) {
// Create new cells only if the distance from the last cell span is equal or greater than the new cells span.
if ( !distanceFromLastCellSpan ) {
createCells( 1, writer, tableSlot.getPositionBefore(), newCellsAttributes );
}

// Increase the distance from the last cell span.
distanceFromLastCellSpan++;
}
}
}
Expand Down
62 changes: 62 additions & 0 deletions packages/ckeditor5-table/tests/tableutils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,68 @@ describe( 'TableUtils', () => {
] ) );
} );

it( 'should properly split large table in two parts with odd amount of rows', () => {
setData( model, modelTable( [
[ '00', { rowspan: 7, contents: '01[]' } ],
[ '10' ],
[ '20' ],
[ '30' ],
[ '40' ],
[ '50' ],
[ '60' ]
] ) );

const tableCell = root.getNodeByPath( [ 0, 0, 1 ] );

tableUtils.splitCellHorizontally( tableCell, 2 );

expect( getData( model ) ).to.equalMarkup( modelTable( [
[ '00', { rowspan: 4, contents: '01[]' } ],
[ '10' ],
[ '20' ],
[ '30' ],
[ '40', { rowspan: 3, contents: '' } ],
[ '50' ],
[ '60' ]
] ) );
} );

it( 'should not insert or modify rest of cells when splitting larger table rowspan with 7 cells ', () => {
setData( model, modelTable( [
[ { rowspan: 2, contents: '00' }, { colspan: 2, contents: '01' }, { colspan: 2, contents: '02' } ],
[ '10', '11', '12', '13' ],
[ { rowspan: 9, contents: '20[]' }, '21', '22', '23', '24' ],
[ '30', '31', '32', '33' ],
[ '40', '41', '42', '43' ],
[ '50', '51', '52', '53' ],
[ '60', '61', '62', '63' ],
[ '70', '71', '72', '73' ],
[ '80', '81', '82', '83' ],
[ '90', '91', '92', '93' ],
[ 'A0', 'A1', 'A2', 'A3' ],
[ { colspan: 5, contents: 'B0' } ]
] ) );

const tableCell = root.getNodeByPath( [ 0, 2, 0 ] );

tableUtils.splitCellHorizontally( tableCell, 2 );

expect( getData( model ) ).to.equalMarkup( modelTable( [
[ { rowspan: 2, contents: '00' }, { colspan: 2, contents: '01' }, { colspan: 2, contents: '02' } ],
[ '10', '11', '12', '13' ],
[ { rowspan: 5, contents: '20[]' }, '21', '22', '23', '24' ],
[ '30', '31', '32', '33' ],
[ '40', '41', '42', '43' ],
[ '50', '51', '52', '53' ],
[ '60', '61', '62', '63' ],
[ { rowspan: 4, contents: '' }, '70', '71', '72', '73' ],
[ '80', '81', '82', '83' ],
[ '90', '91', '92', '93' ],
[ 'A0', 'A1', 'A2', 'A3' ],
[ { colspan: 5, contents: 'B0' } ]
] ) );
} );

it( 'should evenly distribute rowspan attribute', () => {
setData( model, modelTable( [
[ '00', { rowspan: 7, contents: '01[]' } ],
Expand Down

0 comments on commit 3c23755

Please sign in to comment.