From 0ed0d5c5c1c2c6188b333eb37bdb0944d9185124 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 18 Jan 2023 11:58:29 +0100 Subject: [PATCH 01/25] Introduce `` and `` elements to store columns width information --- .../src/schemadefinitions.js | 8 + .../src/converters/downcast.js | 38 +++-- .../tablecolumnresizeediting.js | 152 +++++++++++++----- .../tablecolumnwidthscommand.js | 35 +++- .../tablewidthresizecommand.js | 17 +- .../src/tablecolumnresize/utils.js | 10 +- packages/ckeditor5-table/src/tableediting.js | 80 ++++++++- 7 files changed, 265 insertions(+), 75 deletions(-) diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.js b/packages/ckeditor5-html-support/src/schemadefinitions.js index 32dfc72d4a4..3a439eeea76 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.js +++ b/packages/ckeditor5-html-support/src/schemadefinitions.js @@ -90,6 +90,14 @@ export default { model: 'tableCell', view: 'th' }, + { + model: 'tableColumnGroup', + view: 'colgroup' + }, + { + model: 'tableColumn', + view: 'col' + }, { model: 'caption', view: 'caption' diff --git a/packages/ckeditor5-table/src/converters/downcast.js b/packages/ckeditor5-table/src/converters/downcast.js index 86e17e9661b..3f7b6b54cb3 100644 --- a/packages/ckeditor5-table/src/converters/downcast.js +++ b/packages/ckeditor5-table/src/converters/downcast.js @@ -21,12 +21,16 @@ import { toWidget, toWidgetEditable } from 'ckeditor5/src/widget'; export function downcastTable( tableUtils, options = {} ) { return ( table, { writer } ) => { const headingRows = table.getAttribute( 'headingRows' ) || 0; - const tableSections = []; + const additional = options.additional || []; + const tableElement = writer.createContainerElement( 'table', null, [] ); // Table head slot. if ( headingRows > 0 ) { - tableSections.push( - writer.createContainerElement( 'thead', null, + writer.insert( + writer.createPositionAt( tableElement, 'end' ), + writer.createContainerElement( + 'thead', + null, writer.createSlot( element => element.is( 'element', 'tableRow' ) && element.index < headingRows ) ) ); @@ -34,19 +38,35 @@ export function downcastTable( tableUtils, options = {} ) { // Table body slot. if ( headingRows < tableUtils.getRows( table ) ) { - tableSections.push( - writer.createContainerElement( 'tbody', null, + writer.insert( + writer.createPositionAt( tableElement, 'end' ), + writer.createContainerElement( + 'tbody', + null, writer.createSlot( element => element.is( 'element', 'tableRow' ) && element.index >= headingRows ) ) ); } + // Dynamic slots. + for ( const { positionOffset, filter } of additional ) { + writer.insert( + writer.createPositionAt( tableElement, positionOffset ), + writer.createSlot( filter ) + ); + } + + // Create a figure element with a table and a slot with items that don't fit into the table. const figureElement = writer.createContainerElement( 'figure', { class: 'table' }, [ - // Table with proper sections (thead, tbody). - writer.createContainerElement( 'table', null, tableSections ), + tableElement, + + writer.createSlot( element => { + if ( element.is( 'element', 'tableRow' ) ) { + return false; + } - // Slot for the rest (for example caption). - writer.createSlot( element => !element.is( 'element', 'tableRow' ) ) + return additional.some( ( { filter } ) => !filter( element ) ); + } ) ] ); return options.asWidget ? toTableWidget( figureElement, writer ) : figureElement; diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 28be1d0fdc6..b0672b8ef4c 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -7,7 +7,7 @@ * @module table/tablecolumnresize/tablecolumnresizeediting */ -import { throttle } from 'lodash-es'; +import { throttle, isEqual } from 'lodash-es'; import { global, DomEmitterMixin } from 'ckeditor5/src/utils'; import { Plugin } from 'ckeditor5/src/core'; @@ -19,11 +19,6 @@ import TableWalker from '../tablewalker'; import TableWidthResizeCommand from './tablewidthresizecommand'; import TableColumnWidthsCommand from './tablecolumnwidthscommand'; -import { - upcastColgroupElement, - downcastTableColumnWidthsAttribute -} from './converters'; - import { clamp, createFilledArray, @@ -131,6 +126,11 @@ export default class TableColumnResizeEditing extends Plugin { const editor = this.editor; const columnResizePlugin = editor.plugins.get( 'TableColumnResize' ); + editor.plugins.get( 'TableEditing' ).registerAdditionalSlot( { + filter: element => element.is( 'element', 'tableColumnGroup' ), + positionOffset: 'end' + } ); + editor.commands.add( 'resizeTableWidth', new TableWidthResizeCommand( editor ) ); editor.commands.add( 'resizeColumnWidths', new TableColumnWidthsCommand( editor ) ); @@ -166,7 +166,18 @@ export default class TableColumnResizeEditing extends Plugin { */ _extendSchema() { this.editor.model.schema.extend( 'table', { - allowAttributes: [ 'tableWidth', 'columnWidths' ] + allowAttributes: [ 'tableWidth' ] + } ); + + this.editor.model.schema.register( 'tableColumnGroup', { + allowIn: 'table', + isLimit: true + } ); + + this.editor.model.schema.register( 'tableColumn', { + allowIn: 'tableColumnGroup', + allowAttributes: [ 'columnWidth' ], + isLimit: true } ); } @@ -187,19 +198,35 @@ export default class TableColumnResizeEditing extends Plugin { let changed = false; for ( const table of getChangedResizedTables( model ) ) { - // (1) Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. - const columnWidths = normalizeColumnWidths( table.getAttribute( 'columnWidths' ).split( ',' ) ); + const tableColumnGroup = Array + .from( table.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + + const columns = Array.from( tableColumnGroup.getChildren() ); + const columnWidths = columns.map( element => element.getAttribute( 'columnWidth' ) ); - // (2) If the number of columns has changed, then we need to adjust the widths of the affected columns. - adjustColumnWidths( columnWidths, table, this ); + // Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. + const normalizedWidths = normalizeColumnWidths( columnWidths ); - const columnWidthsAttribute = columnWidths.map( width => `${ width }%` ).join( ',' ); + // If the number of columns has changed, then we need to adjust the widths of the affected columns. + adjustColumnWidths( normalizedWidths, table, this ); - if ( table.getAttribute( 'columnWidths' ) === columnWidthsAttribute ) { + if ( isEqual( columnWidths, normalizedWidths ) ) { continue; } - writer.setAttribute( 'columnWidths', columnWidthsAttribute, table ); + for ( let i = 0; i < Math.max( normalizedWidths.length, columns.length ); i++ ) { + const column = columns[ i ]; + const columnWidth = normalizedWidths[ i ]; + + if ( !columnWidth ) { + writer.remove( column ); + } else if ( !column ) { + writer.appendElement( 'tableColumn', { columnWidth }, tableColumnGroup ); + } else { + writer.setAttribute( 'columnWidth', columnWidth, column ); + } + } changed = true; } @@ -290,7 +317,9 @@ export default class TableColumnResizeEditing extends Plugin { _registerConverters() { const editor = this.editor; const conversion = editor.conversion; - const widthStyleToTableWidthDefinition = { + + // Table width style + conversion.for( 'upcast' ).attributeToAttribute( { view: { name: 'figure', key: 'style', @@ -303,8 +332,9 @@ export default class TableColumnResizeEditing extends Plugin { key: 'tableWidth', value: viewElement => viewElement.getStyle( 'width' ) } - }; - const tableWidthToWidthStyleDefinition = { + } ); + + conversion.for( 'downcast' ).attributeToAttribute( { model: { name: 'table', key: 'tableWidth' @@ -316,13 +346,37 @@ export default class TableColumnResizeEditing extends Plugin { width } } ) - }; + } ); - conversion.for( 'upcast' ).attributeToAttribute( widthStyleToTableWidthDefinition ); - conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); + // Table conversion + conversion.for( 'upcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); + conversion.for( 'downcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); - conversion.for( 'downcast' ).attributeToAttribute( tableWidthToWidthStyleDefinition ); - conversion.for( 'downcast' ).add( downcastTableColumnWidthsAttribute() ); + // Table conversion + conversion.for( 'upcast' ).elementToElement( { model: 'tableColumn', view: 'col' } ); + conversion.for( 'downcast' ).elementToElement( { model: 'tableColumn', view: 'col' } ); + + // Table column width conversion + conversion.for( 'upcast' ).attributeToAttribute( { + view: { + name: 'col', + key: 'style', + value: { width: /[\s\S]+/ } + }, + model: { + name: 'tableColumn', + key: 'columnWidth', + value: element => Number( element.getStyle( 'width' ).replace( '%', '' ) ) + } + } ); + + conversion.for( 'downcast' ).attributeToAttribute( { + model: { + name: 'tableColumn', + key: 'columnWidth' + }, + view: width => ( { key: 'style', value: { width: `${ width }%` } } ) + } ); } /** @@ -374,7 +428,7 @@ export default class TableColumnResizeEditing extends Plugin { const editingView = editor.editing.view; // Insert colgroup for the table that is resized for the first time. - if ( ![ ...viewTable.getChildren() ].find( viewCol => viewCol.is( 'element', 'colgroup' ) ) ) { + if ( !Array.from( viewTable.getChildren() ).find( viewCol => viewCol.is( 'element', 'colgroup' ) ) ) { editingView.change( viewWriter => { _insertColgroupElement( viewWriter, columnWidthsInPx, viewTable ); } ); @@ -550,12 +604,19 @@ export default class TableColumnResizeEditing extends Plugin { const editor = this.editor; const editingView = editor.editing.view; - const columnWidthsAttributeOld = modelTable.getAttribute( 'columnWidths' ); - const columnWidthsAttributeNew = [ ...viewColgroup.getChildren() ] - .map( viewCol => viewCol.getStyle( 'width' ) ) - .join( ',' ); + const tableColumnGroup = Array + .from( modelTable.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + + const columnWidthsAttributeOld = tableColumnGroup ? + Array.from( tableColumnGroup.getChildren() ).map( element => element.getAttribute( 'columnWidth' ) ) : + []; - const isColumnWidthsAttributeChanged = columnWidthsAttributeOld !== columnWidthsAttributeNew; + const columnWidthsAttributeNew = Array + .from( viewColgroup.getChildren() ) + .map( viewCol => viewCol.getStyle( 'width' ) ); + + const isColumnWidthsAttributeChanged = !isEqual( columnWidthsAttributeOld, columnWidthsAttributeNew ); const tableWidthAttributeOld = modelTable.getAttribute( 'tableWidth' ); const tableWidthAttributeNew = viewFigure.getStyle( 'width' ); @@ -566,16 +627,17 @@ export default class TableColumnResizeEditing extends Plugin { if ( this._isResizingAllowed ) { // Commit all changes to the model. if ( isTableWidthAttributeChanged ) { - editor.execute( - 'resizeTableWidth', - { - table: modelTable, - tableWidth: `${ toPrecision( tableWidthAttributeNew ) }%`, - columnWidths: columnWidthsAttributeNew - } - ); - } else { - editor.execute( 'resizeColumnWidths', { columnWidths: columnWidthsAttributeNew, table: modelTable } ); + editor.execute( 'resizeTableWidth', { + table: modelTable, + tableWidth: `${ toPrecision( tableWidthAttributeNew ) }%` + } ); + } + + if ( isColumnWidthsAttributeChanged ) { + editor.execute( 'resizeColumnWidths', { + columnWidths: columnWidthsAttributeNew, + table: modelTable + } ); } } else { // In read-only mode revert all changes in the editing view. The model is not touched so it does not need to be restored. @@ -584,10 +646,8 @@ export default class TableColumnResizeEditing extends Plugin { // If table had resized columns before, restore the previous column widths. // Otherwise clean up the view from the temporary column resizing markup. if ( columnWidthsAttributeOld ) { - const columnWidths = columnWidthsAttributeOld.split( ',' ); - for ( const viewCol of viewColgroup.getChildren() ) { - writer.setStyle( 'width', columnWidths.shift(), viewCol ); + writer.setStyle( 'width', columnWidthsAttributeOld.shift(), viewCol ); } } else { writer.remove( viewColgroup ); @@ -697,12 +757,18 @@ export default class TableColumnResizeEditing extends Plugin { const viewTable = editor.editing.view.document.selection.getFirstPosition().getAncestors().reverse().find( viewElement => viewElement.name === 'table' ); - const viewTableContainsColgroup = viewTable && [ ...viewTable.getChildren() ].find( + const viewTableContainsColgroup = viewTable && Array.from( viewTable.getChildren() ).find( viewElement => viewElement.is( 'element', 'colgroup' ) ); const modelTable = editor.model.document.selection.getFirstPosition().findAncestor( 'table' ); - if ( modelTable && modelTable.hasAttribute( 'columnWidths' ) && viewTable && !viewTableContainsColgroup ) { + if ( !modelTable ) { + return; + } + + const hasColumnGroup = Array.from( modelTable.getChildren() ).find( element => element.is( 'element', 'tableColumnGroup' ) ); + + if ( hasColumnGroup && viewTable && !viewTableContainsColgroup ) { editor.editing.reconvertItem( modelTable ); } }, { priority: 'low' } ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js index cccbaeef9b0..4cb0cae36fa 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js @@ -7,6 +7,7 @@ * @module table/tablecolumnresize/tablecolumnwidthscommand */ +import { normalizeColumnWidths } from './utils'; import TablePropertyCommand from '../tableproperties/commands/tablepropertycommand'; /** @@ -33,22 +34,40 @@ export default class TableColumnWidthsCommand extends TablePropertyCommand { } /** - * Changes the `columnWidths` attribute value for the given or currently selected table. + * Changes the `columnWidth` attribute values for the columns of the given table. * * @param {Object} options - * @param {String} [options.columnWidths] New value of the `columnWidths` attribute. + * @param {Array.} [options.columnWidths] New value of the `columnWidths` attribute. * @param {module:engine/model/element~Element} [options.table] The table that is having the columns resized. */ execute( options = {} ) { - const model = this.editor.model; - const table = options.table || model.document.selection.getSelectedElement(); - const { columnWidths } = options; + const { model } = this.editor; + const { table, columnWidths } = options; model.change( writer => { - if ( columnWidths ) { - writer.setAttribute( this.attributeName, columnWidths, table ); + const tableColumnGroup = Array + .from( table.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + + if ( !columnWidths && !tableColumnGroup ) { + return; + } + + if ( !columnWidths ) { + return writer.remove( tableColumnGroup ); + } + + const widths = normalizeColumnWidths( columnWidths ); + + if ( !tableColumnGroup ) { + const colGroupElement = writer.createElement( 'tableColumnGroup' ); + + widths.forEach( columnWidth => writer.appendElement( 'tableColumn', { columnWidth }, colGroupElement ) ); + writer.append( colGroupElement, table ); } else { - writer.removeAttribute( this.attributeName, table ); + Array + .from( tableColumnGroup.getChildren() ) + .forEach( ( column, index ) => writer.setAttribute( 'columnWidth', widths[ index ], column ) ); } } ); } diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js index 88b131ba571..606459ed90e 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js @@ -40,26 +40,21 @@ export default class TableWidthResizeCommand extends TablePropertyCommand { * * @param {Object} options * @param {String} [options.tableWidth] The new table width. If skipped, the model attribute will be removed. - * @param {String} [options.columnWidths] The new table column widths. If skipped, the model attribute will be removed. * @param {module:engine/model/element~Element} [options.table] The table that is affected by the resize. */ execute( options = {} ) { const model = this.editor.model; - const table = options.table || model.document.selection.getSelectedElement(); - const { tableWidth, columnWidths } = options; + const { + table = model.document.selection.getSelectedElement(), + tableWidth + } = options; model.change( writer => { if ( tableWidth ) { - writer.setAttribute( this.attributeName, tableWidth, table ); - } else { - writer.removeAttribute( this.attributeName, table ); + return writer.setAttribute( this.attributeName, tableWidth, table ); } - if ( columnWidths ) { - writer.setAttribute( 'columnWidths', columnWidths, table ); - } else { - writer.removeAttribute( 'columnWidths', table ); - } + writer.removeAttribute( this.attributeName, table ); } ); } } diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index 287786068b3..0711446dd54 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -67,9 +67,15 @@ export function getChangedResizedTables( model ) { // We iterate over the whole table looking for the nested tables that are also affected. for ( const node of model.createRangeOn( tableNode ).getItems() ) { - if ( node.is( 'element' ) && node.name === 'table' && node.hasAttribute( 'columnWidths' ) ) { - affectedTables.add( node ); + if ( !node.is( 'element', 'table' ) ) { + continue; } + + if ( !Array.from( node.getChildren() ).find( element => element.is( 'element', 'tableColumnGroup' ) ) ) { + continue; + } + + affectedTables.add( node ); } } diff --git a/packages/ckeditor5-table/src/tableediting.js b/packages/ckeditor5-table/src/tableediting.js index c2e3e5a6628..92fe70ed66f 100644 --- a/packages/ckeditor5-table/src/tableediting.js +++ b/packages/ckeditor5-table/src/tableediting.js @@ -54,6 +54,21 @@ export default class TableEditing extends Plugin { return [ TableUtils ]; } + /** + * @inheritDoc + */ + constructor( editor ) { + super( editor ); + + /** + * Handlers for creating additional slots in the table. + * + * @private + * @member {Array.} + */ + this._additionalSlots = []; + } + /** * @inheritDoc */ @@ -93,14 +108,19 @@ export default class TableEditing extends Plugin { name: 'table', attributes: [ 'headingRows' ] }, - view: downcastTable( tableUtils, { asWidget: true } ) + view: downcastTable( tableUtils, { + asWidget: true, + additional: this._additionalSlots + } ) } ); conversion.for( 'dataDowncast' ).elementToStructure( { model: { name: 'table', attributes: [ 'headingRows' ] }, - view: downcastTable( tableUtils ) + view: downcastTable( tableUtils, { + additional: this._additionalSlots + } ) } ); // Table row conversion. @@ -195,6 +215,15 @@ export default class TableEditing extends Plugin { tableCellRefreshHandler( model, editor.editing ); } ); } + + /** + * Registers downcast handler for the additional table slot. + * + * @param {module:table/tablediting~AdditionalSlot} slotHandler + */ + registerAdditionalSlot( slotHandler ) { + this._additionalSlots.push( slotHandler ); + } } // Creates a mapper callback to adjust model position mappings in a table cell containing a paragraph, which is bound to its parent @@ -247,3 +276,50 @@ function upcastCellSpan( type ) { return span; }; } + +/** + * By default, only the `tableRow` elements from the `table` model are downcast inside the `` and + * all other elements are pushed outside the table. This handler allows creating additional slots inside + * the table for other elements. + * + * Take this model as an example: + * + *
+ * ... + * ... + * ... + *
+ * + * By default, downcasting result will be: + * + * + * + * ... + * ... + * + *
+ * ... + * + * To allow the `tableColumnGroup` element at the end of the table, use the following configuration: + * + * const additionalSlot = { + * filter: element => element.is( 'element', 'tableColumnGroup' ), + * positionOffset: 'end' + * } + * + * Now, the downcast result will be: + * + * + * + * ... + * ... + * + * ... + *
+ * + * @typedef {Object} module:table/tablediting~AdditionalSlot + * + * @property {Function} filter Filter for elements that should be placed inside given slot. + * + * @property {number|'before'|'after'|'end'} positionOffset Position of the slot within the table. + */ From c3870512d7a0a098bdd9b191c4f6d9bef482b7c7 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 25 Jan 2023 15:33:06 +0100 Subject: [PATCH 02/25] Fix default slot in table --- packages/ckeditor5-table/src/converters/downcast.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-table/src/converters/downcast.js b/packages/ckeditor5-table/src/converters/downcast.js index 3f7b6b54cb3..616ce6663a2 100644 --- a/packages/ckeditor5-table/src/converters/downcast.js +++ b/packages/ckeditor5-table/src/converters/downcast.js @@ -65,7 +65,7 @@ export function downcastTable( tableUtils, options = {} ) { return false; } - return additional.some( ( { filter } ) => !filter( element ) ); + return !additional.some( ( { filter } ) => filter( element ) ); } ) ] ); From 1c82f3b7ac65173c7a57ed51dc83dfde75d0f539 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 25 Jan 2023 15:34:30 +0100 Subject: [PATCH 03/25] Fix failing table tests --- .../tablecolumnwidthscommand.js | 5 +- .../ckeditor5-table/tests/_utils/utils.js | 32 ++- .../tablecolumnresizeediting.js | 219 ++++++++++++++---- .../tablecolumnwidthscommand.js | 8 +- .../tablewidthresizecommand.js | 25 +- .../tests/tablecolumnresize/utils.js | 14 +- 6 files changed, 229 insertions(+), 74 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js index 4cb0cae36fa..9a7ae349ca4 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js @@ -42,7 +42,10 @@ export default class TableColumnWidthsCommand extends TablePropertyCommand { */ execute( options = {} ) { const { model } = this.editor; - const { table, columnWidths } = options; + const { + table = model.document.selection.getSelectedElement(), + columnWidths + } = options; model.change( writer => { const tableColumnGroup = Array diff --git a/packages/ckeditor5-table/tests/_utils/utils.js b/packages/ckeditor5-table/tests/_utils/utils.js index 8be7abc085e..f094b29c125 100644 --- a/packages/ckeditor5-table/tests/_utils/utils.js +++ b/packages/ckeditor5-table/tests/_utils/utils.js @@ -35,7 +35,9 @@ const WIDGET_TABLE_CELL_CLASS = 'ck-editor__editable ck-editor__nested-editable' * * @returns {String} */ -export function modelTable( tableData, attributes ) { +export function modelTable( tableData, attributes = {} ) { + const { columnWidths, ...attrs } = attributes; + const tableRows = makeRows( tableData, { cellElement: 'tableCell', rowElement: 'tableRow', @@ -44,7 +46,9 @@ export function modelTable( tableData, attributes ) { enforceWrapping: true } ); - return `${ tableRows }`; + const tableCols = makeColGroup( columnWidths ); + + return `${ tableRows }${ tableCols }`; } /** @@ -407,6 +411,20 @@ function makeRows( tableData, options ) { }, '' ); } +function makeColGroup( columnWidths ) { + if ( !columnWidths ) { + return ''; + } + + const cols = columnWidths + .split( ',' ) + .map( width => width.replace( '%', '' ) ) + .map( width => `` ) + .join( '' ); + + return `${ cols }`; +} + // Properly handles passed CSS class - editor do sort them. function getClassToSet( attributes ) { return ( WIDGET_TABLE_CELL_CLASS + ( attributes.class ? ` ${ attributes.class }` : '' ) ) @@ -556,3 +574,13 @@ function getElementPlainText( model, element ) { .map( ( { item: { data } } ) => data ) .join( '' ); } + +export function getTableColumnWidths( table ) { + return Array + .from( table.getChildren() ) + .filter( element => element.is( 'element', 'tableColumnGroup' ) ) + .map( element => Array.from( element.getChildren() ) ) + .flat() + .map( element => element.getAttribute( 'columnWidth' ) ) + .map( width => Number( width ) ); +} diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 504f00b7dfe..b215e9b82d0 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -22,7 +22,7 @@ import HighlightEditing from '@ckeditor/ckeditor5-highlight/src/highlightediting import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport'; import { focusEditor } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; -import { modelTable } from '../_utils/utils'; +import { getTableColumnWidths, modelTable } from '../_utils/utils'; import { getDomTable, getModelTable, @@ -81,9 +81,7 @@ describe( 'TableColumnResizeEditing', () => { [ '10', '11', '12' ] ], { columnWidths: '25%,25%,50%' } ) ); - const tableWidths = model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ); - - expect( tableWidths ).to.be.equal( '25%,25%,50%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ 25, 25, 50 ] ); } ); it( 'should have defined col widths in view', () => { @@ -124,7 +122,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -133,6 +131,10 @@ describe( 'TableColumnResizeEditing', () => { '12' + '' + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -159,7 +161,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -171,6 +173,11 @@ describe( 'TableColumnResizeEditing', () => { '13' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -195,7 +202,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -207,6 +214,11 @@ describe( 'TableColumnResizeEditing', () => { '13' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -233,7 +245,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -245,6 +257,11 @@ describe( 'TableColumnResizeEditing', () => { '13' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -268,7 +285,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -280,6 +297,11 @@ describe( 'TableColumnResizeEditing', () => { '13' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -303,7 +325,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -315,6 +337,11 @@ describe( 'TableColumnResizeEditing', () => { '13' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -379,7 +406,7 @@ describe( 'TableColumnResizeEditing', () => { ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -391,6 +418,11 @@ describe( 'TableColumnResizeEditing', () => { '13' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ); } ); @@ -1342,10 +1374,10 @@ describe( 'TableColumnResizeEditing', () => { const mouseMovementVector = { x: -10, y: 0 }; setModelData( editor.model, - '' + + '
' + '' + '' + - '[
' + + '[
' + '' + '' + 'foo' + @@ -1354,9 +1386,16 @@ describe( 'TableColumnResizeEditing', () => { 'bar' + '' + '' + + '' + + '' + + '' + + '' + '
]' + '' + '' + + '' + + '' + + '' + '' ); @@ -1391,10 +1430,10 @@ describe( 'TableColumnResizeEditing', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.match( new RegExp( - '' + + '
' + '' + '' + - '
' + + '
' + '' + '' + 'foo' + @@ -1403,9 +1442,16 @@ describe( 'TableColumnResizeEditing', () => { 'bar' + '' + '' + + '' + + '' + + '' + + '' + '
' + '' + '' + + '' + + '' + + '' + '' ) ); @@ -1417,10 +1463,10 @@ describe( 'TableColumnResizeEditing', () => { const mouseMovementVector = { x: 10, y: 0 }; setModelData( editor.model, - '' + + '
' + '' + '' + - '[
' + + '[
' + '' + '' + 'foo' + @@ -1429,9 +1475,16 @@ describe( 'TableColumnResizeEditing', () => { 'bar' + '' + '' + + '' + + '' + + '' + + '' + '
]' + '' + '' + + '' + + '' + + '' + '' ); @@ -1466,10 +1519,10 @@ describe( 'TableColumnResizeEditing', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.match( new RegExp( - '' + + '
' + '' + '' + - '
' + + '
' + '' + '' + 'foo' + @@ -1478,9 +1531,16 @@ describe( 'TableColumnResizeEditing', () => { 'bar' + '' + '' + + '' + + '' + + '' + + '' + '
' + '' + '' + + '' + + '' + + '' + '' ) ); @@ -1492,10 +1552,10 @@ describe( 'TableColumnResizeEditing', () => { const mouseMovementVector = { x: 10, y: 0 }; setModelData( editor.model, - '' + + '
' + '' + '' + - '[
' + + '[
' + '' + '' + 'foo' + @@ -1507,9 +1567,17 @@ describe( 'TableColumnResizeEditing', () => { 'baz' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
]' + '' + '' + + '' + + '' + + '' + '' ); @@ -1543,10 +1611,10 @@ describe( 'TableColumnResizeEditing', () => { assertViewPixelWidths( finalViewColumnWidthsPx, expectedViewColumnWidthsPx ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + - '
' + + '
' + '' + '' + 'foo' + @@ -1558,9 +1626,17 @@ describe( 'TableColumnResizeEditing', () => { 'baz' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' + '' + '' + + '' + + '' + + '' + '' ); } ); @@ -1969,9 +2045,9 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { if ( item.item.is( 'element', 'table' ) ) { // Expect `columnWidths` to have 4 values. - expect( item.item.getAttribute( 'columnWidths' ).split( ',' ).length ).to.equal( 4 ); + expect( getTableColumnWidths( item.item ).length ).to.equal( 4 ); // Expect a new column (it is the narrowest one) to be inserted at the first position. - expect( parseFloat( item.item.getAttribute( 'columnWidths' ).split( ',' )[ 0 ] ) < 10 ).to.be.true; + expect( parseFloat( getTableColumnWidths( item.item )[ 0 ] ) < 10 ).to.be.true; } } } ); @@ -1989,9 +2065,9 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { if ( item.item.is( 'element', 'table' ) ) { // Expect `columnWidths` to have 4 values. - expect( item.item.getAttribute( 'columnWidths' ).split( ',' ).length ).to.equal( 4 ); + expect( getTableColumnWidths( item.item ).length ).to.equal( 4 ); // Expect a new column (it is the narrowest one) to be inserted at the second position. - expect( parseFloat( item.item.getAttribute( 'columnWidths' ).split( ',' )[ 1 ] ) < 10 ).to.be.true; + expect( parseFloat( getTableColumnWidths( item.item )[ 1 ] ) < 10 ).to.be.true; } } } ); @@ -2009,9 +2085,9 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { if ( item.item.is( 'element', 'table' ) ) { // Expect `columnWidths` to have 4 values. - expect( item.item.getAttribute( 'columnWidths' ).split( ',' ).length ).to.equal( 4 ); + expect( getTableColumnWidths( item.item ).length ).to.equal( 4 ); // Expect a new column (it is the narrowest one) to be inserted at the last position. - expect( parseFloat( item.item.getAttribute( 'columnWidths' ).split( ',' )[ 3 ] ) < 10 ).to.be.true; + expect( parseFloat( getTableColumnWidths( item.item )[ 3 ] ) < 10 ).to.be.true; } } } ); @@ -2029,7 +2105,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the next column to take over the width of removed one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = item.item.getAttribute( 'columnWidths' ).split( ',' ); + const columnWidths = getTableColumnWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '45%' ); expect( columnWidths[ 1 ] ).to.equal( '55%' ); @@ -2050,7 +2126,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the previous column to take over the width of removed one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = item.item.getAttribute( 'columnWidths' ).split( ',' ); + const columnWidths = getTableColumnWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '45%' ); expect( columnWidths[ 1 ] ).to.equal( '55%' ); @@ -2071,7 +2147,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the previous column to take over the width of removed one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = item.item.getAttribute( 'columnWidths' ).split( ',' ); + const columnWidths = getTableColumnWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '20%' ); expect( columnWidths[ 1 ] ).to.equal( '80%' ); @@ -2099,7 +2175,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the first column to take over the width of merged one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = item.item.getAttribute( 'columnWidths' ).split( ',' ); + const columnWidths = getTableColumnWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '45%' ); } @@ -2128,7 +2204,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the first column to take over the width of merged one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = item.item.getAttribute( 'columnWidths' ).split( ',' ); + const columnWidths = getTableColumnWidths( item.item ); expect( columnWidths.length ).to.equal( 1 ); expect( columnWidths[ 0 ] ).to.equal( '100%' ); } @@ -2155,7 +2231,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 3 unchanged values. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = item.item.getAttribute( 'columnWidths' ).split( ',' ); + const columnWidths = getTableColumnWidths( item.item ); expect( columnWidths.length ).to.equal( 3 ); expect( columnWidths[ 0 ] ).to.equal( '20%' ); expect( columnWidths[ 1 ] ).to.equal( '25%' ); @@ -2193,7 +2269,7 @@ describe( 'TableColumnResizeEditing', () => { ], { columnWidths: '20%,25%,55%' } ) ); expect( getModelData( model ) ).to.equal( - '[' + + '[
' + '' + '' + '00' + @@ -2205,6 +2281,11 @@ describe( 'TableColumnResizeEditing', () => { '02' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
]' ); } ); @@ -2237,7 +2318,7 @@ describe( 'TableColumnResizeEditing', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.match( new RegExp( - '' + + '
' + '' + '' + '00' + @@ -2249,6 +2330,11 @@ describe( 'TableColumnResizeEditing', () => { '02' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ) ); @@ -2267,7 +2353,7 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.resize( editor, getDomTable( view ), columnToResizeIndex, mouseMovementVector ); expect( getModelData( editor.model ) ).to.equal( - '[' + + '[
' + '' + '' + '00' + @@ -2279,6 +2365,11 @@ describe( 'TableColumnResizeEditing', () => { '02' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
]' ); } ); @@ -2299,7 +2390,7 @@ describe( 'TableColumnResizeEditing', () => { // while the rest (column widths) is covered elsewhere not very important. expect( getModelData( model, { withoutSelection: true } ) ).to.match( new RegExp( - '' + + '
' + '' + '' + '00' + @@ -2311,6 +2402,11 @@ describe( 'TableColumnResizeEditing', () => { '02' + '' + '' + + '' + + '' + + '' + + '' + + '' + '
' ) ); @@ -2346,7 +2442,7 @@ describe( 'TableColumnResizeEditing', () => { const unlinkCommand = editor.commands.get( 'unlink' ); setModelData( model, - '' + + '
' + '' + '' + '' + @@ -2354,6 +2450,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + + '' + + '' + + '' + '
' ); @@ -2362,12 +2461,15 @@ describe( 'TableColumnResizeEditing', () => { unlinkCommand.execute(); expect( getModelData( model ) ).to.equal( - '' + + '
' + '' + '' + '[foo]' + '' + '' + + '' + + '' + + '' + '
' ); } ); @@ -2376,7 +2478,7 @@ describe( 'TableColumnResizeEditing', () => { const highlightCommand = editor.commands.get( 'highlight' ); setModelData( model, - '' + + '
' + '' + '' + '' + @@ -2384,6 +2486,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + + '' + + '' + + '' + '
' ); @@ -2392,19 +2497,22 @@ describe( 'TableColumnResizeEditing', () => { highlightCommand.execute(); expect( getModelData( model ) ).to.equal( - '' + + '
' + '' + '' + '[foo]' + '' + '' + + '' + + '' + + '' + '
' ); } ); it( 'when bold is being removed', () => { setModelData( model, - '' + + '
' + '' + '' + '' + @@ -2412,18 +2520,24 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + + '' + + '' + + '' + '
' ); editor.commands.get( 'bold' ).execute(); expect( getModelData( model ) ).to.equal( - '' + + '
' + '' + '' + '[foo]' + '' + '' + + '' + + '' + + '' + '
' ); } ); @@ -2433,7 +2547,7 @@ describe( 'TableColumnResizeEditing', () => { const toolbar = widgetToolbarRepository._toolbarDefinitions.get( 'tableContent' ).view; setModelData( model, - '' + + '
' + '' + '' + '' + @@ -2441,18 +2555,24 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + + '' + + '' + + '' + '
' ); toolbar.items.get( 0 ).fire( 'execute' ); expect( getModelData( model ) ).to.equal( - '' + + '
' + '' + '' + 'foo' + '' + '' + + '' + + '' + + '' + '' + '
[]
' ); @@ -2460,7 +2580,7 @@ describe( 'TableColumnResizeEditing', () => { it( 'when table is being removed', () => { setModelData( model, - '[' + + '[
' + '' + '' + '' + @@ -2468,6 +2588,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + + '' + + '' + + '' + '
]' ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js index e348e5adba5..138ac7561f0 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js @@ -47,10 +47,10 @@ describe( 'TableColumnWidthsCommand', () => { [ '21', '22' ] ] ) ); - command.execute( { columnWidths: '40%,60%' } ); + command.execute( { columnWidths: [ 40, 60 ] } ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -67,6 +67,10 @@ describe( 'TableColumnWidthsCommand', () => { '22' + '' + '' + + '' + + '' + + '' + + '' + '
' ); } ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js index a76f7050d4f..0d5824d23f0 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js @@ -54,7 +54,7 @@ describe( 'TableWidthResizeCommand', () => { it( 'should remove the attributes if new value was not passed', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%', columnWidths: '40%,60%' }; + const attributesBefore = { tableWidth: '40%' }; setModelData( model, modelTable( data, attributesBefore ) ); command.execute(); @@ -63,20 +63,9 @@ describe( 'TableWidthResizeCommand', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); } ); - it( 'should work when only columnWidths is provided', () => { - const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%', columnWidths: '40%,60%' }; - setModelData( model, modelTable( data, attributesBefore ) ); - - command.execute( { columnWidths: '40%,60%' } ); - - const attributesAfter = { columnWidths: '40%,60%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); - } ); - it( 'should work when only tableWidth is provided', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%', columnWidths: '40%,60%' }; + const attributesBefore = { tableWidth: '40%' }; setModelData( model, modelTable( data, attributesBefore ) ); command.execute( { tableWidth: '40%' } ); @@ -90,20 +79,20 @@ describe( 'TableWidthResizeCommand', () => { const attributesBefore = {}; setModelData( model, modelTable( data, attributesBefore ) ); - command.execute( { tableWidth: '40%', columnWidths: '40%,60%' } ); + command.execute( { tableWidth: '40%' } ); - const attributesAfter = { tableWidth: '40%', columnWidths: '40%,60%' }; + const attributesAfter = { tableWidth: '40%' }; expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); } ); it( 'should change attributes when they were present before', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%', columnWidths: '40%,60%' }; + const attributesBefore = { tableWidth: '40%' }; setModelData( model, modelTable( data, attributesBefore ) ); - command.execute( { tableWidth: '30%', columnWidths: '30%,70%' } ); + command.execute( { tableWidth: '30%' } ); - const attributesAfter = { tableWidth: '30%', columnWidths: '30%,70%' }; + const attributesAfter = { tableWidth: '30%' }; expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); } ); } ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index df82766dacc..dda7d082a7c 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -715,11 +715,19 @@ describe( 'TableColumnResize utils', () => { } ); function createTable( rows, cols ) { - // We need to set the `columnWidths` attribute because tables without it are ignored. + return new Element( 'table', {}, [ + ...createTableRows( rows, cols ), + createColGroupRow( cols ) + ] ); +} + +function createColGroupRow( cols ) { const colWidth = `${ 100 / cols }%`; - const columnWidths = new Array( cols ).fill( colWidth ).join( ',' ); + const columns = new Array( cols ) + .fill( colWidth ) + .map( columnWidth => new Element( 'tableColumn', { columnWidth } ) ); - return new Element( 'table', { columnWidths }, createTableRows( rows, cols ) ); + return new Element( 'tableColumnGroup', {}, columns ); } function createTableRows( rows, cols ) { From eac67469d7161888cbb006b6bf990ad79d3ddff6 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Thu, 26 Jan 2023 12:58:32 +0100 Subject: [PATCH 04/25] Update upcasting method in TableColumnResize --- .../src/tablecolumnresize/converters.js | 77 ++----------------- .../tablecolumnresizeediting.js | 24 ++---- .../tablecolumnresizeediting.js | 23 +++--- 3 files changed, 24 insertions(+), 100 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index b0a50c79984..7a63c9cca41 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -48,82 +48,15 @@ export function upcastColgroupElement( tableUtilsPlugin ) { return 'auto'; } - return viewColWidth; + return Number( viewColWidth.replace( '%', '' ) ); } ); if ( columnWidths.includes( 'auto' ) ) { - columnWidths = normalizeColumnWidths( columnWidths ).map( width => width + '%' ); + columnWidths = normalizeColumnWidths( columnWidths ); } - conversionApi.writer.setAttribute( 'columnWidths', columnWidths.join( ',' ), modelTable ); + const colGroupElement = conversionApi.writer.createElement( 'tableColumnGroup' ); + columnWidths.forEach( columnWidth => conversionApi.writer.appendElement( 'tableColumn', { columnWidth }, colGroupElement ) ); + conversionApi.writer.append( colGroupElement, modelTable ); } ); } - -/** - * Returns a helper for converting a model table `columnWidths` attribute to view `` and `` elements. - * - * @returns {Function} Conversion helper. - */ -export function downcastTableColumnWidthsAttribute() { - return dispatcher => dispatcher.on( 'attribute:columnWidths:table', ( evt, data, conversionApi ) => { - const viewWriter = conversionApi.writer; - const modelTable = data.item; - const viewElement = conversionApi.mapper.toViewElement( modelTable ); - - const viewTable = viewElement.is( 'element', 'table' ) ? - viewElement : - Array.from( viewElement.getChildren() ).find( viewChild => viewChild.is( 'element', 'table' ) ); - - if ( data.attributeNewValue ) { - // If new value is the same as the old, the operation is not applied (see the `writer.setAttributeOnItem()`). - // OTOH the model element has the attribute already applied, so we can't compare the values. - // Hence we need to just recreate the element every time. - insertColgroupElement( viewWriter, viewTable, data.attributeNewValue ); - viewWriter.addClass( 'ck-table-resized', viewTable ); - } else { - removeColgroupElement( viewWriter, viewTable ); - viewWriter.removeClass( 'ck-table-resized', viewTable ); - } - } ); -} - -// Inserts the `` with `` elements as the first child in the view table. Each `` element represents a single column -// and it has the inline width style set, taken from the appropriate slot from the `columnWidths` table attribute. -// -// @private -// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter View writer instance. -// @param {module:engine/view/element~Element} viewTable View table. -// @param {String} columnWidthsAttribute Column widths attribute from model table. -function insertColgroupElement( viewWriter, viewTable, columnWidthsAttribute ) { - const columnWidths = columnWidthsAttribute.split( ',' ); - - let viewColgroupElement = [ ...viewTable.getChildren() ].find( viewElement => viewElement.is( 'element', 'colgroup' ) ); - - if ( !viewColgroupElement ) { - viewColgroupElement = viewWriter.createContainerElement( 'colgroup' ); - } else { - for ( const viewChild of [ ...viewColgroupElement.getChildren() ] ) { - viewWriter.remove( viewChild ); - } - } - - for ( const columnIndex of Array( columnWidths.length ).keys() ) { - const viewColElement = viewWriter.createEmptyElement( 'col' ); - - viewWriter.setStyle( 'width', columnWidths[ columnIndex ], viewColElement ); - viewWriter.insert( viewWriter.createPositionAt( viewColgroupElement, 'end' ), viewColElement ); - } - - viewWriter.insert( viewWriter.createPositionAt( viewTable, 'start' ), viewColgroupElement ); -} - -// Removes the `` with `` elements from the view table. -// -// @private -// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter View writer instance. -// @param {module:engine/view/element~Element} viewTable View table. -function removeColgroupElement( viewWriter, viewTable ) { - const viewColgroupElement = [ ...viewTable.getChildren() ].find( viewElement => viewElement.is( 'element', 'colgroup' ) ); - - viewWriter.remove( viewColgroupElement ); -} diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index c3eb68f1370..4d4add6ddea 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -19,6 +19,8 @@ import TableWalker from '../tablewalker'; import TableWidthResizeCommand from './tablewidthresizecommand'; import TableColumnWidthsCommand from './tablecolumnwidthscommand'; +import { upcastColgroupElement } from './converters'; + import { clamp, createFilledArray, @@ -220,10 +222,13 @@ export default class TableColumnResizeEditing extends Plugin { const columnWidth = normalizedWidths[ i ]; if ( !columnWidth ) { + // Number of `` elements exceeds actual number of columns writer.remove( column ); } else if ( !column ) { + // There is fewer `` elements than actual columns writer.appendElement( 'tableColumn', { columnWidth }, tableColumnGroup ); } else { + // Update column width writer.setAttribute( 'columnWidth', columnWidth, column ); } } @@ -349,27 +354,10 @@ export default class TableColumnResizeEditing extends Plugin { } ); // Table conversion - conversion.for( 'upcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); + conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); conversion.for( 'downcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); - - // Table conversion - conversion.for( 'upcast' ).elementToElement( { model: 'tableColumn', view: 'col' } ); conversion.for( 'downcast' ).elementToElement( { model: 'tableColumn', view: 'col' } ); - // Table column width conversion - conversion.for( 'upcast' ).attributeToAttribute( { - view: { - name: 'col', - key: 'style', - value: { width: /[\s\S]+/ } - }, - model: { - name: 'tableColumn', - key: 'columnWidth', - value: element => Number( element.getStyle( 'width' ).replace( '%', '' ) ) - } - } ); - conversion.for( 'downcast' ).attributeToAttribute( { model: { name: 'tableColumn', diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 24265d72dca..744a0caf344 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -438,28 +438,31 @@ describe( 'TableColumnResizeEditing', () => { expect( editor.getData() ).to.equal( '
' + '' + - '' + - '' + - '' + - '' + '' + '' + '' + '' + '' + '' + + '' + + '' + + '' + + '' + '
1112
' + '
' ); } ); - it( 'should remove element if columnWidths attribute was removed', () => { + it( 'should remove element if element was removed', () => { setModelData( model, modelTable( [ [ '11', '12' ] ], { columnWidths: '50%,50%', tableWidth: '100%' } ) ); model.change( writer => { - writer.removeAttribute( 'columnWidths', model.document.getRoot().getChild( 0 ) ); + const tableColumnGroup = Array + .from( model.document.getRoot().getChild( 0 ) ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + writer.remove( tableColumnGroup ); } ); expect( editor.getData() ).to.equal( @@ -488,16 +491,16 @@ describe( 'TableColumnResizeEditing', () => { expect( editor.getData() ).to.equal( '
' + '' + - '' + - '' + - '' + - '' + '' + '' + '' + '' + '' + '' + + '' + + '' + + '' + + '' + '
1112
' + '
' ); From 0cd3b307f332f986374edd8d272bbbcaccdce73d Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Thu, 26 Jan 2023 15:04:09 +0100 Subject: [PATCH 05/25] Add % sign to `columnWidth` attribute values --- .../src/converters/downcast.js | 7 +- .../src/tablecolumnresize/converters.js | 4 +- .../tablecolumnresizeediting.js | 12 +- .../tablecolumnwidthscommand.js | 3 +- packages/ckeditor5-table/src/tableediting.js | 4 +- .../ckeditor5-table/tests/_utils/utils.js | 4 +- .../tests/tablecolumnresize/_utils/utils.js | 3 +- .../tablecolumnresizeediting.js | 169 ++++++++---------- .../tablecolumnwidthscommand.js | 4 +- 9 files changed, 92 insertions(+), 118 deletions(-) diff --git a/packages/ckeditor5-table/src/converters/downcast.js b/packages/ckeditor5-table/src/converters/downcast.js index 0581be652e3..774e980eeff 100644 --- a/packages/ckeditor5-table/src/converters/downcast.js +++ b/packages/ckeditor5-table/src/converters/downcast.js @@ -16,12 +16,13 @@ import { toWidget, toWidgetEditable } from 'ckeditor5/src/widget'; * @param {module:table/tableutils~TableUtils} tableUtils The `TableUtils` plugin instance. * @param {Object} [options] * @param {Boolean} [options.asWidget] If set to `true`, the downcast conversion will produce a widget. + * @param {Array.} [options.additionalSlots] Array of additional slot handlers. * @returns {Function} Element creator. */ export function downcastTable( tableUtils, options = {} ) { return ( table, { writer } ) => { const headingRows = table.getAttribute( 'headingRows' ) || 0; - const additional = options.additional || []; + const additionalSlots = options.additionalSlots || []; const tableElement = writer.createContainerElement( 'table', null, [] ); // Table head slot. @@ -49,7 +50,7 @@ export function downcastTable( tableUtils, options = {} ) { } // Dynamic slots. - for ( const { positionOffset, filter } of additional ) { + for ( const { positionOffset, filter } of additionalSlots ) { writer.insert( writer.createPositionAt( tableElement, positionOffset ), writer.createSlot( filter ) @@ -65,7 +66,7 @@ export function downcastTable( tableUtils, options = {} ) { return false; } - return !additional.some( ( { filter } ) => filter( element ) ); + return !additionalSlots.some( ( { filter } ) => filter( element ) ); } ) ] ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index 7a63c9cca41..c84f7c22386 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -48,11 +48,11 @@ export function upcastColgroupElement( tableUtilsPlugin ) { return 'auto'; } - return Number( viewColWidth.replace( '%', '' ) ); + return viewColWidth; } ); if ( columnWidths.includes( 'auto' ) ) { - columnWidths = normalizeColumnWidths( columnWidths ); + columnWidths = normalizeColumnWidths( columnWidths ).map( width => `${ width }%` ); } const colGroupElement = conversionApi.writer.createElement( 'tableColumnGroup' ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 4d4add6ddea..343c6121ba9 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -205,7 +205,7 @@ export default class TableColumnResizeEditing extends Plugin { .find( element => element.is( 'element', 'tableColumnGroup' ) ); const columns = Array.from( tableColumnGroup.getChildren() ); - const columnWidths = columns.map( element => element.getAttribute( 'columnWidth' ) ); + const columnWidths = columns.map( element => Number( element.getAttribute( 'columnWidth' ).replace( '%', '' ) ) ); // Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. const normalizedWidths = normalizeColumnWidths( columnWidths ); @@ -219,17 +219,17 @@ export default class TableColumnResizeEditing extends Plugin { for ( let i = 0; i < Math.max( normalizedWidths.length, columns.length ); i++ ) { const column = columns[ i ]; - const columnWidth = normalizedWidths[ i ]; + const width = normalizedWidths[ i ]; - if ( !columnWidth ) { + if ( !width ) { // Number of `` elements exceeds actual number of columns writer.remove( column ); } else if ( !column ) { // There is fewer `` elements than actual columns - writer.appendElement( 'tableColumn', { columnWidth }, tableColumnGroup ); + writer.appendElement( 'tableColumn', { columnWidth: `${ width }%` }, tableColumnGroup ); } else { // Update column width - writer.setAttribute( 'columnWidth', columnWidth, column ); + writer.setAttribute( 'columnWidth', `${ width }%`, column ); } } @@ -363,7 +363,7 @@ export default class TableColumnResizeEditing extends Plugin { name: 'tableColumn', key: 'columnWidth' }, - view: width => ( { key: 'style', value: { width: `${ width }%` } } ) + view: width => ( { key: 'style', value: { width } } ) } ); } diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js index 043180603cd..92601f69ab8 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js @@ -60,7 +60,8 @@ export default class TableColumnWidthsCommand extends TablePropertyCommand { return writer.remove( tableColumnGroup ); } - const widths = normalizeColumnWidths( columnWidths ); + let widths = columnWidths.map( widths => Number( widths.replace( '%', '' ) ) ); + widths = normalizeColumnWidths( widths ).map( width => `${ width }%` ); if ( !tableColumnGroup ) { const colGroupElement = writer.createElement( 'tableColumnGroup' ); diff --git a/packages/ckeditor5-table/src/tableediting.js b/packages/ckeditor5-table/src/tableediting.js index c637ff72073..314b5e1b83d 100644 --- a/packages/ckeditor5-table/src/tableediting.js +++ b/packages/ckeditor5-table/src/tableediting.js @@ -110,7 +110,7 @@ export default class TableEditing extends Plugin { }, view: downcastTable( tableUtils, { asWidget: true, - additional: this._additionalSlots + additionalSlots: this._additionalSlots } ) } ); conversion.for( 'dataDowncast' ).elementToStructure( { @@ -119,7 +119,7 @@ export default class TableEditing extends Plugin { attributes: [ 'headingRows' ] }, view: downcastTable( tableUtils, { - additional: this._additionalSlots + additionalSlots: this._additionalSlots } ) } ); diff --git a/packages/ckeditor5-table/tests/_utils/utils.js b/packages/ckeditor5-table/tests/_utils/utils.js index b466b51ca4f..43132e7f856 100644 --- a/packages/ckeditor5-table/tests/_utils/utils.js +++ b/packages/ckeditor5-table/tests/_utils/utils.js @@ -418,7 +418,6 @@ function makeColGroup( columnWidths ) { const cols = columnWidths .split( ',' ) - .map( width => width.replace( '%', '' ) ) .map( width => `` ) .join( '' ); @@ -581,6 +580,5 @@ export function getTableColumnWidths( table ) { .filter( element => element.is( 'element', 'tableColumnGroup' ) ) .map( element => Array.from( element.getChildren() ) ) .flat() - .map( element => element.getAttribute( 'columnWidth' ) ) - .map( width => Number( width ) ); + .map( element => element.getAttribute( 'columnWidth' ) ); } diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js index 3a3a0b54f3b..f6711ad9375 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js @@ -7,6 +7,7 @@ import { global } from 'ckeditor5/src/utils'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import { Point } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; import TableColumnResizeEditing from '../../../src/tablecolumnresize/tablecolumnresizeediting'; +import { getTableColumnWidths } from '../../_utils/utils'; export const tableColumnResizeMouseSimulator = { down( editor, domTarget, options = {} ) { @@ -83,7 +84,7 @@ export function getViewColumnWidthsPx( domTable ) { } export function getModelColumnWidthsPc( modelTable ) { - return modelTable.getAttribute( 'columnWidths' ).replaceAll( '%', '' ).split( ',' ); + return getTableColumnWidths( modelTable ).map( width => width.replace( '%', '' ) ); } export function getViewColumnWidthsPc( viewTable ) { diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 744a0caf344..42d2d79dd63 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -81,7 +81,7 @@ describe( 'TableColumnResizeEditing', () => { [ '10', '11', '12' ] ], { columnWidths: '25%,25%,50%' } ) ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ 25, 25, 50 ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ '25%', '25%', '50%' ] ); } ); it( 'should have defined col widths in view', () => { @@ -132,8 +132,8 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + '' ); @@ -174,9 +174,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ); @@ -215,9 +215,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ); @@ -258,9 +258,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ); @@ -298,9 +298,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ); @@ -338,9 +338,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ); @@ -419,9 +419,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ); @@ -478,33 +478,6 @@ describe( 'TableColumnResizeEditing', () => { '' ); } ); - - it( 'should do nothing if columnWidths value was changed to the same value', () => { - setModelData( model, modelTable( [ - [ '11', '12' ] - ], { columnWidths: '50%,50%', tableWidth: '100%' } ) ); - - model.change( writer => { - writer.setAttribute( 'columnWidths', '50%,50%', model.document.getRoot().getChild( 0 ) ); - } ); - - expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1112
' + - '
' - ); - } ); } ); describe( 'model change integration', () => { @@ -753,7 +726,7 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, view.getDomRoot() ); expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingActive ).to.be.false; - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); it( 'if resizing is not allowed', () => { @@ -767,7 +740,7 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 0, 0 ) ); expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingActive ).to.be.false; - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); it( 'without dragging', () => { @@ -818,7 +791,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); } ); @@ -841,7 +814,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); it( 'does nothing on mouseup if resizing was not started', () => { @@ -857,7 +830,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); it( 'does not change the widths if the movement vector was {0,0}', () => { @@ -914,7 +887,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); it( 'if columnWidths was set for the first time', () => { @@ -935,7 +908,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.be.undefined; + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; } ); it( 'if tableWidth was changed', () => { @@ -956,7 +929,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); it( 'if tableWidth was set for the first time', () => { @@ -977,7 +950,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( model.document.getRoot().getChild( 0 ).getAttribute( 'columnWidths' ) ).to.equal( '20%,25%,55%' ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); } ); } ); @@ -1390,14 +1363,14 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + ']' + '' + '' + '' + - '' + + '' + '' + '' ); @@ -1446,14 +1419,14 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + '' + '' + '' + '' + - '' + + '' + '' + '' ) @@ -1479,14 +1452,14 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + ']' + '' + '' + '' + - '' + + '' + '' + '' ); @@ -1535,14 +1508,14 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + '' + '' + '' + '' + - '' + + '' + '' + '' ) @@ -1571,15 +1544,15 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + ']' + '' + '' + '' + - '' + + '' + '' + '' ); @@ -1630,15 +1603,15 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' + '' + '' + '' + - '' + + '' + '' + '' ); @@ -2285,9 +2258,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + ']' ); @@ -2334,9 +2307,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ) @@ -2369,9 +2342,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + ']' ); @@ -2406,9 +2379,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ) @@ -2454,7 +2427,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2471,7 +2444,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2490,7 +2463,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2507,7 +2480,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2524,7 +2497,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2539,7 +2512,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2559,7 +2532,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' ); @@ -2574,7 +2547,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '[]' + '' @@ -2592,7 +2565,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + ']' ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js index ee291349f28..c6e4a46f137 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js @@ -68,8 +68,8 @@ describe( 'TableColumnWidthsCommand', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + '' ); From 0f850f4e7dc2348a459bf215d5f99881b499c1a8 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Thu, 26 Jan 2023 17:03:20 +0100 Subject: [PATCH 06/25] Downcast 'ck-table-resized' table class --- .../src/tablecolumnresize/converters.js | 30 ++++++++++- .../tablecolumnresizeediting.js | 34 ++++++------ .../tablecolumnwidthscommand.js | 2 +- packages/ckeditor5-table/src/tableediting.js | 54 +++++++++++-------- 4 files changed, 80 insertions(+), 40 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index c84f7c22386..54793c709bf 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -52,11 +52,39 @@ export function upcastColgroupElement( tableUtilsPlugin ) { } ); if ( columnWidths.includes( 'auto' ) ) { - columnWidths = normalizeColumnWidths( columnWidths ).map( width => `${ width }%` ); + columnWidths = normalizeColumnWidths( columnWidths ).map( width => width + '%' ); } const colGroupElement = conversionApi.writer.createElement( 'tableColumnGroup' ); + columnWidths.forEach( columnWidth => conversionApi.writer.appendElement( 'tableColumn', { columnWidth }, colGroupElement ) ); conversionApi.writer.append( colGroupElement, modelTable ); } ); } + +/** + * Returns downcast helper for adding `ck-table-resized` class if there is a `` element inside the table + * + * @returns {Function} Conversion helper. + */ +export function downcastTableResizedClass() { + return dispatcher => dispatcher.on( 'insert:table', ( evt, data, conversionApi ) => { + const viewWriter = conversionApi.writer; + const modelTable = data.item; + const viewElement = conversionApi.mapper.toViewElement( modelTable ); + + const viewTable = viewElement.is( 'element', 'table' ) ? + viewElement : + Array.from( viewElement.getChildren() ).find( viewChild => viewChild.is( 'element', 'table' ) ); + + const tableColumnGroup = Array + .from( data.item.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + + if ( tableColumnGroup ) { + viewWriter.addClass( 'ck-table-resized', viewTable ); + } else { + viewWriter.removeClass( 'ck-table-resized', viewTable ); + } + }, { priority: 'low' } ); +} diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 343c6121ba9..8e519df03c3 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -19,7 +19,10 @@ import TableWalker from '../tablewalker'; import TableWidthResizeCommand from './tablewidthresizecommand'; import TableColumnWidthsCommand from './tablecolumnwidthscommand'; -import { upcastColgroupElement } from './converters'; +import { + upcastColgroupElement, + downcastTableResizedClass +} from './converters'; import { clamp, @@ -353,8 +356,8 @@ export default class TableColumnResizeEditing extends Plugin { } ) } ); - // Table conversion conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); + conversion.for( 'downcast' ).add( downcastTableResizedClass() ); conversion.for( 'downcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); conversion.for( 'downcast' ).elementToElement( { model: 'tableColumn', view: 'col' } ); @@ -613,20 +616,21 @@ export default class TableColumnResizeEditing extends Plugin { if ( isColumnWidthsAttributeChanged || isTableWidthAttributeChanged ) { if ( this._isResizingAllowed ) { - // Commit all changes to the model. - if ( isTableWidthAttributeChanged ) { - editor.execute( 'resizeTableWidth', { - table: modelTable, - tableWidth: `${ toPrecision( tableWidthAttributeNew ) }%` - } ); - } + editor.model.change( () => { + if ( isTableWidthAttributeChanged ) { + editor.execute( 'resizeTableWidth', { + table: modelTable, + tableWidth: `${ toPrecision( tableWidthAttributeNew ) }%` + } ); + } - if ( isColumnWidthsAttributeChanged ) { - editor.execute( 'resizeColumnWidths', { - columnWidths: columnWidthsAttributeNew, - table: modelTable - } ); - } + if ( isColumnWidthsAttributeChanged ) { + editor.execute( 'resizeColumnWidths', { + columnWidths: columnWidthsAttributeNew, + table: modelTable + } ); + } + } ); } else { // In read-only mode revert all changes in the editing view. The model is not touched so it does not need to be restored. // This case can occur if the read-only mode kicks in during the resizing process. diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js index 92601f69ab8..60f30c8612c 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js @@ -37,7 +37,7 @@ export default class TableColumnWidthsCommand extends TablePropertyCommand { * Changes the `columnWidth` attribute values for the columns of the given table. * * @param {Object} options - * @param {Array.} [options.columnWidths] New value of the `columnWidths` attribute. + * @param {Array.} [options.columnWidths] New value of the `columnWidths` attribute. * @param {module:engine/model/element~Element} [options.table] The table that is having the columns resized. */ execute( options = {} ) { diff --git a/packages/ckeditor5-table/src/tableediting.js b/packages/ckeditor5-table/src/tableediting.js index 314b5e1b83d..71cb3ebe1a9 100644 --- a/packages/ckeditor5-table/src/tableediting.js +++ b/packages/ckeditor5-table/src/tableediting.js @@ -245,38 +245,46 @@ function upcastCellSpan( type ) { * * Take this model as an example: * - * - * ... - * ... - * ... - *
+ * ``` + * + * ... + * ... + * ... + *
+ * ``` * * By default, downcasting result will be: * - * - * - * ... - * ... - * - *
- * ... + * ``` + * + * + * ... + * ... + * + *
+ * ... + * ``` * * To allow the `tableColumnGroup` element at the end of the table, use the following configuration: * - * const additionalSlot = { - * filter: element => element.is( 'element', 'tableColumnGroup' ), - * positionOffset: 'end' - * } + * ``` + * const additionalSlot = { + * filter: element => element.is( 'element', 'tableColumnGroup' ), + * positionOffset: 'end' + * } + * ``` * * Now, the downcast result will be: * - * - * - * ... - * ... - * - * ... - *
+ * ``` + * + * + * ... + * ... + * + * ... + *
+ * ``` * * @typedef {Object} module:table/tablediting~AdditionalSlot * From 3d749e6dba09b4c6c7bacc05e6920e5d02da163f Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Thu, 26 Jan 2023 17:03:32 +0100 Subject: [PATCH 07/25] Fix tests --- .../tablecolumnresizeediting.js | 17 +++++++++-------- .../tablecolumnwidthscommand.js | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 42d2d79dd63..33386ad32e6 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -460,7 +460,7 @@ describe( 'TableColumnResizeEditing', () => { model.change( writer => { const tableColumnGroup = Array - .from( model.document.getRoot().getChild( 0 ) ) + .from( model.document.getRoot().getChild( 0 ).getChildren() ) .find( element => element.is( 'element', 'tableColumnGroup' ) ); writer.remove( tableColumnGroup ); } ); @@ -1159,7 +1159,7 @@ describe( 'TableColumnResizeEditing', () => { expect( view.document.getRoot() .getChild( 0 ) // figure .getChild( 1 ) // table - .getChild( 1 ) // tbody + .getChild( 0 ) // tbody .getChild( 0 ) // tr .childCount ).to.equal( 3 ); @@ -1202,7 +1202,7 @@ describe( 'TableColumnResizeEditing', () => { expect( view.document.getRoot() .getChild( 0 ) // figure .getChild( 1 ) // table - .getChild( 1 ) // tbody + .getChild( 0 ) // tbody .getChild( 0 ) // tr .childCount ).to.equal( 3 ); @@ -1419,8 +1419,8 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + '' + '' + @@ -1773,7 +1773,7 @@ describe( 'TableColumnResizeEditing', () => { expect( view.document.getRoot() .getChild( 0 ) // figure .getChild( 1 ) // table - .getChild( 1 ) // tbody + .getChild( 0 ) // tbody .getChild( 0 ) // tr .childCount ).to.equal( 3 ); @@ -1978,7 +1978,8 @@ describe( 'TableColumnResizeEditing', () => { editor.execute( 'undo' ); editor.execute( 'undo' ); - const tableRow = getDomTable( view ).children[ 1 ] // table + const tableRow = getDomTable( view ) + .children[ 1 ] // table .children[ 0 ] // tbody .children[ 0 ]; // tr @@ -2379,7 +2380,7 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + + '' + '' + '' + '' + diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js index c6e4a46f137..86f175ab980 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js @@ -47,7 +47,7 @@ describe( 'TableColumnWidthsCommand', () => { [ '21', '22' ] ] ) ); - command.execute( { columnWidths: [ 40, 60 ] } ); + command.execute( { columnWidths: [ '40%', '%60' ] } ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '' + From cbb4e0206014afba4382b9c83d74cdc11789c90b Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Fri, 27 Jan 2023 12:52:04 +0100 Subject: [PATCH 08/25] Fix reverting changes when read-only mode is enabled when resizing table --- .../src/tablecolumnresize/tablecolumnresizeediting.js | 2 +- .../tests/tablecolumnresize/tablecolumnresizeediting.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 8e519df03c3..e643b7049b0 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -637,7 +637,7 @@ export default class TableColumnResizeEditing extends Plugin { editingView.change( writer => { // If table had resized columns before, restore the previous column widths. // Otherwise clean up the view from the temporary column resizing markup. - if ( columnWidthsAttributeOld ) { + if ( columnWidthsAttributeOld.length > 0 ) { for ( const viewCol of viewColgroup.getChildren() ) { writer.setStyle( 'width', columnWidthsAttributeOld.shift(), viewCol ); } diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 33386ad32e6..3e84def13fc 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -908,7 +908,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [] ); } ); it( 'if tableWidth was changed', () => { From 30588d7620eeca5eb6f60965e09597301e7b1277 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 30 Jan 2023 15:37:08 +0100 Subject: [PATCH 09/25] ElementToStructure should not override attributeToAttribute converter on the same node. --- .../src/conversion/downcasthelpers.ts | 15 ++++++++++++++- .../tablecolumnresize/tablecolumnresizeediting.js | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts b/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts index 1b13edb7057..fdbb90ce9be 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts @@ -2450,8 +2450,21 @@ function createChangeReducer( model: NormalizedModelElementConfig ) { data.reconvertedElements.add( node ); const position = ModelPosition._createBefore( node ); + let idx = reducedChanges.length; + + // We need to insert remove+reinsert before any attribute change on the same node. + // This is important because otherwise we would remove node that had already modified attribute. + for ( let i = reducedChanges.length - 1; i >= 0; i-- ) { + const change = reducedChanges[ i ]; + + if ( change.type == 'attribute' && change.range.start.isEqual( position ) ) { + idx = i; + } else { + break; + } + } - reducedChanges.push( { + reducedChanges.splice( idx, 0, { type: 'remove', name: ( node as ModelElement ).name, position, diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index e643b7049b0..b06bc9cdbf2 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -133,7 +133,7 @@ export default class TableColumnResizeEditing extends Plugin { editor.plugins.get( 'TableEditing' ).registerAdditionalSlot( { filter: element => element.is( 'element', 'tableColumnGroup' ), - positionOffset: 'end' + positionOffset: 0 } ); editor.commands.add( 'resizeTableWidth', new TableWidthResizeCommand( editor ) ); From 9e0bd63f986f1efdbc7efa2c9caf9598a18cc3a0 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Mon, 30 Jan 2023 15:50:07 +0100 Subject: [PATCH 10/25] Fix tests for tablecolumnresize --- .../tablecolumnresize/tablecolumnresizeediting.js | 5 +++-- .../tablecolumnresize/tablecolumnwidthscommand.js | 14 ++------------ .../tablecolumnresize/tablewidthresizecommand.js | 2 +- .../tablecolumnresize/tablecolumnresizeediting.js | 14 +++++++------- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index b06bc9cdbf2..2c8461edae5 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -356,6 +356,7 @@ export default class TableColumnResizeEditing extends Plugin { } ) } ); + // Table and elements and styles conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); conversion.for( 'downcast' ).add( downcastTableResizedClass() ); conversion.for( 'downcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); @@ -601,7 +602,7 @@ export default class TableColumnResizeEditing extends Plugin { const columnWidthsAttributeOld = tableColumnGroup ? Array.from( tableColumnGroup.getChildren() ).map( element => element.getAttribute( 'columnWidth' ) ) : - []; + null; const columnWidthsAttributeNew = Array .from( viewColgroup.getChildren() ) @@ -637,7 +638,7 @@ export default class TableColumnResizeEditing extends Plugin { editingView.change( writer => { // If table had resized columns before, restore the previous column widths. // Otherwise clean up the view from the temporary column resizing markup. - if ( columnWidthsAttributeOld.length > 0 ) { + if ( columnWidthsAttributeOld ) { for ( const viewCol of viewColgroup.getChildren() ) { writer.setStyle( 'width', columnWidthsAttributeOld.shift(), viewCol ); } diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js index 60f30c8612c..c46810d5689 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js @@ -7,23 +7,13 @@ * @module table/tablecolumnresize/tablecolumnwidthscommand */ +import { Command } from 'ckeditor5/src/core'; import { normalizeColumnWidths } from './utils'; -import TablePropertyCommand from '../tableproperties/commands/tablepropertycommand'; /** * @extends module:table/tableproperties/commands/tablepropertycommand~TablePropertyCommand */ -export default class TableColumnWidthsCommand extends TablePropertyCommand { - /** - * Creates a new `TableColumnWidthsCommand` instance. - * - * @param {module:core/editor/editor~Editor} editor An editor in which this command will be used. - * @param {String} defaultValue The default value of the attribute. - */ - constructor( editor, defaultValue ) { - super( editor, 'columnWidths', defaultValue ); - } - +export default class TableColumnWidthsCommand extends Command { /** * @inheritDoc */ diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js index 2e445059ebd..ab0ad72f355 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js @@ -43,7 +43,7 @@ export default class TableWidthResizeCommand extends TablePropertyCommand { * @param {module:engine/model/element~Element} [options.table] The table that is affected by the resize. */ execute( options = {} ) { - const model = this.editor.model; + const { model } = this.editor; const { table = model.document.selection.getSelectedElement(), tableWidth diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 3e84def13fc..12a0be9c296 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -438,16 +438,16 @@ describe( 'TableColumnResizeEditing', () => { expect( editor.getData() ).to.equal( '
' + '
' + + '' + + '' + + '' + + '' + '' + '' + '' + '' + '' + '' + - '' + - '' + - '' + - '' + '
1112
' + '' ); @@ -1159,7 +1159,7 @@ describe( 'TableColumnResizeEditing', () => { expect( view.document.getRoot() .getChild( 0 ) // figure .getChild( 1 ) // table - .getChild( 0 ) // tbody + .getChild( 1 ) // tbody .getChild( 0 ) // tr .childCount ).to.equal( 3 ); @@ -1202,7 +1202,7 @@ describe( 'TableColumnResizeEditing', () => { expect( view.document.getRoot() .getChild( 0 ) // figure .getChild( 1 ) // table - .getChild( 0 ) // tbody + .getChild( 1 ) // tbody .getChild( 0 ) // tr .childCount ).to.equal( 3 ); @@ -1773,7 +1773,7 @@ describe( 'TableColumnResizeEditing', () => { expect( view.document.getRoot() .getChild( 0 ) // figure .getChild( 1 ) // table - .getChild( 0 ) // tbody + .getChild( 1 ) // tbody .getChild( 0 ) // tr .childCount ).to.equal( 3 ); From 773e7cbb5ba291357c4b6ebd500b6dc1122afb85 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 30 Jan 2023 17:30:54 +0100 Subject: [PATCH 11/25] Added tests. --- .../tests/conversion/downcasthelpers.js | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js index 1e41a2b6684..df94c3f528b 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js @@ -665,6 +665,75 @@ describe( 'DowncastHelpers', () => { expect( spy.called ).to.be.true; } ); + it( 'should convert attribute change and add a child at the same time (separate converters)', () => { + model.schema.extend( 'simpleBlock', { allowAttributes: 'someOther' } ); + downcastHelpers.attributeToAttribute( { model: 'someOther', view: 'data-other' } ); + + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.setAttribute( 'someOther', 'foo', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should convert attribute change, remove element before, and add a child at the same time (separate)', () => { + model.schema.extend( 'simpleBlock', { allowAttributes: 'someOther' } ); + downcastHelpers.attributeToAttribute( { model: 'someOther', view: 'data-other' } ); + + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes( 1 ); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.remove( modelRoot.getChild( 0 ) ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.setAttribute( 'someOther', 'foo', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); + it( 'should convert on removing a child', () => { setModelData( model, 'foobar' ); @@ -1502,6 +1571,75 @@ describe( 'DowncastHelpers', () => { expect( spy.called ).to.be.true; } ); + it( 'should convert attribute change and add a child at the same time (separate converters)', () => { + model.schema.extend( 'simpleBlock', { allowAttributes: 'someOther' } ); + downcastHelpers.attributeToAttribute( { model: 'someOther', view: 'data-other' } ); + + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.setAttribute( 'someOther', 'foo', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should convert attribute change, remove element before, and add a child at the same time (separate)', () => { + model.schema.extend( 'simpleBlock', { allowAttributes: 'someOther' } ); + downcastHelpers.attributeToAttribute( { model: 'someOther', view: 'data-other' } ); + + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes( 1 ); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.remove( modelRoot.getChild( 0 ) ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.setAttribute( 'someOther', 'foo', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); + it( 'should convert on removing a child', () => { setModelData( model, 'foobar' ); From cda018ec6d3589dd51c77b96d98a86832df89409 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Tue, 31 Jan 2023 13:38:50 +0100 Subject: [PATCH 12/25] Improve code coverage --- .../src/converters/downcast.js | 7 +- .../tests/converters/downcast.js | 71 +++++++++++++++++++ .../tablecolumnwidthscommand.js | 13 ++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-table/src/converters/downcast.js b/packages/ckeditor5-table/src/converters/downcast.js index 774e980eeff..7c6f6262112 100644 --- a/packages/ckeditor5-table/src/converters/downcast.js +++ b/packages/ckeditor5-table/src/converters/downcast.js @@ -19,10 +19,9 @@ import { toWidget, toWidgetEditable } from 'ckeditor5/src/widget'; * @param {Array.} [options.additionalSlots] Array of additional slot handlers. * @returns {Function} Element creator. */ -export function downcastTable( tableUtils, options = {} ) { +export function downcastTable( tableUtils, options ) { return ( table, { writer } ) => { const headingRows = table.getAttribute( 'headingRows' ) || 0; - const additionalSlots = options.additionalSlots || []; const tableElement = writer.createContainerElement( 'table', null, [] ); // Table head slot. @@ -50,7 +49,7 @@ export function downcastTable( tableUtils, options = {} ) { } // Dynamic slots. - for ( const { positionOffset, filter } of additionalSlots ) { + for ( const { positionOffset, filter } of options.additionalSlots ) { writer.insert( writer.createPositionAt( tableElement, positionOffset ), writer.createSlot( filter ) @@ -66,7 +65,7 @@ export function downcastTable( tableUtils, options = {} ) { return false; } - return !additionalSlots.some( ( { filter } ) => filter( element ) ); + return !options.additionalSlots.some( ( { filter } ) => filter( element ) ); } ) ] ); diff --git a/packages/ckeditor5-table/tests/converters/downcast.js b/packages/ckeditor5-table/tests/converters/downcast.js index 15463529320..c35b466e50f 100644 --- a/packages/ckeditor5-table/tests/converters/downcast.js +++ b/packages/ckeditor5-table/tests/converters/downcast.js @@ -187,6 +187,77 @@ describe( 'downcast converters', () => { ); } ); + it( 'should push table items without dedicated slot outside the table', () => { + editor.model.schema.register( 'foo', { allowIn: 'table' } ); + editor.conversion.elementToElement( { model: 'foo', view: 'foo' } ); + + editor.setData( + `
+ + + + + + + + +
0102
+
` + ); + + expect( editor.getData() ).to.equalMarkup( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
0102
' + + ' ' + + '
' + ); + } ); + + it( 'should create table with custom slot', () => { + editor.model.schema.register( 'foo', { allowIn: 'table' } ); + editor.conversion.elementToElement( { model: 'foo', view: 'foo' } ); + + editor.plugins.get( 'TableEditing' ).registerAdditionalSlot( { + filter: element => element.is( 'element', 'foo' ), + positionOffset: 0 + } ); + + editor.setData( + `
+ + + + + + + + +
0102
+
` + ); + + expect( editor.getData() ).to.equalMarkup( + '
' + + '' + + ' ' + + '' + + '' + + '' + + '' + + '' + + '' + + '
0102
' + + '
' + ); + } ); + it( 'should create table with block content', () => { setModelData( model, modelTable( [ [ '00foo', '01' ] diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js index 86f175ab980..205db4d5b27 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js @@ -104,4 +104,17 @@ describe( 'TableColumnWidthsCommand', () => { '' ); } ); + + it( 'shouldn\'t do anything if the new value is not passed and doesn\'t exists', () => { + setModelData( model, modelTable( [ + [ '11', '12' ], + [ '21', '22' ] + ] ) ); + + const modelDataBeforeExecute = getModelData( model, { withoutSelection: true } ); + + command.execute(); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelDataBeforeExecute ); + } ); } ); From bfecd2084b89917a65cf70f5c0dda5446f1c1c6f Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 31 Jan 2023 18:09:58 +0100 Subject: [PATCH 13/25] Should inject re-conversion change item just before changes applied to the re-converted element. --- .../src/conversion/downcasthelpers.ts | 17 +- .../tests/conversion/downcasthelpers.js | 184 ++++++++++++++++++ 2 files changed, 194 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts b/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts index fdbb90ce9be..345ff8a57bf 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.ts @@ -2450,21 +2450,24 @@ function createChangeReducer( model: NormalizedModelElementConfig ) { data.reconvertedElements.add( node ); const position = ModelPosition._createBefore( node ); - let idx = reducedChanges.length; + let changeIndex = reducedChanges.length; - // We need to insert remove+reinsert before any attribute change on the same node. - // This is important because otherwise we would remove node that had already modified attribute. + // We need to insert remove+reinsert before any other change on and inside the re-converted element. + // This is important because otherwise we would remove element that had already been modified by the previous change. + // Note that there could be some element removed before the re-converted element, so we must not break this behavior. for ( let i = reducedChanges.length - 1; i >= 0; i-- ) { const change = reducedChanges[ i ]; + const changePosition = change.type == 'attribute' ? change.range.start : change.position; + const positionRelation = changePosition.compareWith( position ); - if ( change.type == 'attribute' && change.range.start.isEqual( position ) ) { - idx = i; - } else { + if ( positionRelation == 'before' || change.type == 'remove' && positionRelation == 'same' ) { break; } + + changeIndex = i; } - reducedChanges.splice( idx, 0, { + reducedChanges.splice( changeIndex, 0, { type: 'remove', name: ( node as ModelElement ).name, position, diff --git a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js index df94c3f528b..8efb99c74fa 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js @@ -734,6 +734,98 @@ describe( 'DowncastHelpers', () => { expect( spy.called ).to.be.true; } ); + it( 'should reconvert before content modifications (some deeply nested node added)', () => { + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.insertText( 'abc', modelRoot.getChild( 0 ).getChild( 0 ), 'end' ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

fooabc

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should reconvert before content modifications (some deeply nested node removed)', () => { + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter ] = getNodes(); + + expectResult( '

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should reconvert before content modifications (with element added before)', () => { + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); + writer.insertElement( 'paragraph', modelRoot, 0 ); + } ); + + const [ viewAfter, paraAfter ] = getNodes( 1 ); + + expectResult( '

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( spy.called ).to.be.true; + } ); + it( 'should convert on removing a child', () => { setModelData( model, 'foobar' ); @@ -1640,6 +1732,98 @@ describe( 'DowncastHelpers', () => { expect( spy.called ).to.be.true; } ); + it( 'should reconvert before content modifications (some deeply nested node added)', () => { + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.insertText( 'abc', modelRoot.getChild( 0 ).getChild( 0 ), 'end' ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

fooabc

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should reconvert before content modifications (some deeply nested node removed)', () => { + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter ] = getNodes(); + + expectResult( '

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should reconvert before content modifications (with element added before)', () => { + setModelData( model, 'foo' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); + writer.insertElement( 'paragraph', modelRoot, 0 ); + } ); + + const [ viewAfter, paraAfter ] = getNodes( 1 ); + + expectResult( '

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( spy.called ).to.be.true; + } ); + it( 'should convert on removing a child', () => { setModelData( model, 'foobar' ); From 405b307303f34d58b30f01c51bb79d6fd23e60a0 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 1 Feb 2023 10:40:22 +0100 Subject: [PATCH 14/25] Remove no longer used fixer --- .../tablecolumnresizeediting.js | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 2c8461edae5..4e6794ef1c7 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -125,7 +125,6 @@ export default class TableColumnResizeEditing extends Plugin { this._registerPostFixer(); this._registerConverters(); this._registerResizingListeners(); - this._registerColgroupFixer(); this._registerResizerInserter(); const editor = this.editor; @@ -738,35 +737,6 @@ export default class TableColumnResizeEditing extends Plugin { }; } - /** - * Inserts the `` element if it is missing in the view table (e.g. after table insertion into table). - * - * @private - */ - _registerColgroupFixer() { - const editor = this.editor; - - this.listenTo( editor.editing.view.document, 'layoutChanged', () => { - const viewTable = editor.editing.view.document.selection.getFirstPosition().getAncestors().reverse().find( - viewElement => viewElement.name === 'table' - ); - const viewTableContainsColgroup = viewTable && Array.from( viewTable.getChildren() ).find( - viewElement => viewElement.is( 'element', 'colgroup' ) - ); - const modelTable = editor.model.document.selection.getFirstPosition().findAncestor( 'table' ); - - if ( !modelTable ) { - return; - } - - const hasColumnGroup = Array.from( modelTable.getChildren() ).find( element => element.is( 'element', 'tableColumnGroup' ) ); - - if ( hasColumnGroup && viewTable && !viewTableContainsColgroup ) { - editor.editing.reconvertItem( modelTable ); - } - }, { priority: 'low' } ); - } - /** * Registers a listener ensuring that each resizable cell have a resizer handle. * From 60784800f3a4a934943a5f3ac62568a15eb2c58a Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 1 Feb 2023 11:25:39 +0100 Subject: [PATCH 15/25] Add helpers for accessing table columns information --- .../src/tablecolumnresize/converters.js | 2 +- .../tablecolumnresizeediting.js | 28 +++++----- .../tablecolumnwidthscommand.js | 9 ++-- .../src/tablecolumnresize/utils.js | 53 +++++++++++++++++-- .../tests/tablecolumnresize/utils.js | 36 +++++++------ 5 files changed, 86 insertions(+), 42 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index 54793c709bf..f2840253384 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -52,7 +52,7 @@ export function upcastColgroupElement( tableUtilsPlugin ) { } ); if ( columnWidths.includes( 'auto' ) ) { - columnWidths = normalizeColumnWidths( columnWidths ).map( width => width + '%' ); + columnWidths = normalizeColumnWidths( columnWidths ); } const colGroupElement = conversionApi.writer.createElement( 'tableColumnGroup' ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 4e6794ef1c7..33592fb70e0 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -35,7 +35,10 @@ import { getTableWidthInPixels, normalizeColumnWidths, toPrecision, - getDomCellOuterWidth + getDomCellOuterWidth, + getColumnWidths, + getTableColumns, + getTableColumnGroup } from './utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS } from './constants'; @@ -202,12 +205,9 @@ export default class TableColumnResizeEditing extends Plugin { let changed = false; for ( const table of getChangedResizedTables( model ) ) { - const tableColumnGroup = Array - .from( table.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); - - const columns = Array.from( tableColumnGroup.getChildren() ); - const columnWidths = columns.map( element => Number( element.getAttribute( 'columnWidth' ).replace( '%', '' ) ) ); + const tableColumnGroup = getTableColumnGroup( table ); + const columns = getTableColumns( tableColumnGroup ); + const columnWidths = getColumnWidths( tableColumnGroup ); // Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. const normalizedWidths = normalizeColumnWidths( columnWidths ); @@ -221,17 +221,17 @@ export default class TableColumnResizeEditing extends Plugin { for ( let i = 0; i < Math.max( normalizedWidths.length, columns.length ); i++ ) { const column = columns[ i ]; - const width = normalizedWidths[ i ]; + const columnWidth = normalizedWidths[ i ]; - if ( !width ) { + if ( !columnWidth ) { // Number of `` elements exceeds actual number of columns writer.remove( column ); } else if ( !column ) { // There is fewer `` elements than actual columns - writer.appendElement( 'tableColumn', { columnWidth: `${ width }%` }, tableColumnGroup ); + writer.appendElement( 'tableColumn', { columnWidth }, tableColumnGroup ); } else { // Update column width - writer.setAttribute( 'columnWidth', `${ width }%`, column ); + writer.setAttribute( 'columnWidth', columnWidth, column ); } } @@ -595,12 +595,10 @@ export default class TableColumnResizeEditing extends Plugin { const editor = this.editor; const editingView = editor.editing.view; - const tableColumnGroup = Array - .from( modelTable.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); + const tableColumnGroup = getTableColumnGroup( modelTable ); const columnWidthsAttributeOld = tableColumnGroup ? - Array.from( tableColumnGroup.getChildren() ).map( element => element.getAttribute( 'columnWidth' ) ) : + getColumnWidths( tableColumnGroup ) : null; const columnWidthsAttributeNew = Array diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js index c46810d5689..159587643e0 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js @@ -8,7 +8,7 @@ */ import { Command } from 'ckeditor5/src/core'; -import { normalizeColumnWidths } from './utils'; +import { getTableColumnGroup, normalizeColumnWidths } from './utils'; /** * @extends module:table/tableproperties/commands/tablepropertycommand~TablePropertyCommand @@ -38,9 +38,7 @@ export default class TableColumnWidthsCommand extends Command { } = options; model.change( writer => { - const tableColumnGroup = Array - .from( table.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); + const tableColumnGroup = getTableColumnGroup( table ); if ( !columnWidths && !tableColumnGroup ) { return; @@ -50,8 +48,7 @@ export default class TableColumnWidthsCommand extends Command { return writer.remove( tableColumnGroup ); } - let widths = columnWidths.map( widths => Number( widths.replace( '%', '' ) ) ); - widths = normalizeColumnWidths( widths ).map( width => `${ width }%` ); + const widths = normalizeColumnWidths( columnWidths ); if ( !tableColumnGroup ) { const colGroupElement = writer.createElement( 'tableColumnGroup' ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index 171038b6035..247ef6bf9d6 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -230,10 +230,20 @@ export function sumArray( array ) { * * Currently, only widths provided as percentage values are supported. * - * @param {Array.} columnWidths An array of column widths. - * @returns {Array.} An array of column widths guaranteed to sum up to 100%. + * @param {Array.} columnWidths An array of column widths. + * @returns {Array.} An array of column widths guaranteed to sum up to 100%. */ export function normalizeColumnWidths( columnWidths ) { + columnWidths = columnWidths.map( width => { + // Possible values are 'auto', number or string ending with '%' + if ( width === 'auto' || !isNaN( width ) ) { + // Leave 'auto' and number widths unchanged + return width; + } + + return Number( width.replace( '%', '' ) ); + } ); + columnWidths = calculateMissingColumnWidths( columnWidths ); const totalWidth = sumArray( columnWidths ); @@ -256,7 +266,8 @@ export function normalizeColumnWidths( columnWidths ) { const totalWidth = sumArray( columnWidths ); return toPrecision( columnWidth + 100 - totalWidth ); - } ); + } ) + .map( width => width + '%' ); } // Initializes the column widths by parsing the attribute value and calculating the uninitialized column widths. The special value 'auto' @@ -311,3 +322,39 @@ export function getDomCellOuterWidth( domCell ) { parseFloat( styles.borderWidth ); } } + +/** + * Returns a 'tableColumnGroup' element from the 'table'. + * + * @param {module:engine/model/element~Element} cell A 'table' or 'tableColumnGroup' element. + * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. + */ +export function getTableColumnGroup( element ) { + if ( element.is( 'element', 'tableColumnGroup' ) ) { + return element; + } + + return Array + .from( element.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); +} + +/** + * Returns an array of 'tableColumn' elements. + * + * @param {module:engine/model/element~Element} cell A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of 'tableColumn' elements. + */ +export function getTableColumns( element ) { + return Array.from( getTableColumnGroup( element ).getChildren() ); +} + +/** + * Returns an array of table column widths. + * + * @param {module:engine/model/element~Element} cell A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of table column widths. + */ +export function getColumnWidths( element ) { + return getTableColumns( element ).map( column => column.getAttribute( 'columnWidth' ) ); +} diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index 0ad182bc224..b88a79f51c8 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -575,33 +575,35 @@ describe( 'TableColumnResize utils', () => { describe( 'normalizeColumnWidths()', () => { it( 'should not change the widths of the columns if they sum up to 100%', () => { - expect( normalizeColumnWidths( [ '25%', '25%', '25%', '25%' ] ) ).to.deep.equal( [ 25, 25, 25, 25 ] ); - expect( normalizeColumnWidths( [ '10%', '20%', '30%', '40%' ] ) ).to.deep.equal( [ 10, 20, 30, 40 ] ); - expect( normalizeColumnWidths( [ '10.32%', '20.12%', '30.87%', '38.69%' ] ) ).to.deep.equal( [ 10.32, 20.12, 30.87, 38.69 ] ); - expect( normalizeColumnWidths( [ '100%' ] ) ).to.deep.equal( [ 100 ] ); + [ + [ '25%', '25%', '25%', '25%' ], + [ '10%', '20%', '30%', '40%' ], + [ '10.32%', '20.12%', '30.87%', '38.69%' ], + [ '100%' ] + ].forEach( width => expect( normalizeColumnWidths( width ) ).to.deep.equal( width ) ); } ); it( 'should extend uninitialized columns equally if the free space per column is wider than the minimum column width', () => { - expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', 'auto' ] ) ).to.deep.equal( [ 25, 25, 25, 25 ] ); - expect( normalizeColumnWidths( [ 'auto', '25%', 'auto', '25%' ] ) ).to.deep.equal( [ 25, 25, 25, 25 ] ); - expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', '40%' ] ) ).to.deep.equal( [ 20, 20, 20, 40 ] ); - expect( normalizeColumnWidths( [ 'auto', '45%', '45%', 'auto' ] ) ).to.deep.equal( [ 5, 45, 45, 5 ] ); - expect( normalizeColumnWidths( [ 'auto' ] ) ).to.deep.equal( [ 100 ] ); + expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', 'auto' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); + expect( normalizeColumnWidths( [ 'auto', '25%', 'auto', '25%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); + expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', '40%' ] ) ).to.deep.equal( [ '20%', '20%', '20%', '40%' ] ); + expect( normalizeColumnWidths( [ 'auto', '45%', '45%', 'auto' ] ) ).to.deep.equal( [ '5%', '45%', '45%', '55%' ] ); + expect( normalizeColumnWidths( [ 'auto' ] ) ).to.deep.equal( [ '100%' ] ); } ); it( 'should set the minimum column width for uninitialized columns if there is not enough free space per column', () => { - expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', '90%' ] ) ).to.deep.equal( [ 4.76, 4.76, 4.76, 85.72 ] ); - expect( normalizeColumnWidths( [ 'auto', '50%', 'auto', '50%' ] ) ).to.deep.equal( [ 4.55, 45.45, 4.55, 45.45 ] ); - expect( normalizeColumnWidths( [ 'auto', '50%', '50%', '50%' ] ) ).to.deep.equal( [ 3.23, 32.26, 32.26, 32.25 ] ); + expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', '90%' ] ) ).to.deep.equal( [ '4.76%', '4.76%', '4.76%', '85.72%' ] ); + expect( normalizeColumnWidths( [ 'auto', '50%', 'auto', '50%' ] ) ).to.deep.equal( [ '4.55%', '45.45%', '4.55%', '45.45%' ] ); + expect( normalizeColumnWidths( [ 'auto', '50%', '50%', '50%' ] ) ).to.deep.equal( [ '3.23%', '32.26%', '32.26%', '32.25%' ] ); } ); it( 'should proportionally align all the column widths if their sum is not exactly 100%', () => { - expect( normalizeColumnWidths( [ '10%', '20%', '30%', '50%' ] ) ).to.deep.equal( [ 9.09, 18.18, 27.27, 45.46 ] ); - expect( normalizeColumnWidths( [ '10%', '10%', '10%', '10%' ] ) ).to.deep.equal( [ 25, 25, 25, 25 ] ); - expect( normalizeColumnWidths( [ '100%', '100%', '100%', '100%' ] ) ).to.deep.equal( [ 25, 25, 25, 25 ] ); - expect( normalizeColumnWidths( [ '1%', '2%', '3%', '4%' ] ) ).to.deep.equal( [ 10, 20, 30, 40 ] ); + expect( normalizeColumnWidths( [ '10%', '20%', '30%', '50%' ] ) ).to.deep.equal( [ '9.09%', '18.18%', '27.27%', '45.46%' ] ); + expect( normalizeColumnWidths( [ '10%', '10%', '10%', '10%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); + expect( normalizeColumnWidths( [ '100%', '100%', '100%', '100%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); + expect( normalizeColumnWidths( [ '1%', '2%', '3%', '4%' ] ) ).to.deep.equal( [ '10%', '20%', '30%', '40%' ] ); expect( normalizeColumnWidths( [ '12.33%', '17.4%', '21.49%', '33.52%', '26.6%', '10.43%' ] ) ) - .to.deep.equal( [ 10.13, 14.29, 17.65, 27.53, 21.84, 8.56 ] ); + .to.deep.equal( [ '10.13%', '14.29%', '17.65%', '27.53%', '21.84%', '8.56%' ] ); } ); } ); From 4263b942085e1182956d0b61a4828d67fa40de9e Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 1 Feb 2023 12:41:46 +0100 Subject: [PATCH 16/25] Merge two TableColumnResize commands and fix tests --- .../tablecolumnresizeediting.js | 36 +++---- .../tablewidthresizecommand.js | 60 ------------ ...widthscommand.js => tablewidthscommand.js} | 12 ++- .../src/tablecolumnresize/utils.js | 36 +++---- .../tablewidthresizecommand.js | 98 ------------------- ...widthscommand.js => tablewidthscommand.js} | 54 +++++++++- .../tests/tablecolumnresize/utils.js | 2 +- 7 files changed, 93 insertions(+), 205 deletions(-) delete mode 100644 packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js rename packages/ckeditor5-table/src/tablecolumnresize/{tablecolumnwidthscommand.js => tablewidthscommand.js} (82%) delete mode 100644 packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js rename packages/ckeditor5-table/tests/tablecolumnresize/{tablecolumnwidthscommand.js => tablewidthscommand.js} (61%) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 33592fb70e0..e94dfd326d2 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -16,8 +16,7 @@ import TableEditing from '../tableediting'; import TableUtils from '../tableutils'; import TableWalker from '../tablewalker'; -import TableWidthResizeCommand from './tablewidthresizecommand'; -import TableColumnWidthsCommand from './tablecolumnwidthscommand'; +import TableWidthsCommand from './tablewidthscommand'; import { upcastColgroupElement, @@ -138,8 +137,9 @@ export default class TableColumnResizeEditing extends Plugin { positionOffset: 0 } ); - editor.commands.add( 'resizeTableWidth', new TableWidthResizeCommand( editor ) ); - editor.commands.add( 'resizeColumnWidths', new TableColumnWidthsCommand( editor ) ); + const tableWidthsCommand = new TableWidthsCommand( editor ); + editor.commands.add( 'resizeTableWidth', tableWidthsCommand ); + editor.commands.add( 'resizeColumnWidths', tableWidthsCommand ); const resizeTableWidthCommand = editor.commands.get( 'resizeTableWidth' ); const resizeColumnWidthsCommand = editor.commands.get( 'resizeColumnWidths' ); @@ -210,10 +210,10 @@ export default class TableColumnResizeEditing extends Plugin { const columnWidths = getColumnWidths( tableColumnGroup ); // Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. - const normalizedWidths = normalizeColumnWidths( columnWidths ); + let normalizedWidths = normalizeColumnWidths( columnWidths ); // If the number of columns has changed, then we need to adjust the widths of the affected columns. - adjustColumnWidths( normalizedWidths, table, this ); + normalizedWidths = adjustColumnWidths( normalizedWidths, table, this ); if ( isEqual( columnWidths, normalizedWidths ) ) { continue; @@ -252,9 +252,11 @@ export default class TableColumnResizeEditing extends Plugin { const columnsCountDelta = newTableColumnsCount - columnWidths.length; if ( columnsCountDelta === 0 ) { - return; + return columnWidths; } + columnWidths = columnWidths.map( width => Number( width.replace( '%', '' ) ) ); + // Collect all cells that are affected by the change. const cellSet = getAffectedCells( plugin.editor.model.document.differ, table ); @@ -283,6 +285,8 @@ export default class TableColumnResizeEditing extends Plugin { columnWidths[ currentColumnIndex ] += sumArray( removedColumnWidths ); } } + + return columnWidths.map( width => width + '%' ); } // Returns a set of cells that have been changed in a given table. @@ -614,20 +618,10 @@ export default class TableColumnResizeEditing extends Plugin { if ( isColumnWidthsAttributeChanged || isTableWidthAttributeChanged ) { if ( this._isResizingAllowed ) { - editor.model.change( () => { - if ( isTableWidthAttributeChanged ) { - editor.execute( 'resizeTableWidth', { - table: modelTable, - tableWidth: `${ toPrecision( tableWidthAttributeNew ) }%` - } ); - } - - if ( isColumnWidthsAttributeChanged ) { - editor.execute( 'resizeColumnWidths', { - columnWidths: columnWidthsAttributeNew, - table: modelTable - } ); - } + editor.execute( 'resizeTableWidth', { + table: modelTable, + tableWidth: `${ toPrecision( tableWidthAttributeNew ) }%`, + columnWidths: columnWidthsAttributeNew } ); } else { // In read-only mode revert all changes in the editing view. The model is not touched so it does not need to be restored. diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js deleted file mode 100644 index ab0ad72f355..00000000000 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthresizecommand.js +++ /dev/null @@ -1,60 +0,0 @@ -/** - * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. - * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license - */ - -/** - * @module table/tablecolumnresize/tablewidthresizecommand - */ - -import TablePropertyCommand from '../tableproperties/commands/tablepropertycommand'; - -/** - * @extends module:table/tableproperties/commands/tablepropertycommand~TablePropertyCommand - */ -export default class TableWidthResizeCommand extends TablePropertyCommand { - /** - * Creates a new `TableWidthResizeCommand` instance. - * - * @param {module:core/editor/editor~Editor} editor An editor in which this command will be used. - * @param {String} defaultValue The default value of the attribute. - */ - constructor( editor, defaultValue ) { - // We create a custom command instead of using the existing `TableWidthCommand` - // as we also need to change the `columnWidths` property and running both commands - // separately would make the integration with Track Changes feature more troublesome. - super( editor, 'tableWidth', defaultValue ); - } - - /** - * @inheritDoc - */ - refresh() { - // The command is always enabled as it doesn't care about the actual selection - table can be resized - // even if the selection is elsewhere. - this.isEnabled = true; - } - - /** - * Changes the `tableWidth` and `columnWidths` attribute values for the given or currently selected table. - * - * @param {Object} options - * @param {String} [options.tableWidth] The new table width. If skipped, the model attribute will be removed. - * @param {module:engine/model/element~Element} [options.table] The table that is affected by the resize. - */ - execute( options = {} ) { - const { model } = this.editor; - const { - table = model.document.selection.getSelectedElement(), - tableWidth - } = options; - - model.change( writer => { - if ( tableWidth ) { - return writer.setAttribute( this.attributeName, tableWidth, table ); - } - - writer.removeAttribute( this.attributeName, table ); - } ); - } -} diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js similarity index 82% rename from packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js rename to packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js index 159587643e0..94fc225d025 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js @@ -24,20 +24,28 @@ export default class TableColumnWidthsCommand extends Command { } /** - * Changes the `columnWidth` attribute values for the columns of the given table. + * Updated the `tableWidth` attribute of the table and the `columnWidth` attribute of the columns of that table. * * @param {Object} options * @param {Array.} [options.columnWidths] New value of the `columnWidths` attribute. + * @param {String} [options.tableWidth] The new table width. If skipped, the model attribute will be removed. * @param {module:engine/model/element~Element} [options.table] The table that is having the columns resized. */ execute( options = {} ) { const { model } = this.editor; const { table = model.document.selection.getSelectedElement(), - columnWidths + columnWidths, + tableWidth } = options; model.change( writer => { + if ( tableWidth ) { + writer.setAttribute( 'tableWidth', tableWidth, table ); + } else { + writer.removeAttribute( 'tableWidth', table ); + } + const tableColumnGroup = getTableColumnGroup( table ); if ( !columnWidths && !tableColumnGroup ) { diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index 247ef6bf9d6..96101e93a73 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -247,27 +247,27 @@ export function normalizeColumnWidths( columnWidths ) { columnWidths = calculateMissingColumnWidths( columnWidths ); const totalWidth = sumArray( columnWidths ); - if ( totalWidth === 100 ) { - return columnWidths; - } + if ( totalWidth !== 100 ) { + columnWidths = columnWidths + // Adjust all the columns proportionally. + .map( columnWidth => toPrecision( columnWidth * 100 / totalWidth ) ) + // Due to rounding of numbers it may happen that the sum of the widths of all columns will not be exactly 100%. + // Therefore, the width of the last column is explicitly adjusted (narrowed or expanded), since all the columns + // have been proportionally changed already. + .map( ( columnWidth, columnIndex, columnWidths ) => { + const isLastColumn = columnIndex === columnWidths.length - 1; + + if ( !isLastColumn ) { + return columnWidth; + } - return columnWidths - // Adjust all the columns proportionally. - .map( columnWidth => toPrecision( columnWidth * 100 / totalWidth ) ) - // Due to rounding of numbers it may happen that the sum of the widths of all columns will not be exactly 100%. Therefore, the width - // of the last column is explicitly adjusted (narrowed or expanded), since all the columns have been proportionally changed already. - .map( ( columnWidth, columnIndex, columnWidths ) => { - const isLastColumn = columnIndex === columnWidths.length - 1; - - if ( !isLastColumn ) { - return columnWidth; - } + const totalWidth = sumArray( columnWidths ); - const totalWidth = sumArray( columnWidths ); + return toPrecision( columnWidth + 100 - totalWidth ); + } ); + } - return toPrecision( columnWidth + 100 - totalWidth ); - } ) - .map( width => width + '%' ); + return columnWidths.map( width => width + '%' ); } // Initializes the column widths by parsing the attribute value and calculating the uninitialized column widths. The special value 'auto' diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js deleted file mode 100644 index 9c79fd24751..00000000000 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthresizecommand.js +++ /dev/null @@ -1,98 +0,0 @@ -/** - * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. - * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license - */ - -/* global document */ - -import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; -import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; -import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; - -import TableWidthResizeCommand from '../../src/tablecolumnresize/tablewidthresizecommand'; -import TableColumnResizeEditing from '../../src/tablecolumnresize/tablecolumnresizeediting'; -import TableColumnResize from '../../src/tablecolumnresize'; -import Table from '../../src/table'; -import { modelTable } from '../_utils/utils'; - -describe( 'TableWidthResizeCommand', () => { - let model, editor, editorElement, command; - - beforeEach( async () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - - return ClassicTestEditor.create( editorElement, { - plugins: [ Table, TableColumnResize, TableColumnResizeEditing, Paragraph ] - } ).then( newEditor => { - editor = newEditor; - model = editor.model; - command = new TableWidthResizeCommand( editor ); - } ); - } ); - - afterEach( async () => { - if ( editorElement ) { - editorElement.remove(); - } - - if ( editor ) { - await editor.destroy(); - } - } ); - - it( 'should work on the currently selected table if it was not passed to execute()', () => { - const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = {}; - setModelData( model, modelTable( data, attributesBefore ) ); - - command.execute( { tableWidth: '40%' } ); - - const attributesAfter = { tableWidth: '40%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); - } ); - - it( 'should remove the attributes if new value was not passed', () => { - const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%' }; - setModelData( model, modelTable( data, attributesBefore ) ); - - command.execute(); - - const attributesAfter = {}; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); - } ); - - it( 'should work when only tableWidth is provided', () => { - const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%' }; - setModelData( model, modelTable( data, attributesBefore ) ); - - command.execute( { tableWidth: '40%' } ); - - const attributesAfter = { tableWidth: '40%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); - } ); - - it( 'should add attributes when they are provided, but were not present before', () => { - const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = {}; - setModelData( model, modelTable( data, attributesBefore ) ); - - command.execute( { tableWidth: '40%' } ); - - const attributesAfter = { tableWidth: '40%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); - } ); - - it( 'should change attributes when they were present before', () => { - const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%' }; - setModelData( model, modelTable( data, attributesBefore ) ); - - command.execute( { tableWidth: '30%' } ); - - const attributesAfter = { tableWidth: '30%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); - } ); -} ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js similarity index 61% rename from packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js rename to packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js index 205db4d5b27..1113455401c 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnwidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js @@ -9,13 +9,13 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import TableColumnWidthsCommand from '../../src/tablecolumnresize/tablecolumnwidthscommand'; +import TableWidthsCommand from '../../src/tablecolumnresize/tablewidthscommand'; import TableColumnResizeEditing from '../../src/tablecolumnresize/tablecolumnresizeediting'; import TableColumnResize from '../../src/tablecolumnresize'; import Table from '../../src/table'; import { modelTable } from '../_utils/utils'; -describe( 'TableColumnWidthsCommand', () => { +describe( 'TableWidthsCommand', () => { let model, editor, editorElement, command; beforeEach( async () => { @@ -27,7 +27,7 @@ describe( 'TableColumnWidthsCommand', () => { } ).then( newEditor => { editor = newEditor; model = editor.model; - command = new TableColumnWidthsCommand( editor ); + command = new TableWidthsCommand( editor ); } ); } ); @@ -47,10 +47,10 @@ describe( 'TableColumnWidthsCommand', () => { [ '21', '22' ] ] ) ); - command.execute( { columnWidths: [ '40%', '%60' ] } ); + command.execute( { columnWidths: [ '40%', '%60' ], tableWidth: '40%' } ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + + '
' + '' + '' + '11' + @@ -117,4 +117,48 @@ describe( 'TableColumnWidthsCommand', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelDataBeforeExecute ); } ); + + it( 'should remove the attributes if new value was not passed', () => { + const data = [ [ '11', '12' ], [ '21', '22' ] ]; + const attributesBefore = { tableWidth: '40%' }; + setModelData( model, modelTable( data, attributesBefore ) ); + + command.execute(); + + const attributesAfter = {}; + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + } ); + + it( 'should work when only tableWidth is provided', () => { + const data = [ [ '11', '12' ], [ '21', '22' ] ]; + const attributesBefore = { tableWidth: '40%' }; + setModelData( model, modelTable( data, attributesBefore ) ); + + command.execute( { tableWidth: '40%' } ); + + const attributesAfter = { tableWidth: '40%' }; + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + } ); + + it( 'should add attributes when they are provided, but were not present before', () => { + const data = [ [ '11', '12' ], [ '21', '22' ] ]; + const attributesBefore = {}; + setModelData( model, modelTable( data, attributesBefore ) ); + + command.execute( { tableWidth: '40%' } ); + + const attributesAfter = { tableWidth: '40%' }; + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + } ); + + it( 'should change attributes when they were present before', () => { + const data = [ [ '11', '12' ], [ '21', '22' ] ]; + const attributesBefore = { tableWidth: '40%' }; + setModelData( model, modelTable( data, attributesBefore ) ); + + command.execute( { tableWidth: '30%' } ); + + const attributesAfter = { tableWidth: '30%' }; + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + } ); } ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index b88a79f51c8..44dfec6f90b 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -587,7 +587,7 @@ describe( 'TableColumnResize utils', () => { expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', 'auto' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); expect( normalizeColumnWidths( [ 'auto', '25%', 'auto', '25%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', '40%' ] ) ).to.deep.equal( [ '20%', '20%', '20%', '40%' ] ); - expect( normalizeColumnWidths( [ 'auto', '45%', '45%', 'auto' ] ) ).to.deep.equal( [ '5%', '45%', '45%', '55%' ] ); + expect( normalizeColumnWidths( [ 'auto', '45%', '45%', 'auto' ] ) ).to.deep.equal( [ '5%', '45%', '45%', '5%' ] ); expect( normalizeColumnWidths( [ 'auto' ] ) ).to.deep.equal( [ '100%' ] ); } ); From bfa4a7f565c8f724b85b419c0a036d2df2980583 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 1 Feb 2023 13:34:05 +0100 Subject: [PATCH 17/25] Add tests to cover new table helpers --- .../tablecolumnresize/tablewidthscommand.js | 36 +++--- .../tests/tablecolumnresize/utils.js | 113 +++++++++++++++++- 2 files changed, 130 insertions(+), 19 deletions(-) diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js index 1113455401c..074f0f9382c 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js @@ -79,7 +79,7 @@ describe( 'TableWidthsCommand', () => { setModelData( model, modelTable( [ [ '11', '12' ], [ '21', '22' ] - ], { columnWidths: '40%,60%' } ) ); + ], { columnWidths: '40%,60%', tableWidth: '40%' } ) ); command.execute(); @@ -118,26 +118,26 @@ describe( 'TableWidthsCommand', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelDataBeforeExecute ); } ); - it( 'should remove the attributes if new value was not passed', () => { + it( 'should work when only tableWidth is provided', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%' }; + const attributesBefore = { columnWidths: '40%,60%', tableWidth: '40%' }; setModelData( model, modelTable( data, attributesBefore ) ); - command.execute(); + command.execute( { tableWidth: '50%' } ); - const attributesAfter = {}; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + const expectedModel = modelTable( data, { tableWidth: '50%' } ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModel ); } ); - it( 'should work when only tableWidth is provided', () => { + it( 'should work when only columnWidths is provided', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%' }; + const attributesBefore = { columnWidths: '40%,60%', tableWidth: '40%' }; setModelData( model, modelTable( data, attributesBefore ) ); - command.execute( { tableWidth: '40%' } ); + command.execute( { columnWidths: [ '30%', '70%' ] } ); - const attributesAfter = { tableWidth: '40%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + const expectedModel = modelTable( data, { columnWidths: '30%,70%' } ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModel ); } ); it( 'should add attributes when they are provided, but were not present before', () => { @@ -145,20 +145,20 @@ describe( 'TableWidthsCommand', () => { const attributesBefore = {}; setModelData( model, modelTable( data, attributesBefore ) ); - command.execute( { tableWidth: '40%' } ); + command.execute( { columnWidths: [ '30%', '70%' ], tableWidth: '40%' } ); - const attributesAfter = { tableWidth: '40%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + const expectedModel = modelTable( data, { columnWidths: '30%,70%', tableWidth: '40%' } ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModel ); } ); it( 'should change attributes when they were present before', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; - const attributesBefore = { tableWidth: '40%' }; + const attributesBefore = { columnWidths: '40%,60%', tableWidth: '40%' }; setModelData( model, modelTable( data, attributesBefore ) ); - command.execute( { tableWidth: '30%' } ); + command.execute( { columnWidths: [ '30%', '70%' ], tableWidth: '50%' } ); - const attributesAfter = { tableWidth: '30%' }; - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelTable( data, attributesAfter ) ); + const expectedModel = modelTable( data, { columnWidths: '30%,70%', tableWidth: '50%' } ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModel ); } ); } ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index 44dfec6f90b..2f473a6ad58 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -30,7 +30,10 @@ import { getTableWidthInPixels, getColumnMinWidthAsPercentage, getElementWidthInPixels, - getDomCellOuterWidth + getDomCellOuterWidth, + getTableColumnGroup, + getTableColumns, + getColumnWidths } from '../../src/tablecolumnresize/utils'; /* globals window, document */ @@ -583,6 +586,10 @@ describe( 'TableColumnResize utils', () => { ].forEach( width => expect( normalizeColumnWidths( width ) ).to.deep.equal( width ) ); } ); + it( 'should handle column widths of different formats', () => { + expect( normalizeColumnWidths( [ 'auto', 25, 'auto', '25%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); + } ); + it( 'should extend uninitialized columns equally if the free space per column is wider than the minimum column width', () => { expect( normalizeColumnWidths( [ 'auto', 'auto', 'auto', 'auto' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); expect( normalizeColumnWidths( [ 'auto', '25%', 'auto', '25%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); @@ -714,6 +721,110 @@ describe( 'TableColumnResize utils', () => { expect( result ).to.not.equal( 0 ); } ); } ); + + describe( 'getTableColumnGroup()', () => { + let model, editor, editorElement; + + beforeEach( async () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + editor = await ClassicEditor.create( editorElement, { + plugins: [ Table, TableColumnResize, Paragraph ] + } ); + + model = editor.model; + } ); + + afterEach( async () => { + if ( editorElement ) { + editorElement.remove(); + } + + if ( editor ) { + await editor.destroy(); + } + } ); + + it( 'should return \'tableColumnGroup\' when it exists', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( getTableColumnGroup( model.document.getRoot().getChild( 0 ) ) ).to.not.be.undefined; + } ); + + it( 'should not return anything if \'tableColumnGroup\' doesn\'t exists', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ] ) ); + + expect( getTableColumnGroup( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; + } ); + + it( 'should return the same \'tableColumnGroup\' element if it was passed as an argument', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + const tableColumnGroup = model.document.getRoot().getChild( 0 ).getChild( 1 ); + + expect( getTableColumnGroup( tableColumnGroup ) ).to.equal( tableColumnGroup ); + } ); + } ); + + describe( 'getTableColumns()', () => { + let model, editor, editorElement; + + beforeEach( async () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + editor = await ClassicEditor.create( editorElement, { + plugins: [ Table, TableColumnResize, Paragraph ] + } ); + + model = editor.model; + } ); + + afterEach( async () => { + if ( editorElement ) { + editorElement.remove(); + } + + if ( editor ) { + await editor.destroy(); + } + } ); + + it( 'should return \'tableColumn\' array when there are columns', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( getTableColumns( model.document.getRoot().getChild( 0 ) ) ).to.have.length( 2 ); + } ); + } ); + + describe( 'getColumnWidths()', () => { + let model, editor, editorElement; + + beforeEach( async () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + editor = await ClassicEditor.create( editorElement, { + plugins: [ Table, TableColumnResize, Paragraph ] + } ); + + model = editor.model; + } ); + + afterEach( async () => { + if ( editorElement ) { + editorElement.remove(); + } + + if ( editor ) { + await editor.destroy(); + } + } ); + + it( 'should return \'tableColumnGroup\' count when there are columns', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( getColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ '50%', '50%' ] ); + } ); + } ); } ); function createTable( rows, cols ) { From f0c137c5d476508d8bcc36b4d12d34c80390159e Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 1 Feb 2023 17:27:33 +0100 Subject: [PATCH 18/25] Move TableColumnResize helpers into TableUtils --- .../tablecolumnresizeediting.js | 15 +-- .../tablecolumnresize/tablewidthscommand.js | 17 ++- .../src/tablecolumnresize/utils.js | 36 ------ packages/ckeditor5-table/src/tableutils.js | 38 ++++++ .../tablecolumnresize/tablewidthscommand.js | 11 ++ .../tests/tablecolumnresize/utils.js | 109 +----------------- packages/ckeditor5-table/tests/tableutils.js | 41 ++++++- 7 files changed, 108 insertions(+), 159 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index e94dfd326d2..1d7ac8a3e6e 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -34,10 +34,7 @@ import { getTableWidthInPixels, normalizeColumnWidths, toPrecision, - getDomCellOuterWidth, - getColumnWidths, - getTableColumns, - getTableColumnGroup + getDomCellOuterWidth } from './utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS } from './constants'; @@ -205,9 +202,9 @@ export default class TableColumnResizeEditing extends Plugin { let changed = false; for ( const table of getChangedResizedTables( model ) ) { - const tableColumnGroup = getTableColumnGroup( table ); - const columns = getTableColumns( tableColumnGroup ); - const columnWidths = getColumnWidths( tableColumnGroup ); + const tableColumnGroup = this._tableUtilsPlugin.getColumnGroupElement( table ); + const columns = this._tableUtilsPlugin.getTableColumnElements( tableColumnGroup ); + const columnWidths = this._tableUtilsPlugin.getTableColumnsWidths( tableColumnGroup ); // Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. let normalizedWidths = normalizeColumnWidths( columnWidths ); @@ -599,10 +596,10 @@ export default class TableColumnResizeEditing extends Plugin { const editor = this.editor; const editingView = editor.editing.view; - const tableColumnGroup = getTableColumnGroup( modelTable ); + const tableColumnGroup = this._tableUtilsPlugin.getColumnGroupElement( modelTable ); const columnWidthsAttributeOld = tableColumnGroup ? - getColumnWidths( tableColumnGroup ) : + this._tableUtilsPlugin.getTableColumnsWidths( tableColumnGroup ) : null; const columnWidthsAttributeNew = Array diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js index 94fc225d025..0bdff4ff8e2 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js @@ -8,7 +8,7 @@ */ import { Command } from 'ckeditor5/src/core'; -import { getTableColumnGroup, normalizeColumnWidths } from './utils'; +import { normalizeColumnWidths } from './utils'; /** * @extends module:table/tableproperties/commands/tablepropertycommand~TablePropertyCommand @@ -27,18 +27,25 @@ export default class TableColumnWidthsCommand extends Command { * Updated the `tableWidth` attribute of the table and the `columnWidth` attribute of the columns of that table. * * @param {Object} options - * @param {Array.} [options.columnWidths] New value of the `columnWidths` attribute. + * @param {Array.|String} [options.columnWidths] New value of the `columnWidths` attribute. * @param {String} [options.tableWidth] The new table width. If skipped, the model attribute will be removed. * @param {module:engine/model/element~Element} [options.table] The table that is having the columns resized. */ execute( options = {} ) { - const { model } = this.editor; - const { + const { model, plugins } = this.editor; + let { table = model.document.selection.getSelectedElement(), columnWidths, tableWidth } = options; + if ( columnWidths ) { + // For backwards compatibility, columnWidths might be an array or a string of comma-separated values + columnWidths = Array.isArray( columnWidths ) ? + columnWidths : + columnWidths.split( ',' ); + } + model.change( writer => { if ( tableWidth ) { writer.setAttribute( 'tableWidth', tableWidth, table ); @@ -46,7 +53,7 @@ export default class TableColumnWidthsCommand extends Command { writer.removeAttribute( 'tableWidth', table ); } - const tableColumnGroup = getTableColumnGroup( table ); + const tableColumnGroup = plugins.get( 'TableUtils' ).getColumnGroupElement( table ); if ( !columnWidths && !tableColumnGroup ) { return; diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index 96101e93a73..e599b732a1c 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -322,39 +322,3 @@ export function getDomCellOuterWidth( domCell ) { parseFloat( styles.borderWidth ); } } - -/** - * Returns a 'tableColumnGroup' element from the 'table'. - * - * @param {module:engine/model/element~Element} cell A 'table' or 'tableColumnGroup' element. - * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. - */ -export function getTableColumnGroup( element ) { - if ( element.is( 'element', 'tableColumnGroup' ) ) { - return element; - } - - return Array - .from( element.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); -} - -/** - * Returns an array of 'tableColumn' elements. - * - * @param {module:engine/model/element~Element} cell A 'table' or 'tableColumnGroup' element. - * @returns {Array} An array of 'tableColumn' elements. - */ -export function getTableColumns( element ) { - return Array.from( getTableColumnGroup( element ).getChildren() ); -} - -/** - * Returns an array of table column widths. - * - * @param {module:engine/model/element~Element} cell A 'table' or 'tableColumnGroup' element. - * @returns {Array} An array of table column widths. - */ -export function getColumnWidths( element ) { - return getTableColumns( element ).map( column => column.getAttribute( 'columnWidth' ) ); -} diff --git a/packages/ckeditor5-table/src/tableutils.js b/packages/ckeditor5-table/src/tableutils.js index 17ede2b8c3a..0fdd8de9ebf 100644 --- a/packages/ckeditor5-table/src/tableutils.js +++ b/packages/ckeditor5-table/src/tableutils.js @@ -1057,6 +1057,44 @@ export default class TableUtils extends Plugin { return firstCellIsInHeading === lastCellIsInHeading; } + + /** + * Returns a 'tableColumnGroup' element from the 'table'. + * + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. + */ + getColumnGroupElement( element ) { + if ( element.is( 'element', 'tableColumnGroup' ) ) { + return element; + } + + return Array + .from( element.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + } + + /** + * Returns an array of 'tableColumn' elements. + * + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of 'tableColumn' elements. + */ + getTableColumnElements( element ) { + return Array.from( this.getColumnGroupElement( element ).getChildren() ); + } + + /** + * Returns an array of table column widths. + * + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of table column widths. + */ + getTableColumnsWidths( element ) { + return this + .getTableColumnElements( element ) + .map( column => column.getAttribute( 'columnWidth' ) ); + } } // Creates empty rows at the given index in an existing table. diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js index 074f0f9382c..7e7bc4c0998 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js @@ -140,6 +140,17 @@ describe( 'TableWidthsCommand', () => { expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModel ); } ); + it( 'should work when columnWidths is a string of comma-separated values', () => { + const data = [ [ '11', '12' ], [ '21', '22' ] ]; + const attributesBefore = { columnWidths: '40%,60%', tableWidth: '40%' }; + setModelData( model, modelTable( data, attributesBefore ) ); + + command.execute( { columnWidths: '30%,70%' } ); + + const expectedModel = modelTable( data, { columnWidths: '30%,70%' } ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModel ); + } ); + it( 'should add attributes when they are provided, but were not present before', () => { const data = [ [ '11', '12' ], [ '21', '22' ] ]; const attributesBefore = {}; diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index 2f473a6ad58..a0c5d589c30 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -30,10 +30,7 @@ import { getTableWidthInPixels, getColumnMinWidthAsPercentage, getElementWidthInPixels, - getDomCellOuterWidth, - getTableColumnGroup, - getTableColumns, - getColumnWidths + getDomCellOuterWidth } from '../../src/tablecolumnresize/utils'; /* globals window, document */ @@ -721,110 +718,6 @@ describe( 'TableColumnResize utils', () => { expect( result ).to.not.equal( 0 ); } ); } ); - - describe( 'getTableColumnGroup()', () => { - let model, editor, editorElement; - - beforeEach( async () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - editor = await ClassicEditor.create( editorElement, { - plugins: [ Table, TableColumnResize, Paragraph ] - } ); - - model = editor.model; - } ); - - afterEach( async () => { - if ( editorElement ) { - editorElement.remove(); - } - - if ( editor ) { - await editor.destroy(); - } - } ); - - it( 'should return \'tableColumnGroup\' when it exists', () => { - setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - expect( getTableColumnGroup( model.document.getRoot().getChild( 0 ) ) ).to.not.be.undefined; - } ); - - it( 'should not return anything if \'tableColumnGroup\' doesn\'t exists', () => { - setModelData( model, modelTable( [ [ '01', '02' ] ] ) ); - - expect( getTableColumnGroup( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; - } ); - - it( 'should return the same \'tableColumnGroup\' element if it was passed as an argument', () => { - setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - const tableColumnGroup = model.document.getRoot().getChild( 0 ).getChild( 1 ); - - expect( getTableColumnGroup( tableColumnGroup ) ).to.equal( tableColumnGroup ); - } ); - } ); - - describe( 'getTableColumns()', () => { - let model, editor, editorElement; - - beforeEach( async () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - editor = await ClassicEditor.create( editorElement, { - plugins: [ Table, TableColumnResize, Paragraph ] - } ); - - model = editor.model; - } ); - - afterEach( async () => { - if ( editorElement ) { - editorElement.remove(); - } - - if ( editor ) { - await editor.destroy(); - } - } ); - - it( 'should return \'tableColumn\' array when there are columns', () => { - setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - expect( getTableColumns( model.document.getRoot().getChild( 0 ) ) ).to.have.length( 2 ); - } ); - } ); - - describe( 'getColumnWidths()', () => { - let model, editor, editorElement; - - beforeEach( async () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - editor = await ClassicEditor.create( editorElement, { - plugins: [ Table, TableColumnResize, Paragraph ] - } ); - - model = editor.model; - } ); - - afterEach( async () => { - if ( editorElement ) { - editorElement.remove(); - } - - if ( editor ) { - await editor.destroy(); - } - } ); - - it( 'should return \'tableColumnGroup\' count when there are columns', () => { - setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - expect( getColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ '50%', '50%' ] ); - } ); - } ); } ); function createTable( rows, cols ) { diff --git a/packages/ckeditor5-table/tests/tableutils.js b/packages/ckeditor5-table/tests/tableutils.js index 9a23526cf9b..b3e43f07e09 100644 --- a/packages/ckeditor5-table/tests/tableutils.js +++ b/packages/ckeditor5-table/tests/tableutils.js @@ -13,6 +13,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import TableSelection from '../src/tableselection'; import TableEditing from '../src/tableediting'; import TableUtils from '../src/tableutils'; +import TableColumnResize from '../src/tablecolumnresize'; import { modelTable } from './_utils/utils'; import TableWalker from '../src/tablewalker'; @@ -24,7 +25,7 @@ describe( 'TableUtils', () => { beforeEach( () => { return ModelTestEditor.create( { - plugins: [ Paragraph, TableEditing, TableUtils ] + plugins: [ Paragraph, TableEditing, TableUtils, TableColumnResize ] } ).then( newEditor => { editor = newEditor; model = editor.model; @@ -2024,6 +2025,44 @@ describe( 'TableUtils', () => { ], { headingRows: 2, headingColumns: 2 } ) ); } ); } ); + + describe( 'getTableColumnGroup()', () => { + it( 'should return tableColumnGroup when it exists', () => { + setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( tableUtils.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.not.be.undefined; + } ); + + it( 'should not return anything if tableColumnGroup does not exists', () => { + setData( model, modelTable( [ [ '01', '02' ] ] ) ); + + expect( tableUtils.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; + } ); + + it( 'should return the same tableColumnGroup element if it was passed as an argument', () => { + setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + const tableColumnGroup = model.document.getRoot().getChild( 0 ).getChild( 1 ); + + expect( tableUtils.getColumnGroupElement( tableColumnGroup ) ).to.equal( tableColumnGroup ); + } ); + } ); + + describe( 'getTableColumns()', () => { + it( 'should return tableColumn array when there are columns', () => { + setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( tableUtils.getTableColumnElements( model.document.getRoot().getChild( 0 ) ) ).to.have.length( 2 ); + } ); + } ); + + describe( 'getColumnWidths()', () => { + it( 'should return tableColumnGroup count when there are columns', () => { + setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( tableUtils.getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ '50%', '50%' ] ); + } ); + } ); } ); describe( 'TableUtils - selection methods', () => { From bfccda7095b7e42e1964bbf62cc4d493d87170fa Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 6 Feb 2023 15:35:01 +0100 Subject: [PATCH 19/25] The colgroup and col elements should be converted separately so GHS could handle other attributes. --- .../src/tablecolumnresize/converters.js | 43 +++++------------- .../tablecolumnresizeediting.js | 45 ++++++++++--------- .../src/tablecolumnresize/utils.js | 26 +++++++++++ 3 files changed, 63 insertions(+), 51 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index f2840253384..c7a225faae1 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -7,9 +7,10 @@ * @module table/tablecolumnresize/converters */ -import { normalizeColumnWidths } from './utils'; +import { normalizeColumnWidths, updateColumnElements } from './utils'; /** + * TODO * Returns a helper for converting a view `` and `` elements to the model table `columnWidths` attribute. * * Only the inline width, provided as a percentage value, in the `` element is taken into account. If there are not enough `` @@ -23,43 +24,23 @@ import { normalizeColumnWidths } from './utils'; */ export function upcastColgroupElement( tableUtilsPlugin ) { return dispatcher => dispatcher.on( 'element:colgroup', ( evt, data, conversionApi ) => { - const viewColgroupElement = data.viewItem; + const modelTable = data.modelCursor.findAncestor( 'table' ); + const tableColumnGroup = tableUtilsPlugin.getColumnGroupElement( modelTable ); - if ( !conversionApi.consumable.test( viewColgroupElement, { name: true } ) ) { + if ( !tableColumnGroup ) { return; } - conversionApi.consumable.consume( viewColgroupElement, { name: true } ); - - const modelTable = data.modelCursor.findAncestor( 'table' ); - const numberOfColumns = tableUtilsPlugin.getColumns( modelTable ); - - let columnWidths = [ ...Array( numberOfColumns ).keys() ] - .map( columnIndex => { - const viewChild = viewColgroupElement.getChild( columnIndex ); - - if ( !viewChild || !viewChild.is( 'element', 'col' ) ) { - return 'auto'; - } + const columnElements = tableUtilsPlugin.getTableColumnElements( tableColumnGroup ); + const columnsCount = tableUtilsPlugin.getColumns( modelTable ); + let columnWidths = tableUtilsPlugin.getTableColumnsWidths( tableColumnGroup ); - const viewColWidth = viewChild.getStyle( 'width' ); + columnWidths = Array.from( { length: columnsCount }, ( _, index ) => columnWidths[ index ] || 'auto' ); - if ( !viewColWidth || !viewColWidth.endsWith( '%' ) ) { - return 'auto'; - } - - return viewColWidth; - } ); - - if ( columnWidths.includes( 'auto' ) ) { - columnWidths = normalizeColumnWidths( columnWidths ); + if ( columnWidths.length != columnElements.length || columnWidths.includes( 'auto' ) ) { + updateColumnElements( columnElements, tableColumnGroup, normalizeColumnWidths( columnWidths ), conversionApi.writer ); } - - const colGroupElement = conversionApi.writer.createElement( 'tableColumnGroup' ); - - columnWidths.forEach( columnWidth => conversionApi.writer.appendElement( 'tableColumn', { columnWidth }, colGroupElement ) ); - conversionApi.writer.append( colGroupElement, modelTable ); - } ); + }, { priority: 'low' } ); } /** diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 1d7ac8a3e6e..4d2bc2f04ff 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -34,7 +34,7 @@ import { getTableWidthInPixels, normalizeColumnWidths, toPrecision, - getDomCellOuterWidth + getDomCellOuterWidth, updateColumnElements } from './utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS } from './constants'; @@ -216,21 +216,7 @@ export default class TableColumnResizeEditing extends Plugin { continue; } - for ( let i = 0; i < Math.max( normalizedWidths.length, columns.length ); i++ ) { - const column = columns[ i ]; - const columnWidth = normalizedWidths[ i ]; - - if ( !columnWidth ) { - // Number of `` elements exceeds actual number of columns - writer.remove( column ); - } else if ( !column ) { - // There is fewer `` elements than actual columns - writer.appendElement( 'tableColumn', { columnWidth }, tableColumnGroup ); - } else { - // Update column width - writer.setAttribute( 'columnWidth', columnWidth, column ); - } - } + updateColumnElements( columns, tableColumnGroup, normalizedWidths, writer ); changed = true; } @@ -356,12 +342,31 @@ export default class TableColumnResizeEditing extends Plugin { } ) } ); - // Table and elements and styles - conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); + conversion.elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); + conversion.elementToElement( { model: 'tableColumn', view: 'col' } ); conversion.for( 'downcast' ).add( downcastTableResizedClass() ); - conversion.for( 'downcast' ).elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); - conversion.for( 'downcast' ).elementToElement( { model: 'tableColumn', view: 'col' } ); + conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); + conversion.for( 'upcast' ).attributeToAttribute( { + view: { + name: 'col', + styles: { + width: /.*/ + } + }, + model: { + key: 'columnWidth', + value: viewElement => { + const viewColWidth = viewElement.getStyle( 'width' ); + + if ( !viewColWidth || !viewColWidth.endsWith( '%' ) ) { + return 'auto'; + } + + return viewColWidth; + } + } + } ); conversion.for( 'downcast' ).attributeToAttribute( { model: { name: 'tableColumn', diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index e599b732a1c..5a3011f610d 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -322,3 +322,29 @@ export function getDomCellOuterWidth( domCell ) { parseFloat( styles.borderWidth ); } } + +/** + * Updates column elements to match columns widths. + * + * @param columns + * @param tableColumnGroup + * @param normalizedWidths + * @param writer + */ +export function updateColumnElements( columns, tableColumnGroup, normalizedWidths, writer ) { + for ( let i = 0; i < Math.max( normalizedWidths.length, columns.length ); i++ ) { + const column = columns[ i ]; + const columnWidth = normalizedWidths[ i ]; + + if ( !columnWidth ) { + // Number of `` elements exceeds actual number of columns. + writer.remove( column ); + } else if ( !column ) { + // There is fewer `` elements than actual columns. + writer.appendElement( 'tableColumn', { columnWidth }, tableColumnGroup ); + } else { + // Update column width. + writer.setAttribute( 'columnWidth', columnWidth, column ); + } + } +} From 8837b007c66fb179a8f660cdf68438aae0a9c2ed Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Feb 2023 09:56:14 +0100 Subject: [PATCH 20/25] Add test in GHS checking if attributes in colgroup and col are not striped --- .../tests/integrations/table.js | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/ckeditor5-html-support/tests/integrations/table.js b/packages/ckeditor5-html-support/tests/integrations/table.js index b256689bca0..8bd36d82fcd 100644 --- a/packages/ckeditor5-html-support/tests/integrations/table.js +++ b/packages/ckeditor5-html-support/tests/integrations/table.js @@ -7,6 +7,7 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Table from '@ckeditor/ckeditor5-table/src/table'; import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; +import TableColumnResize from '@ckeditor/ckeditor5-table/src/tablecolumnresize'; import { priorities } from 'ckeditor5/src/utils'; import GeneralHtmlSupport from '../../src/generalhtmlsupport'; @@ -942,6 +943,51 @@ describe( 'TableElementSupport', () => { await editor.destroy(); } ); + // https://github.com/ckeditor/ckeditor5/issues/11479 + it( 'should not strip attributes from and elements', async () => { + const editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Table, TableCaption, TableColumnResize, Paragraph, GeneralHtmlSupport ], + htmlSupport: { + allow: [ + { + name: /^(figure|table|colgroup|col|tbody|thead|tr|th|td)$/, + attributes: true + } + ] + } + } ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
Foo
' + ); + + expect( editor.getData() ).to.equalMarkup( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
Foo
' + + '
' + ); + + await editor.destroy(); + } ); + it( 'should upcast GHS attributes at the low priority (feature attribute converter at low + 1 priority)', () => { dataFilter.loadAllowedConfig( [ { name: /.*/, From 535635372082a131f3dd62ca3838f3e3fabcee74 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Feb 2023 09:57:19 +0100 Subject: [PATCH 21/25] Fix table downcast for custom slots with `after` and `before` position --- .../src/converters/downcast.js | 10 +- .../tests/converters/downcast.js | 114 ++++++++++++++++++ 2 files changed, 119 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-table/src/converters/downcast.js b/packages/ckeditor5-table/src/converters/downcast.js index 7c6f6262112..f1ddcdfd7e6 100644 --- a/packages/ckeditor5-table/src/converters/downcast.js +++ b/packages/ckeditor5-table/src/converters/downcast.js @@ -23,6 +23,7 @@ export function downcastTable( tableUtils, options ) { return ( table, { writer } ) => { const headingRows = table.getAttribute( 'headingRows' ) || 0; const tableElement = writer.createContainerElement( 'table', null, [] ); + const figureElement = writer.createContainerElement( 'figure', { class: 'table' }, tableElement ); // Table head slot. if ( headingRows > 0 ) { @@ -56,10 +57,9 @@ export function downcastTable( tableUtils, options ) { ); } - // Create a figure element with a table and a slot with items that don't fit into the table. - const figureElement = writer.createContainerElement( 'figure', { class: 'table' }, [ - tableElement, - + // Create a slot with items that don't fit into the table. + writer.insert( + writer.createPositionAt( tableElement, 'after' ), writer.createSlot( element => { if ( element.is( 'element', 'tableRow' ) ) { return false; @@ -67,7 +67,7 @@ export function downcastTable( tableUtils, options ) { return !options.additionalSlots.some( ( { filter } ) => filter( element ) ); } ) - ] ); + ); return options.asWidget ? toTableWidget( figureElement, writer ) : figureElement; }; diff --git a/packages/ckeditor5-table/tests/converters/downcast.js b/packages/ckeditor5-table/tests/converters/downcast.js index c35b466e50f..8c587be58ff 100644 --- a/packages/ckeditor5-table/tests/converters/downcast.js +++ b/packages/ckeditor5-table/tests/converters/downcast.js @@ -258,6 +258,120 @@ describe( 'downcast converters', () => { ); } ); + it( 'should create table with custom slot at the `end` position', () => { + editor.model.schema.register( 'foo', { allowIn: 'table' } ); + editor.conversion.elementToElement( { model: 'foo', view: 'foo' } ); + + editor.plugins.get( 'TableEditing' ).registerAdditionalSlot( { + filter: element => element.is( 'element', 'foo' ), + positionOffset: 'end' + } ); + + editor.setData( + `
+ + + + + + + + +
0102
+
` + ); + + expect( editor.getData() ).to.equalMarkup( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ' ' + + '
0102
' + + '
' + ); + } ); + + it( 'should create table with custom slot at the `after` position', () => { + editor.model.schema.register( 'foo', { allowIn: 'table' } ); + editor.conversion.elementToElement( { model: 'foo', view: 'foo' } ); + + editor.plugins.get( 'TableEditing' ).registerAdditionalSlot( { + filter: element => element.is( 'element', 'foo' ), + positionOffset: 'after' + } ); + + editor.setData( + `
+ + + + + + + + +
0102
+
` + ); + + expect( editor.getData() ).to.equalMarkup( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
0102
' + + ' ' + + '
' + ); + } ); + + it( 'should create table with custom slot at the `before` position', () => { + editor.model.schema.register( 'foo', { allowIn: 'table' } ); + editor.conversion.elementToElement( { model: 'foo', view: 'foo' } ); + + editor.plugins.get( 'TableEditing' ).registerAdditionalSlot( { + filter: element => element.is( 'element', 'foo' ), + positionOffset: 'before' + } ); + + editor.setData( + `
+ + + + + + + + +
0102
+
` + ); + + expect( editor.getData() ).to.equalMarkup( + '
' + + ' ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
0102
' + + '
' + ); + } ); + it( 'should create table with block content', () => { setModelData( model, modelTable( [ [ '00foo', '01' ] From 499547c0e5493f88bb4d6641289cf2c012029473 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Feb 2023 09:59:57 +0100 Subject: [PATCH 22/25] Move utils specific to column resize to TableColumnResizeEditing plugin --- .../src/tablecolumnresize/converters.js | 30 +-- .../tablecolumnresizeediting.js | 59 ++++- .../tablecolumnresize/tablewidthscommand.js | 13 +- .../src/tablecolumnresize/utils.js | 14 +- packages/ckeditor5-table/src/tableutils.js | 38 ---- .../tablecolumnresizeediting.js | 209 ++++++++++++------ .../tests/tablecolumnresize/utils.js | 130 ++++------- packages/ckeditor5-table/tests/tableutils.js | 69 ++---- 8 files changed, 280 insertions(+), 282 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index c7a225faae1..11382308dc3 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -10,30 +10,25 @@ import { normalizeColumnWidths, updateColumnElements } from './utils'; /** - * TODO - * Returns a helper for converting a view `` and `` elements to the model table `columnWidths` attribute. + * Returns a upcast helper that ensures the number of `` elements corresponds to the actual number of columns in the table, + * because the input data might have too few or too many elements. * - * Only the inline width, provided as a percentage value, in the `` element is taken into account. If there are not enough `` - * elements matching this condition, the special value `auto` is returned. It indicates that the width of a column will be automatically - * calculated in the - * {@link module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing#_registerPostFixer post-fixer}, depending - * on the available table space. - * - * @param {module:core/plugin~Plugin} tableUtilsPlugin The {@link module:table/tableutils~TableUtils} plugin instance. + * @param {module:core/plugin~Plugin} tableUtilsPlugin + * @param {module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing} resizePlugin * @returns {Function} Conversion helper. */ -export function upcastColgroupElement( tableUtilsPlugin ) { +export function upcastColgroupElement( tableUtilsPlugin, resizePlugin ) { return dispatcher => dispatcher.on( 'element:colgroup', ( evt, data, conversionApi ) => { const modelTable = data.modelCursor.findAncestor( 'table' ); - const tableColumnGroup = tableUtilsPlugin.getColumnGroupElement( modelTable ); + const tableColumnGroup = resizePlugin.getColumnGroupElement( modelTable ); if ( !tableColumnGroup ) { return; } - const columnElements = tableUtilsPlugin.getTableColumnElements( tableColumnGroup ); + const columnElements = resizePlugin.getTableColumnElements( tableColumnGroup ); + let columnWidths = resizePlugin.getTableColumnsWidths( tableColumnGroup ); const columnsCount = tableUtilsPlugin.getColumns( modelTable ); - let columnWidths = tableUtilsPlugin.getTableColumnsWidths( tableColumnGroup ); columnWidths = Array.from( { length: columnsCount }, ( _, index ) => columnWidths[ index ] || 'auto' ); @@ -44,11 +39,12 @@ export function upcastColgroupElement( tableUtilsPlugin ) { } /** - * Returns downcast helper for adding `ck-table-resized` class if there is a `` element inside the table + * Returns downcast helper for adding `ck-table-resized` class if there is a `` element inside the table. * + * @param {module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing} resizePlugin * @returns {Function} Conversion helper. */ -export function downcastTableResizedClass() { +export function downcastTableResizedClass( resizePlugin ) { return dispatcher => dispatcher.on( 'insert:table', ( evt, data, conversionApi ) => { const viewWriter = conversionApi.writer; const modelTable = data.item; @@ -58,9 +54,7 @@ export function downcastTableResizedClass() { viewElement : Array.from( viewElement.getChildren() ).find( viewChild => viewChild.is( 'element', 'table' ) ); - const tableColumnGroup = Array - .from( data.item.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); + const tableColumnGroup = resizePlugin.getColumnGroupElement( data.item ); if ( tableColumnGroup ) { viewWriter.addClass( 'ck-table-resized', viewTable ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 4d2bc2f04ff..84e6c2c65be 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -34,7 +34,8 @@ import { getTableWidthInPixels, normalizeColumnWidths, toPrecision, - getDomCellOuterWidth, updateColumnElements + getDomCellOuterWidth, + updateColumnElements } from './utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS } from './constants'; @@ -135,6 +136,8 @@ export default class TableColumnResizeEditing extends Plugin { } ); const tableWidthsCommand = new TableWidthsCommand( editor ); + + // For backwards compatibility we have two commands that perform exactly the same operation. editor.commands.add( 'resizeTableWidth', tableWidthsCommand ); editor.commands.add( 'resizeColumnWidths', tableWidthsCommand ); @@ -163,6 +166,44 @@ export default class TableColumnResizeEditing extends Plugin { super.destroy(); } + /** + * Returns a 'tableColumnGroup' element from the 'table'. + * + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. + */ + getColumnGroupElement( element ) { + if ( element.is( 'element', 'tableColumnGroup' ) ) { + return element; + } + + return Array + .from( element.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); + } + + /** + * Returns an array of 'tableColumn' elements. + * + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of 'tableColumn' elements. + */ + getTableColumnElements( element ) { + return Array.from( this.getColumnGroupElement( element ).getChildren() ); + } + + /** + * Returns an array of table column widths. + * + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of table column widths. + */ + getTableColumnsWidths( element ) { + return this + .getTableColumnElements( element ) + .map( column => column.getAttribute( 'columnWidth' ) ); + } + /** * Registers new attributes for a table model element. * @@ -201,10 +242,10 @@ export default class TableColumnResizeEditing extends Plugin { model.document.registerPostFixer( writer => { let changed = false; - for ( const table of getChangedResizedTables( model ) ) { - const tableColumnGroup = this._tableUtilsPlugin.getColumnGroupElement( table ); - const columns = this._tableUtilsPlugin.getTableColumnElements( tableColumnGroup ); - const columnWidths = this._tableUtilsPlugin.getTableColumnsWidths( tableColumnGroup ); + for ( const table of getChangedResizedTables( model, this ) ) { + const tableColumnGroup = this.getColumnGroupElement( table ); + const columns = this.getTableColumnElements( tableColumnGroup ); + const columnWidths = this.getTableColumnsWidths( tableColumnGroup ); // Adjust the `columnWidths` attribute to guarantee that the sum of the widths from all columns is 100%. let normalizedWidths = normalizeColumnWidths( columnWidths ); @@ -344,8 +385,8 @@ export default class TableColumnResizeEditing extends Plugin { conversion.elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); conversion.elementToElement( { model: 'tableColumn', view: 'col' } ); - conversion.for( 'downcast' ).add( downcastTableResizedClass() ); - conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); + conversion.for( 'downcast' ).add( downcastTableResizedClass( this ) ); + conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin, this ) ); conversion.for( 'upcast' ).attributeToAttribute( { view: { @@ -601,10 +642,10 @@ export default class TableColumnResizeEditing extends Plugin { const editor = this.editor; const editingView = editor.editing.view; - const tableColumnGroup = this._tableUtilsPlugin.getColumnGroupElement( modelTable ); + const tableColumnGroup = this.getColumnGroupElement( modelTable ); const columnWidthsAttributeOld = tableColumnGroup ? - this._tableUtilsPlugin.getTableColumnsWidths( tableColumnGroup ) : + this.getTableColumnsWidths( tableColumnGroup ) : null; const columnWidthsAttributeNew = Array diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js index 0bdff4ff8e2..39b6bf79389 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablewidthscommand.js @@ -4,16 +4,17 @@ */ /** - * @module table/tablecolumnresize/tablecolumnwidthscommand + * @module table/tablecolumnresize/tablewidthscommand */ import { Command } from 'ckeditor5/src/core'; import { normalizeColumnWidths } from './utils'; /** - * @extends module:table/tableproperties/commands/tablepropertycommand~TablePropertyCommand + * Command used by the {@link module:table/tablecolumnresize~TableColumnResize Table column resize feature} that + * updates the width of the whole table as well as its individual columns. */ -export default class TableColumnWidthsCommand extends Command { +export default class TableWidthsCommand extends Command { /** * @inheritDoc */ @@ -40,7 +41,7 @@ export default class TableColumnWidthsCommand extends Command { } = options; if ( columnWidths ) { - // For backwards compatibility, columnWidths might be an array or a string of comma-separated values + // For backwards compatibility, columnWidths might be an array or a string of comma-separated values. columnWidths = Array.isArray( columnWidths ) ? columnWidths : columnWidths.split( ',' ); @@ -53,7 +54,9 @@ export default class TableColumnWidthsCommand extends Command { writer.removeAttribute( 'tableWidth', table ); } - const tableColumnGroup = plugins.get( 'TableUtils' ).getColumnGroupElement( table ); + const tableColumnGroup = plugins + .get( 'TableColumnResizeEditing' ) + .getColumnGroupElement( table ); if ( !columnWidths && !tableColumnGroup ) { return; diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index 5a3011f610d..b5eb001240d 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -21,9 +21,10 @@ import { * Most notably if an entire table is removed it will not be included in returned set. * * @param {module:engine/model/model~Model} model The model to collect the affected elements from. + * @param {module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing} resizePlugin TableColumnResize plugin. * @returns {Set.} A set of table model elements. */ -export function getChangedResizedTables( model ) { +export function getChangedResizedTables( model, resizePlugin ) { const affectedTables = new Set(); for ( const change of model.document.differ.getChanges() ) { @@ -71,7 +72,7 @@ export function getChangedResizedTables( model ) { continue; } - if ( !Array.from( node.getChildren() ).find( element => element.is( 'element', 'tableColumnGroup' ) ) ) { + if ( !resizePlugin.getColumnGroupElement( node ) ) { continue; } @@ -230,18 +231,17 @@ export function sumArray( array ) { * * Currently, only widths provided as percentage values are supported. * - * @param {Array.} columnWidths An array of column widths. + * @param {Array.} columnWidths An array of column widths. * @returns {Array.} An array of column widths guaranteed to sum up to 100%. */ export function normalizeColumnWidths( columnWidths ) { columnWidths = columnWidths.map( width => { - // Possible values are 'auto', number or string ending with '%' - if ( width === 'auto' || !isNaN( width ) ) { - // Leave 'auto' and number widths unchanged + // Possible values are 'auto' or string ending with '%' + if ( width === 'auto' ) { return width; } - return Number( width.replace( '%', '' ) ); + return parseFloat( width.replace( '%', '' ) ); } ); columnWidths = calculateMissingColumnWidths( columnWidths ); diff --git a/packages/ckeditor5-table/src/tableutils.js b/packages/ckeditor5-table/src/tableutils.js index 0fdd8de9ebf..17ede2b8c3a 100644 --- a/packages/ckeditor5-table/src/tableutils.js +++ b/packages/ckeditor5-table/src/tableutils.js @@ -1057,44 +1057,6 @@ export default class TableUtils extends Plugin { return firstCellIsInHeading === lastCellIsInHeading; } - - /** - * Returns a 'tableColumnGroup' element from the 'table'. - * - * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. - * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. - */ - getColumnGroupElement( element ) { - if ( element.is( 'element', 'tableColumnGroup' ) ) { - return element; - } - - return Array - .from( element.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); - } - - /** - * Returns an array of 'tableColumn' elements. - * - * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. - * @returns {Array} An array of 'tableColumn' elements. - */ - getTableColumnElements( element ) { - return Array.from( this.getColumnGroupElement( element ).getChildren() ); - } - - /** - * Returns an array of table column widths. - * - * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. - * @returns {Array} An array of table column widths. - */ - getTableColumnsWidths( element ) { - return this - .getTableColumnElements( element ) - .map( column => column.getAttribute( 'columnWidth' ) ); - } } // Creates empty rows at the given index in an existing table. diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 12a0be9c296..8c70bc17c22 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -22,7 +22,7 @@ import HighlightEditing from '@ckeditor/ckeditor5-highlight/src/highlightediting import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport'; import { focusEditor } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; -import { getTableColumnWidths, modelTable } from '../_utils/utils'; +import { modelTable } from '../_utils/utils'; import { getDomTable, getModelTable, @@ -34,7 +34,8 @@ import { getDomTableRects, getDomTableCellRects, tableColumnResizeMouseSimulator, - getDomResizer + getDomResizer, + getTableColumnWidths } from './_utils/utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS @@ -42,12 +43,13 @@ import { import { clamp, getDomCellOuterWidth } from '../../src/tablecolumnresize/utils'; +import TableWidthsCommand from '../../src/tablecolumnresize/tablewidthscommand'; import WidgetResize from '@ckeditor/ckeditor5-widget/src/widgetresize'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import { Undo } from '@ckeditor/ckeditor5-undo'; describe( 'TableColumnResizeEditing', () => { - let model, editor, view, editorElement, contentDirection; + let model, editor, view, editorElement, contentDirection, resizePlugin; const PERCENTAGE_PRECISION = 0.001; const PIXEL_PRECISION = 1; @@ -59,6 +61,7 @@ describe( 'TableColumnResizeEditing', () => { model = editor.model; view = editor.editing.view; contentDirection = editor.locale.contentLanguageDirection; + resizePlugin = editor.plugins.get( 'TableColumnResizeEditing' ); } ); afterEach( async () => { @@ -81,7 +84,7 @@ describe( 'TableColumnResizeEditing', () => { [ '10', '11', '12' ] ], { columnWidths: '25%,25%,50%' } ) ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ '25%', '25%', '50%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '25%', '25%', '50%' ] ); } ); it( 'should have defined col widths in view', () => { @@ -101,6 +104,14 @@ describe( 'TableColumnResizeEditing', () => { expect( viewColWidths ).to.be.deep.equal( [ '25%', '25%', '50%' ] ); } ); + it( 'adds `resizeTableWidth` command', () => { + expect( editor.commands.get( 'resizeTableWidth' ) ).to.be.instanceOf( TableWidthsCommand ); + } ); + + it( 'adds `resizeColumnWidths` command', () => { + expect( editor.commands.get( 'resizeColumnWidths' ) ).to.be.instanceOf( TableWidthsCommand ); + } ); + describe( 'conversion', () => { describe( 'upcast', () => { it( 'the table width style to tableWidth attribute correctly', () => { @@ -266,6 +277,48 @@ describe( 'TableColumnResizeEditing', () => { ); } ); + it( 'should handle with pixel width', () => { + editor.setData( + `
+ + + + + + + + + + + + + +
111213
+
` + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + '' + + '11' + + '' + + '' + + '12' + + '' + + '' + + '13' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + ); + } ); + it( 'should adjust the missing column widths proportionally', () => { editor.setData( `
@@ -459,9 +512,8 @@ describe( 'TableColumnResizeEditing', () => { ], { columnWidths: '50%,50%', tableWidth: '100%' } ) ); model.change( writer => { - const tableColumnGroup = Array - .from( model.document.getRoot().getChild( 0 ).getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); + const tableColumnGroup = resizePlugin.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ); + writer.remove( tableColumnGroup ); } ); @@ -563,25 +615,25 @@ describe( 'TableColumnResizeEditing', () => { it( 'if editor is in the read-only mode', () => { editor.enableReadOnlyMode( 'test' ); - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); it( 'if the TableColumnResize plugin is disabled', () => { editor.plugins.get( 'TableColumnResize' ).isEnabled = false; - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); it( 'if resizeTableWidth command is disabled', () => { editor.commands.get( 'resizeTableWidth' ).isEnabled = false; - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); it( 'if resizeColumnWidths command is disabled', () => { editor.commands.get( 'resizeColumnWidths' ).isEnabled = false; - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); } ); @@ -591,57 +643,52 @@ describe( 'TableColumnResizeEditing', () => { editor.commands.get( 'resizeTableWidth' ).isEnabled = true; editor.commands.get( 'resizeColumnWidths' ).isEnabled = true; - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed ).to.equal( true ); + expect( resizePlugin._isResizingAllowed ).to.equal( true ); } ); } ); describe( 'should change value to "false"', () => { it( 'if editor was switched to the read-only mode at runtime', () => { const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.enableReadOnlyMode( 'test' ); expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); it( 'if the TableResizeEditing plugin was disabled at runtime', () => { const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.plugins.get( 'TableColumnResize' ).isEnabled = false; expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); it( 'if resizeTableWidth command was disabled at runtime', () => { const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.commands.get( 'resizeTableWidth' ).isEnabled = false; expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); it( 'if resizeColumnWidths command was disabled at runtime', () => { const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.commands.get( 'resizeColumnWidths' ).isEnabled = false; expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( false ); + expect( resizePlugin._isResizingAllowed ).to.equal( false ); } ); } ); @@ -650,67 +697,63 @@ describe( 'TableColumnResizeEditing', () => { editor.enableReadOnlyMode( 'test' ); const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.disableReadOnlyMode( 'test' ); expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( true ); + expect( resizePlugin._isResizingAllowed ).to.equal( true ); } ); it( 'if the TableResizeEditing plugin was enabled at runtime', () => { editor.plugins.get( 'TableColumnResize' ).isEnabled = false; const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.plugins.get( 'TableColumnResize' ).isEnabled = true; expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( true ); + expect( resizePlugin._isResizingAllowed ).to.equal( true ); } ); it( 'if resizeTableWidth command was enabled at runtime', () => { editor.commands.get( 'resizeTableWidth' ).isEnabled = false; const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.commands.get( 'resizeTableWidth' ).isEnabled = true; expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( true ); + expect( resizePlugin._isResizingAllowed ).to.equal( true ); } ); it( 'if resizeColumnWidths command was enabled at runtime', () => { editor.commands.get( 'resizeColumnWidths' ).isEnabled = false; const spy = sinon.spy(); - const TableColumnResizeEditingPlugin = editor.plugins.get( 'TableColumnResizeEditing' ); - editor.listenTo( TableColumnResizeEditingPlugin, 'change:_isResizingAllowed', spy ); + editor.listenTo( resizePlugin, 'change:_isResizingAllowed', spy ); editor.commands.get( 'resizeColumnWidths' ).isEnabled = true; expect( spy.calledOnce, 'Property value should have changed once' ).to.be.true; - expect( TableColumnResizeEditingPlugin._isResizingAllowed ).to.equal( true ); + expect( resizePlugin._isResizingAllowed ).to.equal( true ); } ); } ); it( 'editable should not have the "ck-column-resize_disabled" class if "_isResizingAllowed" is set to "true"', () => { - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = true; + resizePlugin._isResizingAllowed = true; expect( editor.editing.view.document.getRoot().hasClass( 'ck-column-resize_disabled' ) ).to.equal( false ); } ); it( 'editable should have the "ck-column-resize_disabled" class if "_isResizingAllowed" is set to "false"', () => { - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; expect( editor.editing.view.document.getRoot().hasClass( 'ck-column-resize_disabled' ) ).to.equal( true ); } ); @@ -725,8 +768,8 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, view.getDomRoot() ); - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingActive ).to.be.false; - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( resizePlugin._isResizingActive ).to.be.false; + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'if resizing is not allowed', () => { @@ -735,12 +778,12 @@ describe( 'TableColumnResizeEditing', () => { [ '10', '11', '12' ] ], { columnWidths: '20%,25%,55%', tableWidth: '500px' } ) ); - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 0, 0 ) ); - expect( editor.plugins.get( 'TableColumnResizeEditing' )._isResizingActive ).to.be.false; - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( resizePlugin._isResizingActive ).to.be.false; + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'without dragging', () => { @@ -791,7 +834,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); } ); @@ -807,14 +850,14 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 0, 0 ) ); tableColumnResizeMouseSimulator.move( editor, getDomResizer( getDomTable( view ), 0, 0 ), { x: 10, y: 0 } ); - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; tableColumnResizeMouseSimulator.move( editor, getDomResizer( getDomTable( view ), 0, 0 ), { x: 10, y: 0 } ); const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'does nothing on mouseup if resizing was not started', () => { @@ -830,7 +873,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'does not change the widths if the movement vector was {0,0}', () => { @@ -880,14 +923,14 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 0, 0 ) ); tableColumnResizeMouseSimulator.move( editor, getDomResizer( getDomTable( view ), 0, 0 ), { x: 10, y: 0 } ); - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; tableColumnResizeMouseSimulator.up( editor ); const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'if columnWidths was set for the first time', () => { @@ -901,14 +944,14 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 0, 0 ) ); tableColumnResizeMouseSimulator.move( editor, getDomResizer( getDomTable( view ), 0, 0 ), { x: 10, y: 0 } ); - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; tableColumnResizeMouseSimulator.up( editor ); const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [] ); } ); it( 'if tableWidth was changed', () => { @@ -922,14 +965,14 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 2, 0 ) ); tableColumnResizeMouseSimulator.move( editor, getDomResizer( getDomTable( view ), 2, 0 ), { x: 10, y: 0 } ); - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; tableColumnResizeMouseSimulator.up( editor ); const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'if tableWidth was set for the first time', () => { @@ -943,14 +986,14 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 2, 0 ) ); tableColumnResizeMouseSimulator.move( editor, getDomResizer( getDomTable( view ), 2, 0 ), { x: 10, y: 0 } ); - editor.plugins.get( 'TableColumnResizeEditing' )._isResizingAllowed = false; + resizePlugin._isResizingAllowed = false; tableColumnResizeMouseSimulator.up( editor ); const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.eql( [ '20%', '25%', '55%' ] ); + expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); } ); @@ -1508,8 +1551,8 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + + '' + + '' + '' + '' + '' + @@ -1959,6 +2002,44 @@ describe( 'TableColumnResizeEditing', () => { } ); } ); + describe( 'getTableColumnGroup()', () => { + it( 'should return tableColumnGroup when it exists', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( resizePlugin.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.not.be.undefined; + } ); + + it( 'should not return anything if tableColumnGroup does not exists', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ] ) ); + + expect( resizePlugin.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; + } ); + + it( 'should return the same tableColumnGroup element if it was passed as an argument', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + const tableColumnGroup = model.document.getRoot().getChild( 0 ).getChild( 1 ); + + expect( resizePlugin.getColumnGroupElement( tableColumnGroup ) ).to.equal( tableColumnGroup ); + } ); + } ); + + describe( 'getTableColumns()', () => { + it( 'should return tableColumn array when there are columns', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( resizePlugin.getTableColumnElements( model.document.getRoot().getChild( 0 ) ) ).to.have.length( 2 ); + } ); + } ); + + describe( 'getColumnWidths()', () => { + it( 'should return tableColumnGroup count when there are columns', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( resizePlugin.getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '50%', '50%' ] ); + } ); + } ); + describe( 'in integration with', () => { describe( 'undo', () => { it( 'should resize correctly after undoing column insertion and resize', () => { @@ -2308,9 +2389,9 @@ describe( 'TableColumnResizeEditing', () => { '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' ) @@ -2750,11 +2831,7 @@ describe( 'TableColumnResizeEditing', () => { } function assertModelWidthsSum( columnWidths ) { - const widthsSum = columnWidths.reduce( ( sum, element ) => { - sum += Number( element ); - - return sum; - }, 0 ); + const widthsSum = columnWidths.reduce( ( sum, width ) => sum + parseFloat( width ), 0 ); expect( ( Math.abs( 100 - widthsSum ) ) < PERCENTAGE_PRECISION, 'Models widths dont sum up well' ).to.be.true; } diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index a0c5d589c30..6cef3cdcef7 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -3,7 +3,6 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import Model from '@ckeditor/ckeditor5-engine/src/model/model'; import Element from '@ckeditor/ckeditor5-engine/src/model/element'; import Text from '@ckeditor/ckeditor5-engine/src/model/text'; import Position from '@ckeditor/ckeditor5-engine/src/model/position'; @@ -36,18 +35,34 @@ import { /* globals window, document */ describe( 'TableColumnResize utils', () => { - describe( 'getChangedResizedTables()', () => { - let root, model; + let editorElement, editor, model, root, tableUtils, resizePlugin; - beforeEach( () => { - model = new Model(); - root = model.document.createRoot(); + beforeEach( async () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); - root._appendChild( [ - createTable( 2, 3 ), - createTable( 2, 3 ), - createTable( 2, 3 ) - ] ); + editor = await ClassicEditor.create( editorElement, { + plugins: [ Table, TableColumnResize, Paragraph ] + } ); + + model = editor.model; + root = model.document.getRoot(); + tableUtils = editor.plugins.get( 'TableUtils' ); + resizePlugin = editor.plugins.get( 'TableColumnResizeEditing' ); + } ); + + afterEach( async () => { + editorElement.remove(); + await editor.destroy(); + } ); + + describe( 'getChangedResizedTables()', () => { + beforeEach( () => { + model.change( writer => { + writer.insert( createTable( 2, 3 ), root ); + writer.insert( createTable( 2, 3 ), root ); + writer.insert( createTable( 2, 3 ), root ); + } ); } ); it( 'should do nothing if there is no table affected while inserting', () => { @@ -58,7 +73,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 2, 0, 0 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -78,7 +93,7 @@ describe( 'TableColumnResize utils', () => { attribute( model, range, 'attrName', null, 'attrVal' ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -100,7 +115,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 0, 1, 0 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -123,7 +138,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 0, 1, 3 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -140,7 +155,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 0, 0 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -157,7 +172,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 0, 2 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -180,7 +195,7 @@ describe( 'TableColumnResize utils', () => { 1 ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -203,7 +218,7 @@ describe( 'TableColumnResize utils', () => { 1 ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -220,7 +235,7 @@ describe( 'TableColumnResize utils', () => { 1 ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -237,7 +252,7 @@ describe( 'TableColumnResize utils', () => { 1 ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -252,7 +267,7 @@ describe( 'TableColumnResize utils', () => { model.change( () => { attribute( model, range, 'attrName', null, 'attrVal' ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -267,7 +282,7 @@ describe( 'TableColumnResize utils', () => { model.change( () => { attribute( model, range, 'attrName', null, 'attrVal' ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -282,7 +297,7 @@ describe( 'TableColumnResize utils', () => { model.change( () => { attribute( model, range, 'attrName', null, 'attrVal' ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -322,7 +337,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 2, 1, 0 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 3 ); expect( affectedTables.has( firstTable ), 'first table is affected' ).to.be.true; @@ -335,7 +350,7 @@ describe( 'TableColumnResize utils', () => { model.change( () => { remove( model, new Position( root, [ 0 ] ), 1 ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -352,7 +367,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 0 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -378,7 +393,7 @@ describe( 'TableColumnResize utils', () => { new Position( root, [ 2, 1, 2, 0 ] ) ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -405,7 +420,7 @@ describe( 'TableColumnResize utils', () => { model.change( () => { attribute( model, range, 'linkHref', 'www', null ); - const affectedTables = getChangedResizedTables( model ); + const affectedTables = getChangedResizedTables( model, resizePlugin ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -413,28 +428,6 @@ describe( 'TableColumnResize utils', () => { } ); describe( 'getColumnMinWidthAsPercentage()', () => { - let model, editor, editorElement; - - beforeEach( async () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - editor = await ClassicEditor.create( editorElement, { - plugins: [ Table, TableColumnResize, Paragraph ] - } ); - - model = editor.model; - } ); - - afterEach( async () => { - if ( editorElement ) { - editorElement.remove(); - } - - if ( editor ) { - await editor.destroy(); - } - } ); - it( 'should return the correct value', () => { setModelData( model, modelTable( [ [ '00' ] ], { 'tableWidth': '401px' } ) ); @@ -443,23 +436,6 @@ describe( 'TableColumnResize utils', () => { } ); describe( 'getColumnIndex()', () => { - let editor, tableUtils; - - beforeEach( () => { - return ClassicEditor - .create( '', { - plugins: [ Table, TableColumnResize, Paragraph ] - } ) - .then( newEditor => { - editor = newEditor; - tableUtils = editor.plugins.get( 'TableUtils' ); - } ); - } ); - - afterEach( () => { - editor.destroy(); - } ); - it( 'should properly calculate column edge indexes', () => { setModelData( editor.model, modelTable( [ [ '00', '01', '02' ], @@ -584,7 +560,7 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should handle column widths of different formats', () => { - expect( normalizeColumnWidths( [ 'auto', 25, 'auto', '25%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); + expect( normalizeColumnWidths( [ 'auto', '25%', 'auto', '25%' ] ) ).to.deep.equal( [ '25%', '25%', '25%', '25%' ] ); } ); it( 'should extend uninitialized columns equally if the free space per column is wider than the minimum column width', () => { @@ -669,22 +645,6 @@ describe( 'TableColumnResize utils', () => { } ); describe( 'getTableWidthInPixels()', () => { - let editor; - - beforeEach( () => { - return ClassicEditor - .create( '', { - plugins: [ Table, TableColumnResize ] - } ) - .then( newEditor => { - editor = newEditor; - } ); - } ); - - afterEach( () => { - editor.destroy(); - } ); - // Because the `window.getComputedStyle()` for colgroup will always return 0px on Safari, we needed to change the calculations // to be based on tbody element instead - which works ok in all main browsers. See #1466 for reference. it( 'returns a correct value on Safari', () => { diff --git a/packages/ckeditor5-table/tests/tableutils.js b/packages/ckeditor5-table/tests/tableutils.js index b3e43f07e09..d3a551a7f1a 100644 --- a/packages/ckeditor5-table/tests/tableutils.js +++ b/packages/ckeditor5-table/tests/tableutils.js @@ -23,24 +23,23 @@ describe( 'TableUtils', () => { testUtils.createSinonSandbox(); - beforeEach( () => { - return ModelTestEditor.create( { + beforeEach( async () => { + editor = await ModelTestEditor.create( { plugins: [ Paragraph, TableEditing, TableUtils, TableColumnResize ] - } ).then( newEditor => { - editor = newEditor; - model = editor.model; - root = model.document.getRoot( 'main' ); - tableUtils = editor.plugins.get( TableUtils ); + } ); - model.schema.register( 'foo', { - allowIn: 'table', - allowContentOf: '$block', - isLimit: true - } ); - editor.conversion.elementToElement( { - view: 'foo', - model: 'foo' - } ); + model = editor.model; + root = model.document.getRoot( 'main' ); + tableUtils = editor.plugins.get( 'TableUtils' ); + + model.schema.register( 'foo', { + allowIn: 'table', + allowContentOf: '$block', + isLimit: true + } ); + editor.conversion.elementToElement( { + view: 'foo', + model: 'foo' } ); } ); @@ -2025,44 +2024,6 @@ describe( 'TableUtils', () => { ], { headingRows: 2, headingColumns: 2 } ) ); } ); } ); - - describe( 'getTableColumnGroup()', () => { - it( 'should return tableColumnGroup when it exists', () => { - setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - expect( tableUtils.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.not.be.undefined; - } ); - - it( 'should not return anything if tableColumnGroup does not exists', () => { - setData( model, modelTable( [ [ '01', '02' ] ] ) ); - - expect( tableUtils.getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; - } ); - - it( 'should return the same tableColumnGroup element if it was passed as an argument', () => { - setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - const tableColumnGroup = model.document.getRoot().getChild( 0 ).getChild( 1 ); - - expect( tableUtils.getColumnGroupElement( tableColumnGroup ) ).to.equal( tableColumnGroup ); - } ); - } ); - - describe( 'getTableColumns()', () => { - it( 'should return tableColumn array when there are columns', () => { - setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - expect( tableUtils.getTableColumnElements( model.document.getRoot().getChild( 0 ) ) ).to.have.length( 2 ); - } ); - } ); - - describe( 'getColumnWidths()', () => { - it( 'should return tableColumnGroup count when there are columns', () => { - setData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); - - expect( tableUtils.getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.be.eql( [ '50%', '50%' ] ); - } ); - } ); } ); describe( 'TableUtils - selection methods', () => { From 92b470e7329d624b09057254ad5bad91d7d42c1f Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Feb 2023 10:00:42 +0100 Subject: [PATCH 23/25] Fix tests and move util to better location --- packages/ckeditor5-table/tests/_utils/utils.js | 9 --------- .../tests/tablecolumnresize/_utils/utils.js | 9 ++++++++- .../tests/tablecolumnresize/tablewidthscommand.js | 15 ++++++--------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/packages/ckeditor5-table/tests/_utils/utils.js b/packages/ckeditor5-table/tests/_utils/utils.js index 43132e7f856..d5637b4cbab 100644 --- a/packages/ckeditor5-table/tests/_utils/utils.js +++ b/packages/ckeditor5-table/tests/_utils/utils.js @@ -573,12 +573,3 @@ function getElementPlainText( model, element ) { .map( ( { item: { data } } ) => data ) .join( '' ); } - -export function getTableColumnWidths( table ) { - return Array - .from( table.getChildren() ) - .filter( element => element.is( 'element', 'tableColumnGroup' ) ) - .map( element => Array.from( element.getChildren() ) ) - .flat() - .map( element => element.getAttribute( 'columnWidth' ) ); -} diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js index f6711ad9375..20559bc779d 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js @@ -7,7 +7,6 @@ import { global } from 'ckeditor5/src/utils'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import { Point } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; import TableColumnResizeEditing from '../../../src/tablecolumnresize/tablecolumnresizeediting'; -import { getTableColumnWidths } from '../../_utils/utils'; export const tableColumnResizeMouseSimulator = { down( editor, domTarget, options = {} ) { @@ -83,6 +82,14 @@ export function getViewColumnWidthsPx( domTable ) { return widths; } +export function getTableColumnWidths( table ) { + return Array + .from( table.getChildren() ) + .filter( element => element.is( 'element', 'tableColumnGroup' ) ) + .flatMap( element => Array.from( element.getChildren() ) ) + .map( element => element.getAttribute( 'columnWidth' ) ); +} + export function getModelColumnWidthsPc( modelTable ) { return getTableColumnWidths( modelTable ).map( width => width.replace( '%', '' ) ); } diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js index 7e7bc4c0998..608d936de6c 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js @@ -9,8 +9,6 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import TableWidthsCommand from '../../src/tablecolumnresize/tablewidthscommand'; -import TableColumnResizeEditing from '../../src/tablecolumnresize/tablecolumnresizeediting'; import TableColumnResize from '../../src/tablecolumnresize'; import Table from '../../src/table'; import { modelTable } from '../_utils/utils'; @@ -22,13 +20,12 @@ describe( 'TableWidthsCommand', () => { editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); - return ClassicTestEditor.create( editorElement, { - plugins: [ Table, TableColumnResize, TableColumnResizeEditing, Paragraph ] - } ).then( newEditor => { - editor = newEditor; - model = editor.model; - command = new TableWidthsCommand( editor ); + const editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Table, TableColumnResize, Paragraph ] } ); + + model = editor.model; + command = editor.commands.get( 'resizeTableWidth' ); } ); afterEach( async () => { @@ -47,7 +44,7 @@ describe( 'TableWidthsCommand', () => { [ '21', '22' ] ] ) ); - command.execute( { columnWidths: [ '40%', '%60' ], tableWidth: '40%' } ); + command.execute( { columnWidths: [ '40%', '60%' ], tableWidth: '40%' } ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '' + From f3259381aad05949c24cf05bae7e93af899b97d7 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Feb 2023 13:49:57 +0100 Subject: [PATCH 24/25] Move tablecolumnresize utils to `utils.js` to not require TableColumnResize plugin instance --- .../src/tablecolumnresize/converters.js | 22 +- .../tablecolumnresizeediting.js | 23 +- .../src/tablecolumnresize/utils.js | 44 ++- .../tests/tablecolumnresize/_utils/utils.js | 11 +- .../tablecolumnresizeediting.js | 54 +-- .../tablecolumnresize/tablewidthscommand.js | 2 +- .../tests/tablecolumnresize/utils.js | 354 ++++++++---------- 7 files changed, 256 insertions(+), 254 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecolumnresize/converters.js b/packages/ckeditor5-table/src/tablecolumnresize/converters.js index 11382308dc3..4efb0cd2265 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/converters.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/converters.js @@ -7,27 +7,32 @@ * @module table/tablecolumnresize/converters */ -import { normalizeColumnWidths, updateColumnElements } from './utils'; +import { + normalizeColumnWidths, + updateColumnElements, + getColumnGroupElement, + getTableColumnElements, + getTableColumnsWidths +} from './utils'; /** * Returns a upcast helper that ensures the number of `` elements corresponds to the actual number of columns in the table, * because the input data might have too few or too many elements. * * @param {module:core/plugin~Plugin} tableUtilsPlugin - * @param {module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing} resizePlugin * @returns {Function} Conversion helper. */ -export function upcastColgroupElement( tableUtilsPlugin, resizePlugin ) { +export function upcastColgroupElement( tableUtilsPlugin ) { return dispatcher => dispatcher.on( 'element:colgroup', ( evt, data, conversionApi ) => { const modelTable = data.modelCursor.findAncestor( 'table' ); - const tableColumnGroup = resizePlugin.getColumnGroupElement( modelTable ); + const tableColumnGroup = getColumnGroupElement( modelTable ); if ( !tableColumnGroup ) { return; } - const columnElements = resizePlugin.getTableColumnElements( tableColumnGroup ); - let columnWidths = resizePlugin.getTableColumnsWidths( tableColumnGroup ); + const columnElements = getTableColumnElements( tableColumnGroup ); + let columnWidths = getTableColumnsWidths( tableColumnGroup ); const columnsCount = tableUtilsPlugin.getColumns( modelTable ); columnWidths = Array.from( { length: columnsCount }, ( _, index ) => columnWidths[ index ] || 'auto' ); @@ -41,10 +46,9 @@ export function upcastColgroupElement( tableUtilsPlugin, resizePlugin ) { /** * Returns downcast helper for adding `ck-table-resized` class if there is a `` element inside the table. * - * @param {module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing} resizePlugin * @returns {Function} Conversion helper. */ -export function downcastTableResizedClass( resizePlugin ) { +export function downcastTableResizedClass() { return dispatcher => dispatcher.on( 'insert:table', ( evt, data, conversionApi ) => { const viewWriter = conversionApi.writer; const modelTable = data.item; @@ -54,7 +58,7 @@ export function downcastTableResizedClass( resizePlugin ) { viewElement : Array.from( viewElement.getChildren() ).find( viewChild => viewChild.is( 'element', 'table' ) ); - const tableColumnGroup = resizePlugin.getColumnGroupElement( data.item ); + const tableColumnGroup = getColumnGroupElement( data.item ); if ( tableColumnGroup ) { viewWriter.addClass( 'ck-table-resized', viewTable ); diff --git a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js index 84e6c2c65be..87517da5d4d 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/tablecolumnresizeediting.js @@ -35,7 +35,10 @@ import { normalizeColumnWidths, toPrecision, getDomCellOuterWidth, - updateColumnElements + updateColumnElements, + getColumnGroupElement, + getTableColumnElements, + getTableColumnsWidths } from './utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS } from './constants'; @@ -173,13 +176,7 @@ export default class TableColumnResizeEditing extends Plugin { * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. */ getColumnGroupElement( element ) { - if ( element.is( 'element', 'tableColumnGroup' ) ) { - return element; - } - - return Array - .from( element.getChildren() ) - .find( element => element.is( 'element', 'tableColumnGroup' ) ); + return getColumnGroupElement( element ); } /** @@ -189,7 +186,7 @@ export default class TableColumnResizeEditing extends Plugin { * @returns {Array} An array of 'tableColumn' elements. */ getTableColumnElements( element ) { - return Array.from( this.getColumnGroupElement( element ).getChildren() ); + return getTableColumnElements( element ); } /** @@ -199,9 +196,7 @@ export default class TableColumnResizeEditing extends Plugin { * @returns {Array} An array of table column widths. */ getTableColumnsWidths( element ) { - return this - .getTableColumnElements( element ) - .map( column => column.getAttribute( 'columnWidth' ) ); + return getTableColumnsWidths( element ); } /** @@ -385,8 +380,8 @@ export default class TableColumnResizeEditing extends Plugin { conversion.elementToElement( { model: 'tableColumnGroup', view: 'colgroup' } ); conversion.elementToElement( { model: 'tableColumn', view: 'col' } ); - conversion.for( 'downcast' ).add( downcastTableResizedClass( this ) ); - conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin, this ) ); + conversion.for( 'downcast' ).add( downcastTableResizedClass() ); + conversion.for( 'upcast' ).add( upcastColgroupElement( this._tableUtilsPlugin ) ); conversion.for( 'upcast' ).attributeToAttribute( { view: { diff --git a/packages/ckeditor5-table/src/tablecolumnresize/utils.js b/packages/ckeditor5-table/src/tablecolumnresize/utils.js index b5eb001240d..41f547b8d65 100644 --- a/packages/ckeditor5-table/src/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/src/tablecolumnresize/utils.js @@ -21,10 +21,9 @@ import { * Most notably if an entire table is removed it will not be included in returned set. * * @param {module:engine/model/model~Model} model The model to collect the affected elements from. - * @param {module:table/tablecolumnresize/tablecolumnresizeediting~TableColumnResizeEditing} resizePlugin TableColumnResize plugin. * @returns {Set.} A set of table model elements. */ -export function getChangedResizedTables( model, resizePlugin ) { +export function getChangedResizedTables( model ) { const affectedTables = new Set(); for ( const change of model.document.differ.getChanges() ) { @@ -72,7 +71,7 @@ export function getChangedResizedTables( model, resizePlugin ) { continue; } - if ( !resizePlugin.getColumnGroupElement( node ) ) { + if ( !getColumnGroupElement( node ) ) { continue; } @@ -348,3 +347,42 @@ export function updateColumnElements( columns, tableColumnGroup, normalizedWidth } } } + +/** + * Returns a 'tableColumnGroup' element from the 'table'. + * + * @internal + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {module:engine/model/element~Element|undefined} A 'tableColumnGroup' element. + */ +export function getColumnGroupElement( element ) { + if ( element.is( 'element', 'tableColumnGroup' ) ) { + return element; + } + + return Array + .from( element.getChildren() ) + .find( element => element.is( 'element', 'tableColumnGroup' ) ); +} + +/** + * Returns an array of 'tableColumn' elements. + * + * @internal + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of 'tableColumn' elements. + */ +export function getTableColumnElements( element ) { + return Array.from( getColumnGroupElement( element ).getChildren() ); +} + +/** + * Returns an array of table column widths. + * + * @internal + * @param {module:engine/model/element~Element} element A 'table' or 'tableColumnGroup' element. + * @returns {Array} An array of table column widths. + */ +export function getTableColumnsWidths( element ) { + return getTableColumnElements( element ).map( column => column.getAttribute( 'columnWidth' ) ); +} diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js index 20559bc779d..5093071417d 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/_utils/utils.js @@ -7,6 +7,7 @@ import { global } from 'ckeditor5/src/utils'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import { Point } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; import TableColumnResizeEditing from '../../../src/tablecolumnresize/tablecolumnresizeediting'; +import { getTableColumnsWidths } from '../../../src/tablecolumnresize/utils'; export const tableColumnResizeMouseSimulator = { down( editor, domTarget, options = {} ) { @@ -82,16 +83,8 @@ export function getViewColumnWidthsPx( domTable ) { return widths; } -export function getTableColumnWidths( table ) { - return Array - .from( table.getChildren() ) - .filter( element => element.is( 'element', 'tableColumnGroup' ) ) - .flatMap( element => Array.from( element.getChildren() ) ) - .map( element => element.getAttribute( 'columnWidth' ) ); -} - export function getModelColumnWidthsPc( modelTable ) { - return getTableColumnWidths( modelTable ).map( width => width.replace( '%', '' ) ); + return getTableColumnsWidths( modelTable ).map( width => width.replace( '%', '' ) ); } export function getViewColumnWidthsPc( viewTable ) { diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js index 8c70bc17c22..5e641a71777 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablecolumnresizeediting.js @@ -34,14 +34,16 @@ import { getDomTableRects, getDomTableCellRects, tableColumnResizeMouseSimulator, - getDomResizer, - getTableColumnWidths + getDomResizer } from './_utils/utils'; import { COLUMN_MIN_WIDTH_IN_PIXELS } from '../../src/tablecolumnresize/constants'; import { - clamp, getDomCellOuterWidth + clamp, + getDomCellOuterWidth, + getTableColumnsWidths, + getColumnGroupElement } from '../../src/tablecolumnresize/utils'; import TableWidthsCommand from '../../src/tablecolumnresize/tablewidthscommand'; import WidgetResize from '@ckeditor/ckeditor5-widget/src/widgetresize'; @@ -84,7 +86,7 @@ describe( 'TableColumnResizeEditing', () => { [ '10', '11', '12' ] ], { columnWidths: '25%,25%,50%' } ) ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '25%', '25%', '50%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '25%', '25%', '50%' ] ); } ); it( 'should have defined col widths in view', () => { @@ -769,7 +771,7 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, view.getDomRoot() ); expect( resizePlugin._isResizingActive ).to.be.false; - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'if resizing is not allowed', () => { @@ -783,7 +785,7 @@ describe( 'TableColumnResizeEditing', () => { tableColumnResizeMouseSimulator.down( editor, getDomResizer( getDomTable( view ), 0, 0 ) ); expect( resizePlugin._isResizingActive ).to.be.false; - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'without dragging', () => { @@ -834,7 +836,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); } ); @@ -857,7 +859,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'does nothing on mouseup if resizing was not started', () => { @@ -873,7 +875,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'does not change the widths if the movement vector was {0,0}', () => { @@ -930,7 +932,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'if columnWidths was set for the first time', () => { @@ -951,7 +953,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [] ); + expect( getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; } ); it( 'if tableWidth was changed', () => { @@ -972,7 +974,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); it( 'if tableWidth was set for the first time', () => { @@ -993,7 +995,7 @@ describe( 'TableColumnResizeEditing', () => { const finalViewColumnWidthsPx = getViewColumnWidthsPx( getDomTable( view ) ); expect( finalViewColumnWidthsPx ).to.deep.equal( initialViewColumnWidthsPx ); - expect( getTableColumnWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '20%', '25%', '55%' ] ); } ); } ); @@ -2032,7 +2034,7 @@ describe( 'TableColumnResizeEditing', () => { } ); } ); - describe( 'getColumnWidths()', () => { + describe( 'getTableColumnsWidths()', () => { it( 'should return tableColumnGroup count when there are columns', () => { setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); @@ -2103,9 +2105,9 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { if ( item.item.is( 'element', 'table' ) ) { // Expect `columnWidths` to have 4 values. - expect( getTableColumnWidths( item.item ).length ).to.equal( 4 ); + expect( getTableColumnsWidths( item.item ).length ).to.equal( 4 ); // Expect a new column (it is the narrowest one) to be inserted at the first position. - expect( parseFloat( getTableColumnWidths( item.item )[ 0 ] ) < 10 ).to.be.true; + expect( parseFloat( getTableColumnsWidths( item.item )[ 0 ] ) < 10 ).to.be.true; } } } ); @@ -2123,9 +2125,9 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { if ( item.item.is( 'element', 'table' ) ) { // Expect `columnWidths` to have 4 values. - expect( getTableColumnWidths( item.item ).length ).to.equal( 4 ); + expect( getTableColumnsWidths( item.item ).length ).to.equal( 4 ); // Expect a new column (it is the narrowest one) to be inserted at the second position. - expect( parseFloat( getTableColumnWidths( item.item )[ 1 ] ) < 10 ).to.be.true; + expect( parseFloat( getTableColumnsWidths( item.item )[ 1 ] ) < 10 ).to.be.true; } } } ); @@ -2143,9 +2145,9 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { if ( item.item.is( 'element', 'table' ) ) { // Expect `columnWidths` to have 4 values. - expect( getTableColumnWidths( item.item ).length ).to.equal( 4 ); + expect( getTableColumnsWidths( item.item ).length ).to.equal( 4 ); // Expect a new column (it is the narrowest one) to be inserted at the last position. - expect( parseFloat( getTableColumnWidths( item.item )[ 3 ] ) < 10 ).to.be.true; + expect( parseFloat( getTableColumnsWidths( item.item )[ 3 ] ) < 10 ).to.be.true; } } } ); @@ -2163,7 +2165,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the next column to take over the width of removed one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = getTableColumnWidths( item.item ); + const columnWidths = getTableColumnsWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '45%' ); expect( columnWidths[ 1 ] ).to.equal( '55%' ); @@ -2184,7 +2186,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the previous column to take over the width of removed one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = getTableColumnWidths( item.item ); + const columnWidths = getTableColumnsWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '45%' ); expect( columnWidths[ 1 ] ).to.equal( '55%' ); @@ -2205,7 +2207,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the previous column to take over the width of removed one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = getTableColumnWidths( item.item ); + const columnWidths = getTableColumnsWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '20%' ); expect( columnWidths[ 1 ] ).to.equal( '80%' ); @@ -2233,7 +2235,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the first column to take over the width of merged one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = getTableColumnWidths( item.item ); + const columnWidths = getTableColumnsWidths( item.item ); expect( columnWidths.length ).to.equal( 2 ); expect( columnWidths[ 0 ] ).to.equal( '45%' ); } @@ -2262,7 +2264,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 2 values and the first column to take over the width of merged one. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = getTableColumnWidths( item.item ); + const columnWidths = getTableColumnsWidths( item.item ); expect( columnWidths.length ).to.equal( 1 ); expect( columnWidths[ 0 ] ).to.equal( '100%' ); } @@ -2289,7 +2291,7 @@ describe( 'TableColumnResizeEditing', () => { for ( const item of wholeContentRange ) { // Expect `columnWidths` to have 3 unchanged values. if ( item.item.is( 'element', 'table' ) ) { - const columnWidths = getTableColumnWidths( item.item ); + const columnWidths = getTableColumnsWidths( item.item ); expect( columnWidths.length ).to.equal( 3 ); expect( columnWidths[ 0 ] ).to.equal( '20%' ); expect( columnWidths[ 1 ] ).to.equal( '25%' ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js index 608d936de6c..7115c820092 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/tablewidthscommand.js @@ -20,7 +20,7 @@ describe( 'TableWidthsCommand', () => { editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); - const editor = await ClassicTestEditor.create( editorElement, { + editor = await ClassicTestEditor.create( editorElement, { plugins: [ Table, TableColumnResize, Paragraph ] } ); diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index 6cef3cdcef7..fdcee9d91b5 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -4,7 +4,6 @@ */ import Element from '@ckeditor/ckeditor5-engine/src/model/element'; -import Text from '@ckeditor/ckeditor5-engine/src/model/text'; import Position from '@ckeditor/ckeditor5-engine/src/model/position'; import Range from '@ckeditor/ckeditor5-engine/src/model/range'; import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; @@ -13,10 +12,6 @@ import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-util import { modelTable } from '../_utils/utils'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import InsertOperation from '@ckeditor/ckeditor5-engine/src/model/operation/insertoperation'; -import MoveOperation from '@ckeditor/ckeditor5-engine/src/model/operation/moveoperation'; -import AttributeOperation from '@ckeditor/ckeditor5-engine/src/model/operation/attributeoperation'; - import TableColumnResize from '../../src/tablecolumnresize'; import { getColumnEdgesIndexes, @@ -29,13 +24,16 @@ import { getTableWidthInPixels, getColumnMinWidthAsPercentage, getElementWidthInPixels, - getDomCellOuterWidth + getDomCellOuterWidth, + getColumnGroupElement, + getTableColumnElements, + getTableColumnsWidths } from '../../src/tablecolumnresize/utils'; /* globals window, document */ describe( 'TableColumnResize utils', () => { - let editorElement, editor, model, root, tableUtils, resizePlugin; + let editorElement, editor, model, root, tableUtils; beforeEach( async () => { editorElement = document.createElement( 'div' ); @@ -48,7 +46,6 @@ describe( 'TableColumnResize utils', () => { model = editor.model; root = model.document.getRoot(); tableUtils = editor.plugins.get( 'TableUtils' ); - resizePlugin = editor.plugins.get( 'TableColumnResizeEditing' ); } ); afterEach( async () => { @@ -66,56 +63,51 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should do nothing if there is no table affected while inserting', () => { - model.change( () => { - insert( - model, + model.change( writer => { + writer.insert( new Element( 'paragraph' ), - new Position( root, [ 2, 0, 0 ] ) + root.getChild( 2 ).getChild( 0 ).getChild( 0 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 0 ); } ); } ); it( 'should do nothing if there is no table affected while changing attribute', () => { - model.change( () => { - insert( - model, - new Element( 'paragraph' ), - new Position( root, [ 2, 0, 0 ] ) - ); - } ); + let paragraph; - model.change( () => { - const range = new Range( new Position( root, [ 2, 0, 0 ] ), new Position( root, [ 2, 0, 1 ] ) ); + model.change( writer => { + paragraph = writer.createElement( 'paragraph' ); - attribute( model, range, 'attrName', null, 'attrVal' ); + writer.insert( paragraph, root.getChild( 2 ).getChild( 0 ).getChild( 0 ) ); + } ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + model.change( writer => { + writer.setAttribute( 'attrName', 'attrVal', paragraph ); + + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 0 ); } ); } ); it( 'should find affected table - cells insertion in first column', () => { - const firstTable = root.getChild( 0 ); + model.change( writer => { + const firstTable = root.getChild( 0 ); - model.change( () => { - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 0, 0, 0 ] ) + writer.insert( + writer.createElement( 'tableCell' ), + firstTable.getChild( 0 ) ); - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 0, 1, 0 ] ) + writer.insert( + writer.createElement( 'tableCell' ), + firstTable.getChild( 1 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -123,22 +115,22 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should find affected table - cells insertion in last column', () => { - const firstTable = root.getChild( 0 ); + model.change( writer => { + const firstTable = root.getChild( 0 ); - model.change( () => { - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 0, 0, 3 ] ) + writer.insert( + writer.createElement( 'tableCell' ), + firstTable.getChild( 0 ), + 'end' ); - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 0, 1, 3 ] ) + writer.insert( + writer.createElement( 'tableCell' ), + firstTable.getChild( 1 ), + 'end' ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -146,16 +138,15 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should find affected table - cells insertion in first row', () => { - const firstTable = root.getChild( 0 ); + model.change( writer => { + const firstTable = root.getChild( 0 ); - model.change( () => { - insert( - model, + writer.insert( new Element( 'tableRow', {}, createTableCells( 3 ) ), - new Position( root, [ 0, 0 ] ) + firstTable ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -163,16 +154,20 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should find affected table - cells insertion in last row', () => { - const firstTable = root.getChild( 0 ); - - model.change( () => { - insert( - model, - new Element( 'tableRow', {}, createTableCells( 3 ) ), - new Position( root, [ 0, 2 ] ) + model.change( writer => { + const firstTable = root.getChild( 0 ); + + writer.insert( + new Element( 'tableRow', {}, [ + new Element( 'tableCell' ), + new Element( 'tableCell' ), + new Element( 'tableCell' ) + ] ), + root.getChild( 0 ), + 2 ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -182,20 +177,11 @@ describe( 'TableColumnResize utils', () => { it( 'should find affected table - cells deletion in first column', () => { const firstTable = root.getChild( 0 ); - model.change( () => { - remove( - model, - new Position( root, [ 0, 0, 0 ] ), - 1 - ); - - remove( - model, - new Position( root, [ 0, 1, 0 ] ), - 1 - ); + model.change( writer => { + writer.remove( root.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); + writer.remove( root.getChild( 0 ).getChild( 1 ).getChild( 0 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -205,20 +191,11 @@ describe( 'TableColumnResize utils', () => { it( 'should find affected table - cells deletion in last column', () => { const firstTable = root.getChild( 0 ); - model.change( () => { - remove( - model, - new Position( root, [ 0, 0, 2 ] ), - 1 - ); - - remove( - model, - new Position( root, [ 0, 1, 2 ] ), - 1 - ); + model.change( writer => { + writer.remove( root.getChild( 0 ).getChild( 0 ).getChild( 2 ) ); + writer.remove( root.getChild( 0 ).getChild( 1 ).getChild( 2 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -228,14 +205,10 @@ describe( 'TableColumnResize utils', () => { it( 'should find affected table - cells deletion in first row', () => { const firstTable = root.getChild( 0 ); - model.change( () => { - remove( - model, - new Position( root, [ 0, 0 ] ), - 1 - ); + model.change( writer => { + writer.remove( root.getChild( 0 ).getChild( 0 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -245,14 +218,10 @@ describe( 'TableColumnResize utils', () => { it( 'should find affected table - cells deletion in last row', () => { const firstTable = root.getChild( 0 ); - model.change( () => { - remove( - model, - new Position( root, [ 0, 1 ] ), - 1 - ); + model.change( writer => { + writer.remove( root.getChild( 0 ).getChild( 1 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -264,10 +233,10 @@ describe( 'TableColumnResize utils', () => { const range = new Range( new Position( root, [ 0, 0, 0 ] ), new Position( root, [ 0, 0, 3 ] ) ); - model.change( () => { - attribute( model, range, 'attrName', null, 'attrVal' ); + model.change( writer => { + writer.setAttribute( 'attrName', 'attrVal', range ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -279,10 +248,10 @@ describe( 'TableColumnResize utils', () => { const range = new Range( new Position( root, [ 0, 0 ] ), new Position( root, [ 0, 2 ] ) ); - model.change( () => { - attribute( model, range, 'attrName', null, 'attrVal' ); + model.change( writer => { + writer.setAttribute( 'attrName', 'attrVal', range ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -294,10 +263,10 @@ describe( 'TableColumnResize utils', () => { const range = new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ); - model.change( () => { - attribute( model, range, 'attrName', null, 'attrVal' ); + model.change( writer => { + writer.setAttribute( 'attrName', 'attrVal', range ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 1 ); expect( affectedTables.has( firstTable ) ).to.be.true; @@ -305,39 +274,31 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should find all affected tables - mixed operations', () => { - const firstTable = root.getChild( 0 ); - const secondTable = root.getChild( 1 ); - const thirdTable = root.getChild( 2 ); - - model.change( () => { - remove( - model, - new Position( root, [ 0, 0 ] ), - 1 - ); - - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 0, 0, 0 ] ) + model.change( writer => { + const firstTable = root.getChild( 0 ); + const secondTable = root.getChild( 1 ); + const thirdTable = root.getChild( 2 ); + + writer.remove( firstTable.getChild( 0 ) ); + writer.insert( + writer.createElement( 'tableCell' ), + firstTable.getChild( 0 ) ); const range = new Range( new Position( root, [ 1 ] ), new Position( root, [ 2 ] ) ); - attribute( model, range, 'attrName', null, 'attrVal' ); - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 2, 0, 0 ] ) + writer.setAttribute( 'attrName', 'attrVal', range ); + writer.insert( + writer.createElement( 'tableCell' ), + root.getChild( 2 ).getChild( 0 ) ); - insert( - model, - new Element( 'tableCell' ), - new Position( root, [ 2, 1, 0 ] ) + writer.insert( + writer.createElement( 'tableCell' ), + root.getChild( 2 ).getChild( 1 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 3 ); expect( affectedTables.has( firstTable ), 'first table is affected' ).to.be.true; @@ -347,53 +308,48 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should not find affected table - table removal', () => { - model.change( () => { - remove( model, new Position( root, [ 0 ] ), 1 ); + model.change( writer => { + writer.remove( root.getChild( 0 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 0 ); } ); } ); it( 'should not find affected table - table replacement', () => { - model.change( () => { - remove( model, new Position( root, [ 0 ] ), 1 ); + model.change( writer => { + writer.remove( root.getChild( 0 ) ); // Table plugin inserts a paragraph when a table is removed - #12201. - insert( - model, - new Element( 'paragraph' ), - new Position( root, [ 0 ] ) - ); + writer.insert( writer.createElement( 'paragraph' ), root ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 0 ); } ); } ); it( 'should not find any affected table if operation is not related to a table, row or cell element', () => { - model.change( () => { - insert( - model, - new Text( 'foo', { bold: true } ), - new Position( root, [ 0, 0, 0, 0 ] ) + model.change( writer => { + writer.insertText( + 'foo', + { bold: true }, + root.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); - insert( - model, - new Text( 'foo', { italic: true } ), - new Position( root, [ 1, 1, 1, 0 ] ) + writer.insertText( + 'foo', + { italic: true }, + root.getChild( 1 ).getChild( 1 ).getChild( 1 ) ); - insert( - model, - new Text( 'foo' ), - new Position( root, [ 2, 1, 2, 0 ] ) + writer.insertText( + 'foo', + root.getChild( 2 ).getChild( 1 ).getChild( 2 ) ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -404,23 +360,21 @@ describe( 'TableColumnResize utils', () => { // To test the getChangedResizedTables(), when the attribute is being removed we need // to first insert the text inside one of the table cells. - model.change( () => { - insert( - model, - new Text( 'foo' ), - new Position( root, [ 0, 0, 0, 0 ] ) - ); + model.change( writer => { + const position = root.getChild( 0 ).getChild( 0 ).getChild( 0 ); + + writer.insertText( 'foo', position ); - range = new Range( new Position( root, [ 0, 0, 0, 1 ] ), new Position( root, [ 0, 0, 0, 3 ] ) ); + range = writer.createRangeIn( position ); - attribute( model, range, 'linkHref', null, 'www' ); + writer.setAttribute( 'linkHref', 'www', range ); } ); // And in a different model.change() remove the attribute, because otherwise the changes would be empty. - model.change( () => { - attribute( model, range, 'linkHref', 'www', null ); + model.change( writer => { + writer.setAttribute( 'linkHref', null, range ); - const affectedTables = getChangedResizedTables( model, resizePlugin ); + const affectedTables = getChangedResizedTables( model ); expect( affectedTables.size ).to.equal( 0 ); } ); @@ -678,6 +632,44 @@ describe( 'TableColumnResize utils', () => { expect( result ).to.not.equal( 0 ); } ); } ); + + describe( 'getTableColumnGroup()', () => { + it( 'should return tableColumnGroup when it exists', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.not.be.undefined; + } ); + + it( 'should not return anything if tableColumnGroup does not exists', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ] ) ); + + expect( getColumnGroupElement( model.document.getRoot().getChild( 0 ) ) ).to.be.undefined; + } ); + + it( 'should return the same tableColumnGroup element if it was passed as an argument', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + const tableColumnGroup = model.document.getRoot().getChild( 0 ).getChild( 1 ); + + expect( getColumnGroupElement( tableColumnGroup ) ).to.equal( tableColumnGroup ); + } ); + } ); + + describe( 'getTableColumns()', () => { + it( 'should return tableColumn array when there are columns', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( getTableColumnElements( model.document.getRoot().getChild( 0 ) ) ).to.have.length( 2 ); + } ); + } ); + + describe( 'getColumnWidths()', () => { + it( 'should return tableColumnGroup count when there are columns', () => { + setModelData( model, modelTable( [ [ '01', '02' ] ], { columnWidths: '50%,50%' } ) ); + + expect( getTableColumnsWidths( model.document.getRoot().getChild( 0 ) ) ).to.deep.equal( [ '50%', '50%' ] ); + } ); + } ); } ); function createTable( rows, cols ) { @@ -703,25 +695,3 @@ function createTableRows( rows, cols ) { function createTableCells( cols ) { return [ ...Array( cols ) ].map( () => new Element( 'tableCell' ) ); } - -function insert( model, item, position ) { - const doc = model.document; - const operation = new InsertOperation( position, item, doc.version ); - - model.applyOperation( operation ); -} - -function remove( model, sourcePosition, howMany ) { - const doc = model.document; - const targetPosition = Position._createAt( doc.graveyard, doc.graveyard.maxOffset ); - const operation = new MoveOperation( sourcePosition, howMany, targetPosition, doc.version ); - - model.applyOperation( operation ); -} - -function attribute( model, range, key, oldValue, newValue ) { - const doc = model.document; - const operation = new AttributeOperation( range, key, oldValue, newValue, doc.version ); - - model.applyOperation( operation ); -} From 2f9d641d50601c2480dc1043556e153203782764 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Thu, 9 Feb 2023 09:32:14 +0100 Subject: [PATCH 25/25] Update tests to take better use of model writer --- .../tests/tablecolumnresize/utils.js | 71 +++++++++++-------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js index fdcee9d91b5..92c520b1cbb 100644 --- a/packages/ckeditor5-table/tests/tablecolumnresize/utils.js +++ b/packages/ckeditor5-table/tests/tablecolumnresize/utils.js @@ -4,8 +4,6 @@ */ import Element from '@ckeditor/ckeditor5-engine/src/model/element'; -import Position from '@ckeditor/ckeditor5-engine/src/model/position'; -import Range from '@ckeditor/ckeditor5-engine/src/model/range'; import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; import Table from '../../src/table'; import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; @@ -66,7 +64,7 @@ describe( 'TableColumnResize utils', () => { model.change( writer => { writer.insert( new Element( 'paragraph' ), - root.getChild( 2 ).getChild( 0 ).getChild( 0 ) + root.getNodeByPath( [ 2, 0, 0 ] ) ); const affectedTables = getChangedResizedTables( model ); @@ -81,7 +79,7 @@ describe( 'TableColumnResize utils', () => { model.change( writer => { paragraph = writer.createElement( 'paragraph' ); - writer.insert( paragraph, root.getChild( 2 ).getChild( 0 ).getChild( 0 ) ); + writer.insert( paragraph, root.getNodeByPath( [ 2, 0, 0 ] ) ); } ); model.change( writer => { @@ -158,12 +156,8 @@ describe( 'TableColumnResize utils', () => { const firstTable = root.getChild( 0 ); writer.insert( - new Element( 'tableRow', {}, [ - new Element( 'tableCell' ), - new Element( 'tableCell' ), - new Element( 'tableCell' ) - ] ), - root.getChild( 0 ), + new Element( 'tableRow', {}, createTableCells( 3 ) ), + firstTable, 2 ); @@ -178,8 +172,8 @@ describe( 'TableColumnResize utils', () => { const firstTable = root.getChild( 0 ); model.change( writer => { - writer.remove( root.getChild( 0 ).getChild( 0 ).getChild( 0 ) ); - writer.remove( root.getChild( 0 ).getChild( 1 ).getChild( 0 ) ); + writer.remove( root.getNodeByPath( [ 0, 0, 0 ] ) ); + writer.remove( root.getNodeByPath( [ 0, 1, 0 ] ) ); const affectedTables = getChangedResizedTables( model ); @@ -192,8 +186,8 @@ describe( 'TableColumnResize utils', () => { const firstTable = root.getChild( 0 ); model.change( writer => { - writer.remove( root.getChild( 0 ).getChild( 0 ).getChild( 2 ) ); - writer.remove( root.getChild( 0 ).getChild( 1 ).getChild( 2 ) ); + writer.remove( root.getNodeByPath( [ 0, 0, 2 ] ) ); + writer.remove( root.getNodeByPath( [ 0, 1, 2 ] ) ); const affectedTables = getChangedResizedTables( model ); @@ -206,7 +200,7 @@ describe( 'TableColumnResize utils', () => { const firstTable = root.getChild( 0 ); model.change( writer => { - writer.remove( root.getChild( 0 ).getChild( 0 ) ); + writer.remove( root.getNodeByPath( [ 0, 0, 0 ] ) ); const affectedTables = getChangedResizedTables( model ); @@ -219,7 +213,7 @@ describe( 'TableColumnResize utils', () => { const firstTable = root.getChild( 0 ); model.change( writer => { - writer.remove( root.getChild( 0 ).getChild( 1 ) ); + writer.remove( root.getNodeByPath( [ 0, 1 ] ) ); const affectedTables = getChangedResizedTables( model ); @@ -231,9 +225,12 @@ describe( 'TableColumnResize utils', () => { it( 'should find affected table - attribute change on multiple cells', () => { const firstTable = root.getChild( 0 ); - const range = new Range( new Position( root, [ 0, 0, 0 ] ), new Position( root, [ 0, 0, 3 ] ) ); - model.change( writer => { + const range = writer.createRange( + writer.createPositionAt( root.getNodeByPath( [ 0, 0 ] ), 0 ), + writer.createPositionAt( root.getNodeByPath( [ 0, 0 ] ), 3 ) + ); + writer.setAttribute( 'attrName', 'attrVal', range ); const affectedTables = getChangedResizedTables( model ); @@ -244,11 +241,14 @@ describe( 'TableColumnResize utils', () => { } ); it( 'should find affected table - attribute change on multiple rows', () => { - const firstTable = root.getChild( 0 ); + model.change( writer => { + const firstTable = root.getChild( 0 ); - const range = new Range( new Position( root, [ 0, 0 ] ), new Position( root, [ 0, 2 ] ) ); + const range = writer.createRange( + writer.createPositionAt( firstTable, 0 ), + writer.createPositionAt( firstTable, 2 ) + ); - model.change( writer => { writer.setAttribute( 'attrName', 'attrVal', range ); const affectedTables = getChangedResizedTables( model ); @@ -261,9 +261,12 @@ describe( 'TableColumnResize utils', () => { it( 'should find affected table - attribute change on a table', () => { const firstTable = root.getChild( 0 ); - const range = new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ); - model.change( writer => { + const range = writer.createRange( + writer.createPositionAt( root, 0 ), + writer.createPositionAt( root, 1 ) + ); + writer.setAttribute( 'attrName', 'attrVal', range ); const affectedTables = getChangedResizedTables( model ); @@ -285,7 +288,10 @@ describe( 'TableColumnResize utils', () => { firstTable.getChild( 0 ) ); - const range = new Range( new Position( root, [ 1 ] ), new Position( root, [ 2 ] ) ); + const range = writer.createRange( + writer.createPositionAt( root, 1 ), + writer.createPositionAt( root, 3 ) + ); writer.setAttribute( 'attrName', 'attrVal', range ); writer.insert( @@ -335,18 +341,18 @@ describe( 'TableColumnResize utils', () => { writer.insertText( 'foo', { bold: true }, - root.getChild( 0 ).getChild( 0 ).getChild( 0 ) + root.getNodeByPath( [ 0, 0, 0, 0 ] ) ); writer.insertText( 'foo', { italic: true }, - root.getChild( 1 ).getChild( 1 ).getChild( 1 ) + root.getNodeByPath( [ 1, 1, 1, 0 ] ) ); writer.insertText( 'foo', - root.getChild( 2 ).getChild( 1 ).getChild( 2 ) + root.getNodeByPath( [ 2, 1, 2, 0 ] ) ); const affectedTables = getChangedResizedTables( model ); @@ -361,18 +367,21 @@ describe( 'TableColumnResize utils', () => { // To test the getChangedResizedTables(), when the attribute is being removed we need // to first insert the text inside one of the table cells. model.change( writer => { - const position = root.getChild( 0 ).getChild( 0 ).getChild( 0 ); + const paragraph = root.getNodeByPath( [ 0, 0, 0, 0 ] ); - writer.insertText( 'foo', position ); + writer.insertText( 'foo', paragraph, 0 ); - range = writer.createRangeIn( position ); + range = writer.createRange( + writer.createPositionAt( paragraph, 1 ), + writer.createPositionAt( paragraph, 3 ) + ); writer.setAttribute( 'linkHref', 'www', range ); } ); // And in a different model.change() remove the attribute, because otherwise the changes would be empty. model.change( writer => { - writer.setAttribute( 'linkHref', null, range ); + writer.removeAttribute( 'linkHref', range ); const affectedTables = getChangedResizedTables( model );