Skip to content

Commit

Permalink
Merge pull request #14720 from ckeditor/ck/14521-support-cell-widths-…
Browse files Browse the repository at this point in the history
…in-paste-from-excel

Fix (table,paste-from-office): Tables pasted from MS Excel will have proper column widths. Closes #14521. Closes #14516.
  • Loading branch information
mlewand authored Aug 14, 2023
2 parents 781afe3 + 07dfbcb commit cc319ca
Show file tree
Hide file tree
Showing 7 changed files with 387 additions and 12 deletions.
5 changes: 3 additions & 2 deletions packages/ckeditor5-table/src/tablecolumnresize/converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
updateColumnElements,
getColumnGroupElement,
getTableColumnElements,
getTableColumnsWidths
translateColSpanAttribute
} from './utils';

/**
Expand All @@ -38,9 +38,10 @@ export function upcastColgroupElement( tableUtilsPlugin: TableUtils ): ( dispatc
}

const columnElements = getTableColumnElements( tableColumnGroup );
let columnWidths = getTableColumnsWidths( tableColumnGroup );
const columnsCount = tableUtilsPlugin.getColumns( modelTable );
let columnWidths = translateColSpanAttribute( tableColumnGroup, conversionApi.writer );

// Fill the array with 'auto' values if the number of columns is higher than number of declared values.
columnWidths = Array.from( { length: columnsCount }, ( _, index ) => columnWidths[ index ] || 'auto' );

if ( columnWidths.length != columnElements.length || columnWidths.includes( 'auto' ) ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ export default class TableColumnResizeEditing extends Plugin {

this.editor.model.schema.register( 'tableColumn', {
allowIn: 'tableColumnGroup',
allowAttributes: [ 'columnWidth' ],
allowAttributes: [ 'columnWidth', 'colSpan' ],
isLimit: true
} );
}
Expand Down Expand Up @@ -418,14 +418,28 @@ export default class TableColumnResizeEditing extends Plugin {
value: ( viewElement: ViewElement ) => {
const viewColWidth = viewElement.getStyle( 'width' );

if ( !viewColWidth || !viewColWidth.endsWith( '%' ) ) {
// 'pt' is the default unit for table column width pasted from MS Office.
// See https://github.com/ckeditor/ckeditor5/issues/14521#issuecomment-1662102889 for more details.
if ( !viewColWidth || ( !viewColWidth.endsWith( '%' ) && !viewColWidth.endsWith( 'pt' ) ) ) {
return 'auto';
}

return viewColWidth;
}
}
} );

// The `col[span]` attribute is present in tables pasted from MS Excel. We use it to set the temporary `colSpan` model attribute,
// which is consumed during the `colgroup` element upcast.
// See https://github.com/ckeditor/ckeditor5/issues/14521#issuecomment-1662102889 for more details.
conversion.for( 'upcast' ).attributeToAttribute( {
view: {
name: 'col',
key: 'span'
},
model: 'colSpan'
} );

conversion.for( 'downcast' ).attributeToAttribute( {
model: {
name: 'tableColumn',
Expand Down
46 changes: 41 additions & 5 deletions packages/ckeditor5-table/src/tablecolumnresize/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,11 @@ export function sumArray( array: Array<number | string> ): number {
* changed proportionally so that they all sum back to 100%. If there are columns without specified width, the amount remaining
* after assigning the known widths will be distributed equally between them.
*
* Currently, only widths provided as percentage values are supported.
*
* @param columnWidths An array of column widths.
* @returns An array of column widths guaranteed to sum up to 100%.
*/
export function normalizeColumnWidths( columnWidths: Array<string> ): Array<string> {
const widths: Array<number | 'auto'> = columnWidths.map( width => {
// Possible values are 'auto' or string ending with '%'
if ( width === 'auto' ) {
return width;
}
Expand Down Expand Up @@ -376,14 +373,20 @@ export function getColumnGroupElement( element: Element ): Element {
}

/**
* Returns an array of 'tableColumn' elements.
* Returns an array of 'tableColumn' elements. It may be empty if there's no `tableColumnGroup` element.
*
* @internal
* @param element A 'table' or 'tableColumnGroup' element.
* @returns An array of 'tableColumn' elements.
*/
export function getTableColumnElements( element: Element ): Array<Element> {
return Array.from( getColumnGroupElement( element ).getChildren() as IterableIterator<Element> );
const columnGroupElement = getColumnGroupElement( element );

if ( !columnGroupElement ) {
return [];
}

return Array.from( columnGroupElement.getChildren() as IterableIterator<Element> );
}

/**
Expand All @@ -396,3 +399,36 @@ export function getTableColumnElements( element: Element ): Array<Element> {
export function getTableColumnsWidths( element: Element ): Array<string> {
return getTableColumnElements( element ).map( column => column.getAttribute( 'columnWidth' ) as string );
}

/**
* Translates the `colSpan` model attribute into additional column widths and returns the resulting array.
*
* @internal
* @param element A 'table' or 'tableColumnGroup' element.
* @param writer A writer instance.
* @returns An array of table column widths.
*/
export function translateColSpanAttribute( element: Element, writer: Writer ): Array<string> {
const tableColumnElements = getTableColumnElements( element );

return tableColumnElements.reduce( ( acc: Array<string>, element ) => {
const columnWidth = element.getAttribute( 'columnWidth' ) as string;
const colSpan = element.getAttribute( 'colSpan' ) as number | undefined;

if ( !colSpan ) {
acc.push( columnWidth );
return acc;
}

// Translate the `colSpan` model attribute on to the proper number of column widths
// and remove it from the element.
// See https://github.com/ckeditor/ckeditor5/issues/14521#issuecomment-1662102889 for more details.
for ( let i = 0; i < colSpan; i++ ) {
acc.push( columnWidth );
}

writer.removeAttribute( 'colSpan', element );

return acc;
}, [] );
}
18 changes: 18 additions & 0 deletions packages/ckeditor5-table/src/tableutils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
import TableWalker, { type TableWalkerOptions } from './tablewalker';
import { createEmptyTableCell, updateNumericAttribute } from './utils/common';
import { removeEmptyColumns, removeEmptyRows } from './utils/structure';
import { getTableColumnElements } from './tablecolumnresize/utils';

type Cell = { cell: Element; rowspan: number };
type CellsToMove = Map<number, Cell>;
Expand Down Expand Up @@ -471,6 +472,7 @@ export default class TableUtils extends Plugin {

model.change( writer => {
adjustHeadingColumns( table, { first, last }, writer );
const tableColumns = getTableColumnElements( table );

for ( let removedColumnIndex = last; removedColumnIndex >= first; removedColumnIndex-- ) {
for ( const { cell, column, cellWidth } of [ ...new TableWalker( table ) ] ) {
Expand All @@ -482,6 +484,22 @@ export default class TableUtils extends Plugin {
writer.remove( cell );
}
}

// If table has `tableColumn` elements, we need to update it manually.
// See https://github.com/ckeditor/ckeditor5/issues/14521#issuecomment-1662102889 for details.
if ( tableColumns[ removedColumnIndex ] ) {
// If the removed column is the first one then we need to add its width to the next column.
// Otherwise we add it to the previous column.
const adjacentColumn = removedColumnIndex === 0 ? tableColumns[ 1 ] : tableColumns[ removedColumnIndex - 1 ];

const removedColumnWidth = parseFloat( tableColumns[ removedColumnIndex ].getAttribute( 'columnWidth' ) as string );
const adjacentColumnWidth = parseFloat( adjacentColumn.getAttribute( 'columnWidth' ) as string );

writer.remove( tableColumns[ removedColumnIndex ] );

// Add the removed column width (in %) to the adjacent column.
writer.setAttribute( 'columnWidth', removedColumnWidth + adjacentColumnWidth + '%', adjacentColumn );
}
}

// Remove empty rows that could appear after removing columns.
Expand Down
70 changes: 70 additions & 0 deletions packages/ckeditor5-table/tests/tableclipboard-paste.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import TableCellWidthEditing from '../src/tablecellwidth/tablecellwidthediting';
import TableWalker from '../src/tablewalker';

import TableClipboard from '../src/tableclipboard';
import TableColumnResize from '../src/tablecolumnresize';

describe( 'table clipboard', () => {
let editor, model, modelRoot, tableSelection, viewDocument, element;
Expand Down Expand Up @@ -183,6 +184,75 @@ describe( 'table clipboard', () => {
);
} );

it( 'should adjust `<colgroup>` element while normalizing pasted table', async () => {
const editorWithColumnResize = await ClassicTestEditor.create( element, {
plugins: [ TableEditing, TableClipboard, Paragraph, Clipboard, TableColumnResize ]
} );

model = editorWithColumnResize.model;
modelRoot = model.document.getRoot();
viewDocument = editorWithColumnResize.editing.view.document;
tableSelection = editorWithColumnResize.plugins.get( 'TableSelection' );

model.change( writer => {
writer.insertElement( 'paragraph', modelRoot.getChild( 0 ), 'before' );
writer.setSelection( modelRoot.getChild( 0 ), 'before' );
} );

const table =
'<table>' +
'<colgroup>' +
'<col span="2" style="width:20%">' +
'</col>' +
'<col style="width:60%">' +
'</col>' +
'</colgroup>' +
'<tbody>' +
'<tr>' +
'<td colspan="3">foo</td>' +
'</tr>' +
'<tr>' +
'<td colspan="2">bar</td>' +
'<td>baz</td>' +
'</tr>' +
'</tbody>' +
'</table>';

const data = {
dataTransfer: createDataTransfer(),
preventDefault: sinon.spy(),
stopPropagation: sinon.spy()
};

data.dataTransfer.setData( 'text/html', table );
viewDocument.fire( 'paste', data );

expect( getModelData( model ) ).to.equalMarkup(
'[<table>' +
'<tableRow>' +
'<tableCell colspan="2">' +
'<paragraph>foo</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph>bar</paragraph>' +
'</tableCell>' +
'<tableCell>' +
'<paragraph>baz</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'<tableColumnGroup>' +
'<tableColumn columnWidth="40%"></tableColumn>' +
'<tableColumn columnWidth="60%"></tableColumn>' +
'</tableColumnGroup>' +
'</table>]' +
'<paragraph></paragraph>'
);

await editorWithColumnResize.destroy();
} );

it( 'should not alter model.insertContent if no table pasted', () => {
tableSelection.setCellSelection(
modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
Expand Down
Loading

0 comments on commit cc319ca

Please sign in to comment.