Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support col[span] and pts in paste from excel #14720

Merged
merged 16 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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' ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not triggering column width normalization for such case:

<table>
    <colgroup>
        <col style="width: 30pt;">
        <col style="width: 70pt;">
    </colgroup>
    <tbody>
        <tr>
            <td>a</td>
            <td>b</td>
        </tr>
    </tbody>
</table>

And the final table ends up like this:

<figure class="table">
    <table class="ck-table-resized">
        <colgroup>
            <col style="width:30%;">
            <col style="width:70%;">
        </colgroup>
        <tbody>
        <tr>
            <td>a</td>
            <td>b</td>
        </tr>
        </tbody>
    </table>
</figure>

so pt values are treated as they would be %, the table gets full page width and the original sizes are not preserved.

The column width is normalized in post-fixer (but should be normalized before post-fixers are triggered).

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.
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
// 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 [];
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
}

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