Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #132 from ckeditor/t/130
Browse files Browse the repository at this point in the history 
Other: The table cell view post-fixer should use changed elements from the view to run fixes. Closes #130.
  • Loading branch information
Piotr Jasiun authored Oct 2, 2018
2 parents 1eb5d6d + 2fbcc91 commit efc53c9
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 96 deletions.
143 changes: 75 additions & 68 deletions src/converters/tablecell-post-fixer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,102 +71,109 @@
* @param {module:engine/controller/editingcontroller~EditingController} editing
*/
export default function injectTableCellPostFixer( model, editing ) {
editing.view.document.registerPostFixer( writer => tableCellPostFixer( writer, model, editing.mapper ) );
editing.view.document.registerPostFixer( writer => tableCellPostFixer( writer, model, editing.mapper, editing.view ) );
}

// The table cell post-fixer.
//
// @param {module:engine/view/writer~Writer} writer
// @param {module:engine/model/model~Model} model
// @param {module:engine/conversion/mapper~Mapper} mapper
function tableCellPostFixer( writer, model, mapper ) {
const changes = model.document.differ.getChanges();
function tableCellPostFixer( writer, model, mapper, view ) {
let wasFixed = false;

// While this is view post fixer only nodes that changed are worth investigating.
for ( const entry of changes ) {
// Attribute change - check if it is single paragraph inside table cell that has attributes changed.
if ( entry.type == 'attribute' && entry.range.start.parent.name == 'tableCell' ) {
const tableCell = entry.range.start.parent;

if ( tableCell.childCount === 1 ) {
const singleChild = tableCell.getChild( 0 );
const renameTo = Array.from( singleChild.getAttributes() ).length ? 'p' : 'span';

wasFixed = renameParagraphIfDifferent( singleChild, renameTo, writer, model, mapper ) || wasFixed;
}
} else {
// Check all nodes inside table cell on insert/remove operations (also other blocks).
const parent = entry.position && entry.position.parent;

if ( parent && parent.is( 'tableCell' ) ) {
const renameTo = parent.childCount > 1 ? 'p' : 'span';

for ( const child of parent.getChildren() ) {
wasFixed = renameParagraphIfDifferent( child, renameTo, writer, model, mapper ) || wasFixed;
}
}
}
const elementsToCheck = getElementsToCheck( view );

for ( const element of elementsToCheck ) {
wasFixed = ensureProperElementName( element, mapper, writer ) || wasFixed;
}

// Selection in the view might not be updated to renamed elements. Happens mostly when other feature inserts paragraph to the table cell
// (ie. when deleting table cell contents) and sets selection to it while table-post fixer changes view <p> to <span> element.
// The view.selection would have outdated nodes.
if ( wasFixed ) {
updateRangesInViewSelection( model.document.selection, mapper, writer );
}

return wasFixed;
}

// Renames associated view element to a desired one. It will only rename if:
// - model element is a paragraph
// - view element is converted (mapped)
// - view element has different name then requested.
// Returns view elements changed in current view.change() block.
//
// @param {module:engine/model/element~Element} modelElement
// @param {String} desiredElementName
// @param {module:engine/view/writer~Writer} writer
// @param {module:engine/model/model~Model} model
// @param {module:engine/conversion/mapper~Mapper} mapper
function renameParagraphIfDifferent( modelElement, desiredElementName, writer, model, mapper ) {
// Only rename paragraph elements.
if ( !modelElement.is( 'paragraph' ) ) {
return false;
}
// **Note**: Currently it uses private property of the view: _renderer to get changed view elements to check.
//
// @param {module:engine/view/view~View} view
function getElementsToCheck( view ) {
const elementsWithChangedAttributes = Array.from( view._renderer.markedAttributes )
.filter( el => !!el.parent )
.filter( isSpanOrP )
.filter( el => isTdOrTh( el.parent ) );

const changedChildren = Array.from( view._renderer.markedChildren )
.filter( el => !!el.parent )
.filter( isTdOrTh )
.reduce( ( prev, element ) => {
const childrenToCheck = Array.from( element.getChildren() ).filter( isSpanOrP );

return [ ...prev, ...childrenToCheck ];
}, [] );

return [ ...elementsWithChangedAttributes, ...changedChildren ];
}

const viewElement = mapper.toViewElement( modelElement );
// This method checks if view element for model's <paragraph> was properly converter.
// Paragraph should be either
// - span: for single paragraph with no attributes.
// - p : in other cases.
function ensureProperElementName( currentViewElement, mapper, writer ) {
const modelParagraph = mapper.toModelElement( currentViewElement );
const expectedViewElementName = getExpectedElementName( modelParagraph.parent, modelParagraph );

// Only rename converted elements which aren't desired ones.
if ( !viewElement || viewElement.name === desiredElementName ) {
return false;
}
if ( currentViewElement.name !== expectedViewElementName ) {
// Unbind current view element as it should be cleared from mapper.
mapper.unbindViewElement( currentViewElement );

// After renaming element in the view by a post-fixer the selection would have references to the previous element.
const selection = model.document.selection;
const shouldFixSelection = checkSelectionForRenamedElement( selection, modelElement );
const renamedViewElement = writer.rename( expectedViewElementName, currentViewElement );

// Unbind current view element as it should be cleared from mapper.
mapper.unbindViewElement( viewElement );
const renamedViewElement = writer.rename( desiredElementName, viewElement );
// Bind paragraph inside table cell to the renamed view element.
mapper.bindElements( modelElement, renamedViewElement );
// Bind paragraph inside table cell to the renamed view element.
mapper.bindElements( modelParagraph, renamedViewElement );

if ( shouldFixSelection ) {
// Re-create view selection based on model selection.
updateRangesInViewSelection( selection, mapper, writer );
return true;
}

return true;
return false;
}

// Checks if model selection contains renamed element.
// Expected view element name depends on model elements:
// - <paragraph> with any attribute set should be rendered as <p>
// - all <paragraphs> in <tableCell> that has more then one children should be rendered as <p>
// - an only <paragraph> child with no attributes should be rendered as <span>
//
// @param {module:engine/model/selection~Selection} selection
// @param {module:engine/model/element~Element} modelElement
// @returns {boolean}
function checkSelectionForRenamedElement( selection, modelElement ) {
return !![ ...selection.getSelectedBlocks() ].find( block => block === modelElement );
// @param {module:engine/model/element~Element} tableCell
// @param {module:engine/model/element~Element} paragraph
// @returns {String}
function getExpectedElementName( tableCell, paragraph ) {
const isOnlyChild = tableCell.childCount > 1;
const hasAttributes = !![ ...paragraph.getAttributes() ].length;

return ( isOnlyChild || hasAttributes ) ? 'p' : 'span';
}

// Re-create view selection from model selection.
// Method to filter out <span> and <p> elements.
//
// @param {module:engine/model/selection~Selection} selection
// @param {module:engine/view/writer~Writer} writer
// @param {module:engine/conversion/mapper~Mapper} mapper
// @param {module:engine/view/element~Element} element
function isSpanOrP( element ) {
return element.is( 'p' ) || element.is( 'span' );
}

// Method to filter out <td> and <th> elements.
//
// @param {module:engine/view/element~Element} element
function isTdOrTh( element ) {
return element.is( 'td' ) || element.is( 'th' );
}

// Resets view selections based on model selection.
function updateRangesInViewSelection( selection, mapper, writer ) {
const fixedRanges = Array.from( selection.getRanges() )
.map( range => mapper.toViewRange( range ) );
Expand Down
4 changes: 3 additions & 1 deletion tests/_utils/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
} from '../../src/converters/downcast';
import upcastTable, { upcastTableCell } from '../../src/converters/upcasttable';

const WIDGET_TABLE_CELL_CLASS = 'ck-editor__editable ck-editor__nested-editable';

/**
* Returns a model representation of a table shorthand notation:
*
Expand Down Expand Up @@ -264,7 +266,7 @@ function makeRows( tableData, options ) {
const attributes = isObject ? tableCellData : {};

if ( asWidget ) {
attributes.class = 'ck-editor__editable ck-editor__nested-editable';
attributes.class = WIDGET_TABLE_CELL_CLASS + ( attributes.class ? ` ${ attributes.class }` : '' );
attributes.contenteditable = 'true';
}

Expand Down
Loading

0 comments on commit efc53c9

Please sign in to comment.