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 #967 from ckeditor/t/799
Browse files Browse the repository at this point in the history
Feature: UIElement has it's own render method used by DomConverter and can create DOM children. Improved integration with observers and other view elements. Closes #799.
  • Loading branch information
Piotr Jasiun authored Jun 14, 2017
2 parents 9176877 + 157ca15 commit 7fc52ea
Show file tree
Hide file tree
Showing 16 changed files with 501 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/controller/editingcontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export default class EditingController {
* stop listening on {@link #destroy}.
*
* @private
* @member {utils.EmitterMixin} #_listenter
* @member {utils.EmitterMixin} #_listener
*/
this._listener = Object.create( EmitterMixin );

Expand Down
14 changes: 11 additions & 3 deletions src/view/attributeelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,24 +101,32 @@ AttributeElement.DEFAULT_PRIORITY = DEFAULT_PRIORITY;
// @returns {Number|null} Block filler offset or `null` if block filler is not needed.
function getFillerOffset() {
// <b>foo</b> does not need filler.
if ( this.childCount ) {
if ( nonUiChildrenCount( this ) ) {
return null;
}

let element = this.parent;

// <p><b></b></p> needs filler -> <p><b><br></b></p>
while ( element && element.is( 'attributeElement' ) ) {
if ( element.childCount > 1 ) {
if ( nonUiChildrenCount( element ) > 1 ) {
return null;
}

element = element.parent;
}

if ( !element || element.childCount > 1 ) {
if ( !element || nonUiChildrenCount( element ) > 1 ) {
return null;
}

return 0;
}

// Returns total count of children that are not {@link module:engine/view/uielement~UIElement UIElements}.
//
// @param {module:engine/view/element~Element} element
// @return {Number}
function nonUiChildrenCount( element ) {
return Array.from( element.getChildren() ).filter( element => !element.is( 'uiElement' ) ).length;
}
2 changes: 1 addition & 1 deletion src/view/containerelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ export default class ContainerElement extends Element {
//
// @returns {Number|null} Block filler offset or `null` if block filler is not needed.
function getFillerOffset() {
return this.childCount === 0 ? 0 : null;
return Array.from( this.getChildren() ).some( element => !element.is( 'uiElement' ) ) ? null : 0;
}
64 changes: 63 additions & 1 deletion src/view/domconverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,16 @@ export default class DomConverter {
if ( options.bind ) {
this.bindDocumentFragments( domElement, viewNode );
}
} else if ( viewNode.is( 'uiElement' ) ) {
// UIElement has it's own render() method.
// https://github.com/ckeditor/ckeditor5-engine/issues/799
domElement = viewNode.render( domDocument );

if ( options.bind ) {
this.bindElements( domElement, viewNode );
}

return domElement;
} else {
// Create DOM element.
domElement = domDocument.createElement( viewNode.name );
Expand Down Expand Up @@ -339,6 +349,7 @@ export default class DomConverter {
* Converts DOM to view. For all text nodes, not bound elements and document fragments new items will
* be created. For bound elements and document fragments function will return corresponding items. For
* {@link module:engine/view/filler fillers} `null` will be returned.
* For all DOM elements rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned.
*
* @param {Node|DocumentFragment} domNode DOM node or document fragment to transform.
* @param {Object} [options] Conversion options.
Expand All @@ -353,6 +364,13 @@ export default class DomConverter {
return null;
}

// When node is inside UIElement return that UIElement as it's view representation.
const uiElement = this.getParentUIElement( domNode, this._domToViewMapping );

if ( uiElement ) {
return uiElement;
}

if ( this.isText( domNode ) ) {
if ( isInlineFiller( domNode ) ) {
return null;
Expand Down Expand Up @@ -488,6 +506,9 @@ export default class DomConverter {
* If the position is inside a {@link module:engine/view/filler filler} which has no corresponding view node,
* position of the filler will be converted and returned.
*
* If the position is inside DOM element rendered by {@link module:engine/view/uielement~UIElement}
* that position will be converted to view position before that UIElement.
*
* If structures are too different and it is not possible to find corresponding position then `null` will be returned.
*
* @param {Node} domParent DOM position parent.
Expand All @@ -499,6 +520,12 @@ export default class DomConverter {
return this.domPositionToView( domParent.parentNode, indexOf( domParent ) );
}

// If position is somewhere inside UIElement - return position before that element.
const viewElement = this.getCorrespondingViewElement( domParent );
if ( viewElement && viewElement.is( 'uiElement' ) ) {
return ViewPosition.createBefore( viewElement );
}

if ( this.isText( domParent ) ) {
if ( isInlineFiller( domParent ) ) {
return this.domPositionToView( domParent.parentNode, indexOf( domParent ) );
Expand Down Expand Up @@ -567,12 +594,13 @@ export default class DomConverter {
/**
* Gets corresponding view element. Returns element if an view element was
* {@link module:engine/view/domconverter~DomConverter#bindElements bound} to the given DOM element or `null` otherwise.
* For all DOM elements rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned.
*
* @param {HTMLElement} domElement DOM element.
* @returns {module:engine/view/element~Element|null} Corresponding element or `null` if no element was bound.
*/
getCorrespondingViewElement( domElement ) {
return this._domToViewMapping.get( domElement );
return this.getParentUIElement( domElement ) || this._domToViewMapping.get( domElement );
}

/**
Expand All @@ -597,6 +625,8 @@ export default class DomConverter {
* If this is a first child in the parent and the parent is a {@link module:engine/view/domconverter~DomConverter#bindElements bound}
* element, it is used to find the corresponding text node.
*
* For all text nodes rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned.
*
* Otherwise `null` is returned.
*
* Note that for the block or inline {@link module:engine/view/filler filler} this method returns `null`.
Expand All @@ -610,6 +640,13 @@ export default class DomConverter {
return null;
}

// If DOM text was rendered by UIElement - return that element.
const uiElement = this.getParentUIElement( domText );

if ( uiElement ) {
return uiElement;
}

const previousSibling = domText.previousSibling;

// Try to use previous sibling to find the corresponding text node.
Expand Down Expand Up @@ -832,6 +869,31 @@ export default class DomConverter {
return backward;
}

/**
* Returns parent {@link module:engine/view/uielement~UIElement} for provided DOM node. Returns null if there is no
* parent UIElement.
*
* @param {Node} domNode
* @return {module:engine/view/uielement~UIElement|null}
*/
getParentUIElement( domNode ) {
const ancestors = getAncestors( domNode );

// Remove domNode from the list.
ancestors.pop();

while ( ancestors.length ) {
const domNode = ancestors.pop();
const viewNode = this._domToViewMapping.get( domNode );

if ( viewNode && viewNode.is( 'uiElement' ) ) {
return viewNode;
}
}

return null;
}

/**
* Takes text data from given {@link module:engine/view/text~Text#data} and processes it so it is correctly displayed in DOM.
*
Expand Down
12 changes: 12 additions & 0 deletions src/view/observer/mutationobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ export default class MutationObserver extends Observer {
if ( mutation.type === 'childList' ) {
const element = domConverter.getCorrespondingViewElement( mutation.target );

// Do not collect mutations from UIElements.
if ( element && element.is( 'uiElement' ) ) {
continue;
}

if ( element && !this._isBogusBrMutation( mutation ) ) {
mutatedElements.add( element );
}
Expand All @@ -157,6 +162,13 @@ export default class MutationObserver extends Observer {

// Handle `characterData` mutations later, when we have the full list of nodes which changed structure.
for ( const mutation of domMutations ) {
const element = domConverter.getCorrespondingViewElement( mutation.target );

// Do not collect mutations from UIElements.
if ( element && element.is( 'uiElement' ) ) {
continue;
}

if ( mutation.type === 'characterData' ) {
const text = domConverter.getCorrespondingViewText( mutation.target );

Expand Down
17 changes: 17 additions & 0 deletions src/view/uielement.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,23 @@ export default class UIElement extends Element {
throw new CKEditorError( 'view-uielement-cannot-add: Cannot add child nodes to UIElement instance.' );
}
}

/**
* Renders this {@link module:engine/view/uielement~UIElement} to DOM. This method is called by
* {@link engine/view/domconverter~DomConverter}.
*
* @param {Document} domDocument
* @return {HTMLElement}
*/
render( domDocument ) {
const domElement = domDocument.createElement( this.name );

for ( const key of this.getAttributeKeys() ) {
domElement.setAttribute( key, this.getAttribute( key ) );
}

return domElement;
}
}

// Returns `null` because block filler is not needed for UIElements.
Expand Down
15 changes: 15 additions & 0 deletions tests/view/attributeelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,20 @@ describe( 'AttributeElement', () => {

expect( attribute.getFillerOffset() ).to.be.null;
} );

it( 'should return position 0 if it is the only nested element in the container and has UIElement inside', () => {
const { selection } = parse(
'<container:p><attribute:b><attribute:i>[]<ui:span></ui:span></attribute:i></attribute:b></container:p>' );
const attribute = selection.getFirstPosition().parent;

expect( attribute.getFillerOffset() ).to.equals( 0 );
} );

it( 'should return 0 if there is no parent container element and has UIElement inside', () => {
const { selection } = parse( '<attribute:b>[]<ui:span></ui:span></attribute:b>' );
const attribute = selection.getFirstPosition().parent;

expect( attribute.getFillerOffset() ).to.equal( 0 );
} );
} );
} );
4 changes: 4 additions & 0 deletions tests/view/containerelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ describe( 'ContainerElement', () => {
expect( parse( '<container:p></container:p>' ).getFillerOffset() ).to.equals( 0 );
} );

it( 'should return position 0 if element contains only ui elements', () => {
expect( parse( '<container:p><ui:span></ui:span></container:p>' ).getFillerOffset() ).to.equals( 0 );
} );

it( 'should return null if element is not empty', () => {
expect( parse( '<container:p>foo</container:p>' ).getFillerOffset() ).to.be.null;
} );
Expand Down
50 changes: 50 additions & 0 deletions tests/view/domconverter/binding.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,4 +365,54 @@ describe( 'DomConverter', () => {
expect( bindSelection.isEqual( selectionCopy ) ).to.be.true;
} );
} );

describe( 'unbindDomElement', () => {
it( 'should unbind elements', () => {
const domElement = document.createElement( 'p' );
const viewElement = new ViewElement( 'p' );

converter.bindElements( domElement, viewElement );

expect( converter.getCorrespondingView( domElement ) ).to.equal( viewElement );
expect( converter.getCorrespondingDom( viewElement ) ).to.equal( domElement );

converter.unbindDomElement( domElement );

expect( converter.getCorrespondingView( domElement ) ).to.be.undefined;
expect( converter.getCorrespondingDom( viewElement ) ).to.be.undefined;
} );

it( 'should unbind element\'s child nodes', () => {
const domElement = document.createElement( 'p' );
const domChild = document.createElement( 'span' );
domElement.appendChild( domChild );

const viewElement = new ViewElement( 'p' );
const viewChild = new ViewElement( 'span' );

converter.bindElements( domElement, viewElement );
converter.bindElements( domChild, viewChild );

expect( converter.getCorrespondingView( domChild ) ).to.equal( viewChild );
expect( converter.getCorrespondingDom( viewChild ) ).to.equal( domChild );

converter.unbindDomElement( domElement );

expect( converter.getCorrespondingView( domChild ) ).to.be.undefined;
expect( converter.getCorrespondingDom( viewChild ) ).to.be.undefined;
} );

it( 'should do nothing if there are no elements bind', () => {
const domElement = document.createElement( 'p' );
const viewElement = new ViewElement( 'p' );

expect( converter.getCorrespondingView( domElement ) ).to.be.undefined;
expect( converter.getCorrespondingDom( viewElement ) ).to.be.undefined;

converter.unbindDomElement( domElement );

expect( converter.getCorrespondingView( domElement ) ).to.be.undefined;
expect( converter.getCorrespondingDom( viewElement ) ).to.be.undefined;
} );
} );
} );
Loading

0 comments on commit 7fc52ea

Please sign in to comment.