Skip to content

Commit

Permalink
Merge pull request #7410 from ckeditor/i/6574
Browse files Browse the repository at this point in the history 
Tests (table): Use real schema and conversion definitions in table tests. Closes #6574.

Other (table): Removed `options.asWidget` from most of the table converters which are never run in data pipeline.
  • Loading branch information
@niegowski
niegowski authored Jun 10, 2020
2 parents e81cbbb + 353fe5f commit b127f41
Show file tree
Hide file tree
Showing 18 changed files with 1,233 additions and 1,420 deletions.
68 changes: 28 additions & 40 deletions packages/ckeditor5-table/src/converters/downcast.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function downcastInsertTable( options = {} ) {
*
* @returns {Function} Conversion helper.
*/
export function downcastInsertRow( options = {} ) {
export function downcastInsertRow() {
return dispatcher => dispatcher.on( 'insert:tableRow', ( evt, data, conversionApi ) => {
const tableRow = data.item;

Expand Down Expand Up @@ -120,7 +120,7 @@ export function downcastInsertRow( options = {} ) {

const insertPosition = conversionApi.writer.createPositionAt( trElement, 'end' );

createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi, options );
createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi, { asWidget: true } );
}
} );
}
Expand All @@ -133,7 +133,7 @@ export function downcastInsertRow( options = {} ) {
*
* @returns {Function} Conversion helper.
*/
export function downcastInsertCell( options = {} ) {
export function downcastInsertCell() {
return dispatcher => dispatcher.on( 'insert:tableCell', ( evt, data, conversionApi ) => {
const tableCell = data.item;

Expand All @@ -158,7 +158,7 @@ export function downcastInsertCell( options = {} ) {
const trElement = conversionApi.mapper.toViewElement( tableRow );
const insertPosition = conversionApi.writer.createPositionAt( trElement, tableRow.getChildIndex( tableCell ) );

createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi, options );
createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi, { asWidget: true } );

// No need to iterate further.
return;
Expand All @@ -178,9 +178,7 @@ export function downcastInsertCell( options = {} ) {
*
* @returns {Function} Conversion helper.
*/
export function downcastTableHeadingRowsChange( options = {} ) {
const asWidget = !!options.asWidget;

export function downcastTableHeadingRowsChange() {
return dispatcher => dispatcher.on( 'attribute:headingRows:table', ( evt, data, conversionApi ) => {
const table = data.item;

Expand All @@ -205,7 +203,7 @@ export function downcastTableHeadingRowsChange( options = {} ) {
// Rename all table cells from moved rows to 'th' as they lands in <thead>.
for ( const tableRow of rowsToMove ) {
for ( const tableCell of tableRow.getChildren() ) {
renameViewTableCell( tableCell, 'th', conversionApi, asWidget );
renameViewTableCell( tableCell, 'th', conversionApi );
}
}
}
Expand All @@ -228,7 +226,7 @@ export function downcastTableHeadingRowsChange( options = {} ) {
};

for ( const tableSlot of tableWalker ) {
renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionApi, asWidget );
renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionApi );
}
}

Expand All @@ -249,9 +247,7 @@ export function downcastTableHeadingRowsChange( options = {} ) {
*
* @returns {Function} Conversion helper.
*/
export function downcastTableHeadingColumnsChange( options = {} ) {
const asWidget = !!options.asWidget;

export function downcastTableHeadingColumnsChange() {
return dispatcher => dispatcher.on( 'attribute:headingColumns:table', ( evt, data, conversionApi ) => {
const table = data.item;

Expand All @@ -270,7 +266,7 @@ export function downcastTableHeadingColumnsChange( options = {} ) {
const lastColumnToCheck = ( oldColumns > newColumns ? oldColumns : newColumns ) - 1;

for ( const tableSlot of new TableWalker( table, { endColumn: lastColumnToCheck } ) ) {
renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionApi, asWidget );
renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionApi );
}
} );
}
Expand Down Expand Up @@ -327,8 +323,7 @@ function toTableWidget( viewElement, writer ) {
// @param {module:engine/model/element~Element} tableCell
// @param {String} desiredCellElementName
// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi
// @param {Boolean} asWidget
function renameViewTableCell( tableCell, desiredCellElementName, conversionApi, asWidget ) {
function renameViewTableCell( tableCell, desiredCellElementName, conversionApi ) {
const viewWriter = conversionApi.writer;
const viewCell = conversionApi.mapper.toViewElement( tableCell );

Expand All @@ -337,41 +332,30 @@ function renameViewTableCell( tableCell, desiredCellElementName, conversionApi,
return;
}

let renamedCell;

if ( asWidget ) {
const editable = viewWriter.createEditableElement( desiredCellElementName, viewCell.getAttributes() );
renamedCell = toWidgetEditable( editable, viewWriter );
const editable = viewWriter.createEditableElement( desiredCellElementName, viewCell.getAttributes() );
const renamedCell = toWidgetEditable( editable, viewWriter );

setHighlightHandling(
renamedCell,
viewWriter,
( element, descriptor, writer ) => writer.addClass( normalizeToArray( descriptor.classes ), element ),
( element, descriptor, writer ) => writer.removeClass( normalizeToArray( descriptor.classes ), element )
);
setHighlightHandling(
renamedCell,
viewWriter,
( element, descriptor, writer ) => writer.addClass( normalizeToArray( descriptor.classes ), element ),
( element, descriptor, writer ) => writer.removeClass( normalizeToArray( descriptor.classes ), element )
);

viewWriter.insert( viewWriter.createPositionAfter( viewCell ), renamedCell );
viewWriter.move( viewWriter.createRangeIn( viewCell ), viewWriter.createPositionAt( renamedCell, 0 ) );
viewWriter.remove( viewWriter.createRangeOn( viewCell ) );
} else {
renamedCell = viewWriter.rename( desiredCellElementName, viewCell );
}
viewWriter.insert( viewWriter.createPositionAfter( viewCell ), renamedCell );
viewWriter.move( viewWriter.createRangeIn( viewCell ), viewWriter.createPositionAt( renamedCell, 0 ) );
viewWriter.remove( viewWriter.createRangeOn( viewCell ) );

conversionApi.mapper.unbindViewElement( viewCell );
conversionApi.mapper.bindElements( tableCell, renamedCell );
}

function normalizeToArray( classes ) {
return Array.isArray( classes ) ? classes : [ classes ];
}

// Renames a table cell element in the view according to its location in the table.
//
// @param {module:table/tablewalker~TableSlot} tableSlot
// @param {{headingColumns, headingRows}} tableAttributes
// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi
// @param {Boolean} asWidget
function renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionApi, asWidget ) {
function renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionApi ) {
const { cell } = tableSlot;

// Check whether current columnIndex is overlapped by table cells from previous rows.
Expand All @@ -382,7 +366,7 @@ function renameViewTableCellIfRequired( tableSlot, tableAttributes, conversionAp
// If in single change we're converting attribute changes and inserting cell the table cell might not be inserted into view
// because of child conversion is done after parent.
if ( viewCell && viewCell.name !== desiredCellElementName ) {
renameViewTableCell( cell, desiredCellElementName, conversionApi, asWidget );
renameViewTableCell( cell, desiredCellElementName, conversionApi );
}
}

Expand Down Expand Up @@ -421,7 +405,7 @@ function createViewTableCellElement( tableSlot, tableAttributes, insertPosition,

conversionApi.consumable.consume( innerParagraph, 'insert' );

if ( options.asWidget ) {
if ( asWidget ) {
// Use display:inline-block to force Chrome/Safari to limit text mutations to this element.
// See #6062.
const fakeParagraph = conversionApi.writer.createContainerElement( 'span', { style: 'display:inline-block' } );
Expand Down Expand Up @@ -589,3 +573,7 @@ function getViewTable( viewFigure ) {
function hasAnyAttribute( element ) {
return !![ ...element.getAttributeKeys() ].length;
}

function normalizeToArray( classes ) {
return Array.isArray( classes ) ? classes : [ classes ];
}
14 changes: 5 additions & 9 deletions packages/ckeditor5-table/src/tableediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,26 +100,22 @@ export default class TableEditing extends Plugin {
conversion.for( 'upcast' ).elementToElement( { model: 'tableRow', view: 'tr' } );
conversion.for( 'upcast' ).add( skipEmptyTableRow() );

conversion.for( 'editingDowncast' ).add( downcastInsertRow( { asWidget: true } ) );
conversion.for( 'dataDowncast' ).add( downcastInsertRow() );
conversion.for( 'downcast' ).add( downcastRemoveRow() );
conversion.for( 'editingDowncast' ).add( downcastInsertRow() );
conversion.for( 'editingDowncast' ).add( downcastRemoveRow() );

// Table cell conversion.
conversion.for( 'upcast' ).add( upcastTableCell( 'td' ) );
conversion.for( 'upcast' ).add( upcastTableCell( 'th' ) );

conversion.for( 'editingDowncast' ).add( downcastInsertCell( { asWidget: true } ) );
conversion.for( 'dataDowncast' ).add( downcastInsertCell() );
conversion.for( 'editingDowncast' ).add( downcastInsertCell() );

// Table attributes conversion.
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );

// Table heading rows and columns conversion.
conversion.for( 'editingDowncast' ).add( downcastTableHeadingColumnsChange( { asWidget: true } ) );
conversion.for( 'dataDowncast' ).add( downcastTableHeadingColumnsChange() );
conversion.for( 'editingDowncast' ).add( downcastTableHeadingRowsChange( { asWidget: true } ) );
conversion.for( 'dataDowncast' ).add( downcastTableHeadingRowsChange() );
conversion.for( 'editingDowncast' ).add( downcastTableHeadingColumnsChange() );
conversion.for( 'editingDowncast' ).add( downcastTableHeadingRowsChange() );

// Define all the commands.
editor.commands.add( 'insertTable', new InsertTableCommand( editor ) );
Expand Down
106 changes: 12 additions & 94 deletions packages/ckeditor5-table/tests/_utils/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,11 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import {
downcastInsertCell,
downcastInsertRow,
downcastInsertTable,
downcastRemoveRow,
downcastTableHeadingColumnsChange,
downcastTableHeadingRowsChange
} from '../../src/converters/downcast';
import upcastTable, { upcastTableCell } from '../../src/converters/upcasttable';
import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import TableWalker from '../../src/tablewalker';

const WIDGET_TABLE_CELL_CLASS = 'ck-editor__editable ck-editor__nested-editable';
const BORDER_REG_EXP = /[\s\S]+/;

/**
* Returns a model representation of a table shorthand notation:
Expand Down Expand Up @@ -180,85 +170,6 @@ export function viewTable( tableData, attributes = {} ) {
return `<figure ${ figureAttributes }>${ asWidget ? widgetHandler : '' }<table>${ thead }${ tbody }</table></figure>`;
}

export function defaultSchema( schema, registerParagraph = true ) {
schema.register( 'table', {
allowWhere: '$block',
allowAttributes: [ 'headingRows', 'headingColumns' ],
isLimit: true,
isObject: true,
isBlock: true
} );

schema.register( 'tableRow', {
allowIn: 'table',
isLimit: true
} );

schema.register( 'tableCell', {
allowIn: 'tableRow',
allowAttributes: [ 'colspan', 'rowspan' ],
isObject: true
} );

// Allow all $block content inside table cell.
schema.extend( '$block', { allowIn: 'tableCell' } );

// Disallow table in table.
schema.addChildCheck( ( context, childDefinition ) => {
if ( childDefinition.name == 'table' && Array.from( context.getNames() ).includes( 'table' ) ) {
return false;
}
} );

if ( registerParagraph ) {
schema.register( 'paragraph', { inheritAllFrom: '$block' } );
}

// Styles
schema.extend( 'tableCell', {
allowAttributes: [ 'border' ]
} );
}

export function defaultConversion( conversion, asWidget = false ) {
conversion.elementToElement( { model: 'paragraph', view: 'p' } );

// Table conversion.
conversion.for( 'upcast' ).add( upcastTable() );
conversion.for( 'downcast' ).add( downcastInsertTable( { asWidget } ) );

// Table row conversion.
conversion.for( 'upcast' ).elementToElement( { model: 'tableRow', view: 'tr' } );
conversion.for( 'downcast' ).add( downcastInsertRow( { asWidget } ) );
conversion.for( 'downcast' ).add( downcastRemoveRow( { asWidget } ) );

// Table cell conversion.
conversion.for( 'upcast' ).add( upcastTableCell( 'td' ) );
conversion.for( 'upcast' ).add( upcastTableCell( 'th' ) );
conversion.for( 'downcast' ).add( downcastInsertCell( { asWidget } ) );

// Table attributes conversion.
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );

conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange( { asWidget } ) );
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange( { asWidget } ) );

// Styles
conversion.for( 'upcast' ).attributeToAttribute( {
view: {
name: 'td',
styles: {
border: BORDER_REG_EXP
}
},
model: {
key: 'border',
value: viewElement => viewElement.getStyle( 'border' )
}
} );
}

/**
* An assertion helper for top-right-bottom-left attribute object.
*
Expand Down Expand Up @@ -420,10 +331,10 @@ function formatAttributes( attributes ) {
let attributesString = '';

if ( attributes ) {
const entries = Object.entries( attributes );
const sortedKeys = Object.keys( attributes ).sort();

if ( entries.length ) {
attributesString = ' ' + entries.map( entry => `${ entry[ 0 ] }="${ entry[ 1 ] }"` ).join( ' ' );
if ( sortedKeys.length ) {
attributesString = ' ' + sortedKeys.map( key => `${ key }="${ attributes[ key ] }"` ).join( ' ' );
}
}

Expand Down Expand Up @@ -456,17 +367,24 @@ function makeRows( tableData, options ) {
delete tableCellData.isSelected;
}

const attributes = isObject ? tableCellData : {};
let attributes = {};

if ( asWidget ) {
attributes.class = getClassToSet( attributes );
attributes.contenteditable = 'true';
}

if ( isObject ) {
attributes = {
...attributes,
...tableCellData
};
}

if ( !( contents.replace( '[', '' ).replace( ']', '' ).startsWith( '<' ) ) && enforceWrapping ) {
contents =
`<${ wrappingElement == 'span' ? 'span style="display:inline-block"' : wrappingElement }>` +
contents +
contents +
`</${ wrappingElement }>`;
}

Expand Down
Loading

0 comments on commit b127f41

Please sign in to comment.