From b6948f0107061bafce98cdcf1282529bb61433ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Tue, 14 Aug 2018 17:56:30 +0200 Subject: [PATCH 1/7] Added writer for manipulating non-semantic view. --- src/view/rawwriter.js | 244 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 src/view/rawwriter.js diff --git a/src/view/rawwriter.js b/src/view/rawwriter.js new file mode 100644 index 000000000..de951d11d --- /dev/null +++ b/src/view/rawwriter.js @@ -0,0 +1,244 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module module:engine/view/rawwriter + */ + +import Element from './element'; +import { isPlainObject } from 'lodash-es'; + +/** + * View raw writer class. Provides set of methods used to properly manipulate nodes attached to + * {@link module:engine/view/view~View view instance}. It should be only used to manipulate non-semantic view + * (view created from HTML string). For view which was downcasted from the {@link module:engine/model/model~Model model} + * see {@link module:engine/view/writer~Writer writer}. + */ +export default class RawWriter { + /** + * Clones provided element. + * + * @see module:engine/view/element~Element#_clone + * @param {module:engine/view/element~Element} element Element to be cloned. + * @param {Boolean} [deep=false] If set to `true` clones element and all its children recursively. When set to `false`, + * element will be cloned without any children. + * @returns {module:engine/view/element~Element} Clone of this element. + */ + clone( element, deep = false ) { + return element._clone( deep ); + } + + /** + * Appends a child node or a list of child nodes at the end of this node + * and sets the parent of these nodes to this element. + * + * @see module:engine/view/element~Element#_appendChild + * @param {module:engine/view/element~Element} element Element to which items will be appended. + * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. + * @fires module:engine/view/node~Node#change + * @returns {Number} Number of appended nodes. + */ + appendChild( element, items ) { + return element._appendChild( items ); + } + + /** + * Inserts a child node or a list of child nodes on the given index and sets the parent of these nodes to + * this element. + * + * @see module:engine/view/element~Element#_insertChild + * @param {module:engine/view/element~Element} element Element to which items will be inserted. + * @param {Number} index Position where nodes should be inserted. + * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. + * @fires module:engine/view/node~Node#change + * @returns {Number} Number of inserted nodes. + */ + insertChild( element, index, items ) { + return element._insertChild( element, index, items ); + } + + /** + * Removes number of child nodes starting at the given index and set the parent of these nodes to `null`. + * + * @see module:engine/view/element~Element#_removeChildren + * @param {module:engine/view/element~Element} element Element from which children will be removed. + * @param {Number} index Number of the first node to remove. + * @param {Number} [howMany=1] Number of nodes to remove. + * @fires module:engine/view/node~Node#change + * @returns {Array.} The array of removed nodes. + */ + removeChildren( element, index, howMany = 1 ) { + return element._removeChildren( index, howMany ); + } + + /** + * Removes given element from the view structure. Will not have effect on detached elements. + * + * @param {module:engine/view/element~Element} element Element which will be removed. + * @returns {Array.|null} The array of removed nodes or null if element was already detached. + */ + remove( element ) { + const parent = element.parent; + if ( parent ) { + return this.removeChildren( parent, parent.getChildIndex( element ) ); + } + + return null; + } + + /** + * Replaces given element with the new one in the view structure. Will not have effect on detached elements. + * + * @param {module:engine/view/element~Element} oldElement Element which will be replaced. + * @param {module:engine/view/element~Element} newElement Element which will inserted in the place of the old element. + * @returns {Boolean} Whether old element was successfully replaced. + */ + replace( oldElement, newElement ) { + const parent = oldElement.parent; + + if ( parent ) { + const index = parent.getChildIndex( oldElement ); + + this.removeChildren( parent, index ); + this.insertChild( parent, index, newElement ); + } + + return null; + } + + /** + * Renames element by creating a copy of renamed element but with changed name and then moving contents of the + * old element to the new one. + * + * Since this function creates a new element and removes the given one, the new element is returned to keep reference. + * + * @param {module:engine/view/element~Element} element Element to be renamed. + * @param {String} newName New name for element. + * @returns {module:engine/view/element~Element} New element. + */ + rename( element, newName ) { + const newElement = new Element( newName, element.getAttributes(), element.getChildren() ); + + this.replace( element, newElement ); + + return newElement; + } + + /** + * Adds or overwrite element's attribute with a specified key and value. + * + * writer.setAttribute( 'href', 'http://ckeditor.com', linkElement ); + * + * @see module:engine/view/element~Element#_setAttribute + * @param {module:engine/view/element~Element} element + * @param {String} key Attribute key. + * @param {String} value Attribute value. + */ + setAttribute( element, key, value ) { + element._setAttribute( key, value ); + } + + /** + * Removes attribute from the element. + * + * writer.removeAttribute( 'href', linkElement ); + * + * @see module:engine/view/element~Element#_removeAttribute + * @param {module:engine/view/element~Element} element + * @param {String} key Attribute key. + */ + removeAttribute( element, key ) { + element._removeAttribute( key ); + } + + /** + * Adds specified class to the element. + * + * writer.addClass( 'foo', linkElement ); + * writer.addClass( [ 'foo', 'bar' ], linkElement ); + * + * @see module:engine/view/element~Element#_addClass + * @param {module:engine/view/element~Element} element + * @param {Array.|String} className + */ + addClass( element, className ) { + element._addClass( className ); + } + + /** + * Removes specified class from the element. + * + * writer.removeClass( 'foo', linkElement ); + * writer.removeClass( [ 'foo', 'bar' ], linkElement ); + * + * @see module:engine/view/element~Element#_removeClass + * @param {module:engine/view/element~Element} element + * @param {Array.|String} className + */ + removeClass( element, className ) { + element._removeClass( className ); + } + + /** + * Adds style to the element. + * + * writer.setStyle( 'color', 'red', element ); + * writer.setStyle( { + * color: 'red', + * position: 'fixed' + * }, element ); + * + * @see module:engine/view/element~Element#_setStyle + * @param {module:engine/view/element~Element} element + * @param {String|Object} property Property name or object with key - value pairs. + * @param {String} [value] Value to set. This parameter is ignored if object is provided as the first parameter. + */ + setStyle( element, property, value ) { + if ( isPlainObject( property ) && element === undefined ) { + element = value; + } + + element._setStyle( property, value ); + } + + /** + * Removes specified style from the element. + * + * writer.removeStyle( 'color', element ); // Removes 'color' style. + * writer.removeStyle( [ 'color', 'border-top' ], element ); // Removes both 'color' and 'border-top' styles. + * + * @see module:engine/view/element~Element#_removeStyle + * @param {module:engine/view/element~Element} element + * @param {Array.|String} property + */ + removeStyle( element, property ) { + element._removeStyle( property ); + } + + /** + * Sets a custom property on element. Unlike attributes, custom properties are not rendered to the DOM, + * so they can be used to add special data to elements. + * + * @see module:engine/view/element~Element#_setCustomProperty + * @param {module:engine/view/element~Element} element + * @param {String|Symbol} key + * @param {*} value + */ + setCustomProperty( element, key, value ) { + element._setCustomProperty( key, value ); + } + + /** + * Removes a custom property stored under the given key. + * + * @see module:engine/view/element~Element#_removeCustomProperty + * @param {module:engine/view/element~Element} element + * @param {String|Symbol} key + * @returns {Boolean} Returns true if property was removed. + */ + removeCustomProperty( element, key ) { + return element._removeCustomProperty( key ); + } +} From e098d3eb613d48f5393d43b3a80814536f4d8fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Tue, 14 Aug 2018 17:57:23 +0200 Subject: [PATCH 2/7] Tests: writer unit tests - WIP. --- tests/view/rawwriter.js | 97 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/view/rawwriter.js diff --git a/tests/view/rawwriter.js b/tests/view/rawwriter.js new file mode 100644 index 000000000..89e595860 --- /dev/null +++ b/tests/view/rawwriter.js @@ -0,0 +1,97 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import Element from '../../src/view/element'; +import RawWriter from '../../src/view/rawwriter'; +import HtmlDataProcessor from '../../src/dataprocessor/htmldataprocessor'; + +describe( 'RawWriter', () => { + let writer, view, dataprocessor; + + before( () => { + writer = new RawWriter(); + dataprocessor = new HtmlDataProcessor(); + } ); + + beforeEach( () => { + const html = '' + + '

Heading 1

' + + '

Foo Bar Bold

' + + '

Some underlined text

' + + '
    ' + + '
  • Item 1
  • ' + + '
  • Item 1
  • ' + + '
  • Item 1

  • ' + + '
'; + + view = dataprocessor.toView( html ); + } ); + + describe( 'clone', () => { + it( 'should clone simple element', () => { + const el = view.getChild( 0 ); + const clone = writer.clone( el ); + + expect( clone ).to.not.equal( el ); + expect( clone.isSimilar( el ) ).to.true; + expect( clone.childCount ).to.equal( 0 ); + } ); + + it( 'should clone element with all attributes', () => { + const el = view.getChild( 1 ); + const clone = writer.clone( el ); + + expect( clone ).to.not.equal( el ); + expect( clone.isSimilar( el ) ).to.true; + expect( clone.childCount ).to.equal( 0 ); + } ); + + it( 'should deep clone element', () => { + const el = view.getChild( 0 ); + const clone = writer.clone( el, true ); + + expect( clone ).to.not.equal( el ); + expect( clone.isSimilar( el ) ).to.true; + expect( clone.childCount ).to.equal( el.childCount ); + } ); + } ); + + describe( 'appendChild', () => { + it( 'should append inline child to paragraph', () => { + const el = view.getChild( 2 ); + const newChild = new Element( 'span' ); + + const appended = writer.appendChild( el, newChild ); + + expect( appended ).to.equal( 1 ); + expect( newChild.parent ).to.equal( el ); + expect( el.childCount ).to.equal( 3 ); + } ); + + it( 'should append block children to paragraph', () => { + const el = view.getChild( 2 ); + const newChild1 = new Element( 'p' ); + const newChild2 = new Element( 'h2' ); + + const appended = writer.appendChild( el, [ newChild1, newChild2 ] ); + + expect( appended ).to.equal( 2 ); + expect( newChild1.parent ).to.equal( el ); + expect( newChild2.parent ).to.equal( el ); + expect( el.childCount ).to.equal( 4 ); + } ); + + it( 'should append list item to the list', () => { + const el = view.getChild( 3 ); + const newChild = new Element( 'li' ); + + const appended = writer.appendChild( el, newChild ); + + expect( appended ).to.equal( 1 ); + expect( newChild.parent ).to.equal( el ); + expect( el.childCount ).to.equal( 4 ); + } ); + } ); +} ); From f75d8d64c26b3af25bf8deef53f7fe6da49b5a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Thu, 16 Aug 2018 17:17:32 +0200 Subject: [PATCH 3/7] Writer docs rewording and code adjustments. --- src/view/rawwriter.js | 53 +++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/view/rawwriter.js b/src/view/rawwriter.js index de951d11d..68b3f9383 100644 --- a/src/view/rawwriter.js +++ b/src/view/rawwriter.js @@ -8,7 +8,6 @@ */ import Element from './element'; -import { isPlainObject } from 'lodash-es'; /** * View raw writer class. Provides set of methods used to properly manipulate nodes attached to @@ -35,7 +34,8 @@ export default class RawWriter { * and sets the parent of these nodes to this element. * * @see module:engine/view/element~Element#_appendChild - * @param {module:engine/view/element~Element} element Element to which items will be appended. + * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} element Element + * to which items will be appended. * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. * @fires module:engine/view/node~Node#change * @returns {Number} Number of appended nodes. @@ -49,21 +49,23 @@ export default class RawWriter { * this element. * * @see module:engine/view/element~Element#_insertChild - * @param {module:engine/view/element~Element} element Element to which items will be inserted. + * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} element Element + * to which items will be inserted. * @param {Number} index Position where nodes should be inserted. * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. * @fires module:engine/view/node~Node#change * @returns {Number} Number of inserted nodes. */ insertChild( element, index, items ) { - return element._insertChild( element, index, items ); + return element._insertChild( index, items ); } /** * Removes number of child nodes starting at the given index and set the parent of these nodes to `null`. * * @see module:engine/view/element~Element#_removeChildren - * @param {module:engine/view/element~Element} element Element from which children will be removed. + * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} element Element + * from which children will be removed. * @param {Number} index Number of the first node to remove. * @param {Number} [howMany=1] Number of nodes to remove. * @fires module:engine/view/node~Node#change @@ -77,7 +79,7 @@ export default class RawWriter { * Removes given element from the view structure. Will not have effect on detached elements. * * @param {module:engine/view/element~Element} element Element which will be removed. - * @returns {Array.|null} The array of removed nodes or null if element was already detached. + * @returns {Array.} The array of removed nodes. */ remove( element ) { const parent = element.parent; @@ -85,7 +87,7 @@ export default class RawWriter { return this.removeChildren( parent, parent.getChildIndex( element ) ); } - return null; + return []; } /** @@ -103,9 +105,11 @@ export default class RawWriter { this.removeChildren( parent, index ); this.insertChild( parent, index, newElement ); + + return true; } - return null; + return false; } /** @@ -116,20 +120,19 @@ export default class RawWriter { * * @param {module:engine/view/element~Element} element Element to be renamed. * @param {String} newName New name for element. - * @returns {module:engine/view/element~Element} New element. + * @returns {module:engine/view/element~Element|null} New element or null if the old element + * was not replaced (happens for detached elements). */ rename( element, newName ) { const newElement = new Element( newName, element.getAttributes(), element.getChildren() ); - this.replace( element, newElement ); - - return newElement; + return this.replace( element, newElement ) ? newElement : null; } /** * Adds or overwrite element's attribute with a specified key and value. * - * writer.setAttribute( 'href', 'http://ckeditor.com', linkElement ); + * writer.setAttribute( linkElement, 'href', 'http://ckeditor.com' ); * * @see module:engine/view/element~Element#_setAttribute * @param {module:engine/view/element~Element} element @@ -143,7 +146,7 @@ export default class RawWriter { /** * Removes attribute from the element. * - * writer.removeAttribute( 'href', linkElement ); + * writer.removeAttribute( linkElement, 'href' ); * * @see module:engine/view/element~Element#_removeAttribute * @param {module:engine/view/element~Element} element @@ -156,8 +159,8 @@ export default class RawWriter { /** * Adds specified class to the element. * - * writer.addClass( 'foo', linkElement ); - * writer.addClass( [ 'foo', 'bar' ], linkElement ); + * writer.addClass( linkElement, 'foo' ); + * writer.addClass( linkElement, [ 'foo', 'bar' ] ); * * @see module:engine/view/element~Element#_addClass * @param {module:engine/view/element~Element} element @@ -170,8 +173,8 @@ export default class RawWriter { /** * Removes specified class from the element. * - * writer.removeClass( 'foo', linkElement ); - * writer.removeClass( [ 'foo', 'bar' ], linkElement ); + * writer.removeClass( linkElement, 'foo' ); + * writer.removeClass( linkElement, [ 'foo', 'bar' ] ); * * @see module:engine/view/element~Element#_removeClass * @param {module:engine/view/element~Element} element @@ -184,11 +187,11 @@ export default class RawWriter { /** * Adds style to the element. * - * writer.setStyle( 'color', 'red', element ); - * writer.setStyle( { + * writer.setStyle( element, 'color', 'red' ); + * writer.setStyle( element, { * color: 'red', * position: 'fixed' - * }, element ); + * } ); * * @see module:engine/view/element~Element#_setStyle * @param {module:engine/view/element~Element} element @@ -196,18 +199,14 @@ export default class RawWriter { * @param {String} [value] Value to set. This parameter is ignored if object is provided as the first parameter. */ setStyle( element, property, value ) { - if ( isPlainObject( property ) && element === undefined ) { - element = value; - } - element._setStyle( property, value ); } /** * Removes specified style from the element. * - * writer.removeStyle( 'color', element ); // Removes 'color' style. - * writer.removeStyle( [ 'color', 'border-top' ], element ); // Removes both 'color' and 'border-top' styles. + * writer.removeStyle( element, 'color' ); // Removes 'color' style. + * writer.removeStyle( element, [ 'color', 'border-top' ] ); // Removes both 'color' and 'border-top' styles. * * @see module:engine/view/element~Element#_removeStyle * @param {module:engine/view/element~Element} element From 1cfe049bf81d2bb1e6abca3f0b63f679aa028a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Thu, 16 Aug 2018 17:23:30 +0200 Subject: [PATCH 4/7] Tests: writer unit tests - 100% cc. --- tests/view/rawwriter.js | 388 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 386 insertions(+), 2 deletions(-) diff --git a/tests/view/rawwriter.js b/tests/view/rawwriter.js index 89e595860..f6c7eb319 100644 --- a/tests/view/rawwriter.js +++ b/tests/view/rawwriter.js @@ -17,11 +17,11 @@ describe( 'RawWriter', () => { beforeEach( () => { const html = '' + - '

Heading 1

' + + '

Heading 1

' + '

Foo Bar Bold

' + '

Some underlined text

' + '
    ' + - '
  • Item 1
  • ' + + '
  • Item 1
  • ' + '
  • Item 1
  • ' + '
  • Item 1

  • ' + '
'; @@ -93,5 +93,389 @@ describe( 'RawWriter', () => { expect( newChild.parent ).to.equal( el ); expect( el.childCount ).to.equal( 4 ); } ); + + it( 'should append element to DocumentFragment element', () => { + const newChild = new Element( 'p' ); + + const appended = writer.appendChild( view, newChild ); + + expect( appended ).to.equal( 1 ); + expect( newChild.parent ).to.equal( view ); + expect( view.childCount ).to.equal( 5 ); + } ); + } ); + + describe( 'insertChild', () => { + it( 'should insert inline child into the paragraph on the first position', () => { + const el = view.getChild( 2 ); + const newChild = new Element( 'span' ); + + const inserted = writer.insertChild( el, 0, newChild ); + + expect( inserted ).to.equal( 1 ); + expect( newChild.parent ).to.equal( el ); + expect( el.getChild( 0 ) ).to.equal( newChild ); + expect( el.childCount ).to.equal( 3 ); + } ); + + it( 'should insert block children into the paragraph on the last position', () => { + const el = view.getChild( 2 ); + const newChild1 = new Element( 'blockquote' ); + const newChild2 = new Element( 'h2' ); + + const inserted = writer.insertChild( el, 2, [ newChild1, newChild2 ] ); + + expect( inserted ).to.equal( 2 ); + expect( newChild1.parent ).to.equal( el ); + expect( newChild2.parent ).to.equal( el ); + expect( el.getChild( 2 ) ).to.equal( newChild1 ); + expect( el.getChild( 3 ) ).to.equal( newChild2 ); + expect( el.childCount ).to.equal( 4 ); + } ); + + it( 'should insert list item into the list element', () => { + const el = view.getChild( 3 ); + const newChild = new Element( 'li' ); + + const inserted = writer.insertChild( el, 1, newChild ); + + expect( inserted ).to.equal( 1 ); + expect( newChild.parent ).to.equal( el ); + expect( el.getChild( 1 ) ).to.equal( newChild ); + expect( el.childCount ).to.equal( 4 ); + } ); + + it( 'should insert element to DocumentFragment element', () => { + const newChild = new Element( 'p' ); + + const inserted = writer.insertChild( view, 4, newChild ); + + expect( inserted ).to.equal( 1 ); + expect( newChild.parent ).to.equal( view ); + expect( view.getChild( 4 ) ).to.equal( newChild ); + expect( view.childCount ).to.equal( 5 ); + } ); + } ); + + describe( 'removeChildren', () => { + it( 'should remove child from the beginning of the paragraph', () => { + const el = view.getChild( 1 ); + const toRemove = el.getChild( 0 ); + + const removed = writer.removeChildren( el, 0 ); + + expect( removed.length ).to.equal( 1 ); + expect( removed[ 0 ] ).to.equal( toRemove ); + expect( el.childCount ).to.equal( 3 ); + } ); + + it( 'should remove two last list items from the list element', () => { + const el = view.getChild( 3 ); + const toRemove1 = el.getChild( 1 ); + const toRemove2 = el.getChild( 2 ); + + const removed = writer.removeChildren( el, 1, 2 ); + + expect( removed.length ).to.equal( 2 ); + expect( removed[ 0 ] ).to.equal( toRemove1 ); + expect( removed[ 1 ] ).to.equal( toRemove2 ); + expect( el.childCount ).to.equal( 1 ); + } ); + + it( 'should remove child from DocumentFragment element', () => { + const toRemove = view.getChild( 2 ); + + const removed = writer.removeChildren( view, 2 ); + + expect( removed.length ).to.equal( 1 ); + expect( removed[ 0 ] ).to.equal( toRemove ); + expect( view.childCount ).to.equal( 3 ); + } ); + } ); + + describe( 'remove', () => { + it( 'should remove list item from the list element', () => { + const toRemove = view.getChild( 3 ).getChild( 1 ); + + const removed = writer.remove( toRemove ); + + expect( removed.length ).to.equal( 1 ); + expect( removed[ 0 ] ).to.equal( toRemove ); + expect( view.getChild( 3 ).childCount ).to.equal( 2 ); + } ); + + it( 'should have no effect on detached elements', () => { + const newChild = new Element( 'h2' ); + + const removed = writer.remove( newChild ); + + expect( removed.length ).to.equal( 0 ); + expect( view.childCount ).to.equal( 4 ); + } ); + + it( 'should remove direct root (DocumentFragment) child', () => { + const toRemove = view.getChild( 3 ); + + const removed = writer.remove( toRemove ); + + expect( removed.length ).to.equal( 1 ); + expect( removed[ 0 ] ).to.equal( toRemove ); + expect( view.childCount ).to.equal( 3 ); + } ); + } ); + + describe( 'replace', () => { + it( 'should replace single element', () => { + const el = view.getChild( 0 ).getChild( 1 ); + const newChild = new Element( 'span' ); + + const replacement = writer.replace( el, newChild ); + + expect( replacement ).to.true; + expect( view.getChild( 0 ).getChild( 1 ) ).to.equal( newChild ); + expect( view.getChild( 0 ).childCount ).to.equal( 2 ); + } ); + + it( 'should replace element with children', () => { + const el = view.getChild( 3 ); + const newChild = new Element( 'ol' ); + + const replacement = writer.replace( el, newChild ); + + expect( replacement ).to.true; + expect( view.getChild( 3 ) ).to.equal( newChild ); + expect( view.childCount ).to.equal( 4 ); + } ); + + it( 'should have no effect on detached elements', () => { + const oldChild = new Element( 'h2' ); + const newChild = new Element( 'h2' ); + + const replacement = writer.replace( oldChild, newChild ); + + expect( replacement ).to.false; + expect( view.childCount ).to.equal( 4 ); + } ); + } ); + + describe( 'rename', () => { + it( 'should rename simple element', () => { + const el = view.getChild( 0 ).getChild( 1 ); + + const renamed = writer.rename( el, 'i' ); + + expect( renamed ).to.not.equal( el ); + expect( renamed ).to.equal( view.getChild( 0 ).getChild( 1 ) ); + expect( renamed.name ).to.equal( 'i' ); + expect( view.getChild( 0 ).childCount ).to.equal( 2 ); + } ); + + it( 'should rename direct root (DocumentFragment) child element', () => { + const el = view.getChild( 1 ); + + const renamed = writer.rename( el, 'h3' ); + + expect( renamed ).to.not.equal( el ); + expect( renamed ).to.equal( view.getChild( 1 ) ); + expect( renamed.name ).to.equal( 'h3' ); + expect( view.childCount ).to.equal( 4 ); + } ); + + it( 'should have no effect on detached element', () => { + const element = new Element( 'h2' ); + + const renamed = writer.rename( element, 'h3' ); + + expect( renamed ).to.null; + expect( view.childCount ).to.equal( 4 ); + } ); + } ); + + describe( 'setAttribute', () => { + it( 'should add new attribute', () => { + const el = view.getChild( 0 ); + + writer.setAttribute( el, 'testAttr', 'testVal' ); + + expect( el.getAttribute( 'testAttr' ) ).to.equal( 'testVal' ); + } ); + + it( 'should update existing attribute', () => { + const el = view.getChild( 1 ); + + writer.setAttribute( el, 'data-attr', 'foo' ); + + expect( el.getAttribute( 'data-attr' ) ).to.equal( 'foo' ); + } ); + } ); + + describe( 'removeAttribute', () => { + it( 'should remove existing attribute', () => { + const el = view.getChild( 1 ); + + writer.removeAttribute( el, 'data-attr' ); + + expect( el.hasAttribute( 'data-attr' ) ).to.false; + } ); + + it( 'should have no effect if attribute does not exists', () => { + const el = view.getChild( 0 ); + + writer.removeAttribute( el, 'non-existent' ); + + expect( el.hasAttribute( 'non-existent' ) ).to.false; + } ); + } ); + + describe( 'addClass', () => { + it( 'should add new classes if no classes', () => { + const el = view.getChild( 2 ); + + writer.addClass( el, [ 'foo', 'bar' ] ); + + expect( el.hasClass( 'foo' ) ).to.true; + expect( el.hasClass( 'bar' ) ).to.true; + expect( Array.from( el.getClassNames() ).length ).to.equal( 2 ); + } ); + + it( 'should add new class to existing classes', () => { + const el = view.getChild( 1 ); + + writer.addClass( el, 'newClass' ); + + expect( el.hasClass( 'newClass' ) ).to.true; + expect( Array.from( el.getClassNames() ).length ).to.equal( 3 ); + } ); + } ); + + describe( 'removeClass', () => { + it( 'should remove existing class', () => { + const el = view.getChild( 3 ).getChild( 0 ); + + writer.removeClass( el, 'single' ); + + expect( el.hasClass( 'single' ) ).to.false; + expect( Array.from( el.getClassNames() ).length ).to.equal( 0 ); + } ); + + it( 'should remove existing class from many classes', () => { + const el = view.getChild( 1 ); + + writer.removeClass( el, 'foo1' ); + + expect( el.hasClass( 'foo1' ) ).to.false; + expect( el.hasClass( 'bar2' ) ).to.true; + expect( Array.from( el.getClassNames() ).length ).to.equal( 1 ); + } ); + + it( 'should have no effect if there are no classes', () => { + const el = view.getChild( 0 ); + + writer.removeClass( el, 'non-existent' ); + + expect( el.hasClass( 'non-existent' ) ).to.false; + expect( Array.from( el.getClassNames() ).length ).to.equal( 0 ); + } ); + } ); + + describe( 'setStyle', () => { + it( 'should add new style', () => { + const el = view.getChild( 2 ); + + writer.setStyle( el, { + color: 'red', + position: 'fixed' + } ); + + expect( el.getStyle( 'color' ) ).to.equal( 'red' ); + expect( el.getStyle( 'position' ) ).to.equal( 'fixed' ); + expect( Array.from( el.getStyleNames() ).length ).to.equal( 2 ); + } ); + + it( 'should update existing styles', () => { + const el = view.getChild( 1 ); + + writer.setStyle( el, 'text-align', 'center' ); + + expect( el.getStyle( 'text-align' ) ).to.equal( 'center' ); + expect( Array.from( el.getStyleNames() ).length ).to.equal( 1 ); + } ); + } ); + + describe( 'removeStyle', () => { + it( 'should remove existing style', () => { + const el = view.getChild( 0 ); + + writer.removeStyle( el, [ 'color', 'position' ] ); + + expect( el.hasStyle( 'color' ) ).to.false; + expect( el.hasStyle( 'position' ) ).to.false; + expect( Array.from( el.getStyleNames() ).length ).to.equal( 0 ); + } ); + + it( 'should remove value from existing styles', () => { + const el = view.getChild( 0 ); + + writer.removeStyle( el, 'position' ); + + expect( el.hasStyle( 'color' ) ).to.true; + expect( el.hasStyle( 'position' ) ).to.false; + expect( Array.from( el.getStyleNames() ).length ).to.equal( 1 ); + } ); + + it( 'should have no effect if styles does not exists', () => { + const el = view.getChild( 2 ); + + writer.removeStyle( el, [ 'color', 'position' ] ); + + expect( el.hasStyle( 'color' ) ).to.false; + expect( el.hasStyle( 'position' ) ).to.false; + expect( Array.from( el.getStyleNames() ).length ).to.equal( 0 ); + } ); + } ); + + describe( 'setCustomProperty', () => { + it( 'should add or update custom property', () => { + const el = new Element( 'span' ); + + writer.setCustomProperty( el, 'prop1', 'foo' ); + writer.setCustomProperty( el, 'prop2', 'bar' ); + + expect( el.getCustomProperty( 'prop1' ) ).to.equal( 'foo' ); + expect( el.getCustomProperty( 'prop2' ) ).to.equal( 'bar' ); + expect( Array.from( el.getCustomProperties() ).length ).to.equal( 2 ); + + const objectProperty = { foo: 'bar' }; + writer.setCustomProperty( el, 'prop2', objectProperty ); + + expect( el.getCustomProperty( 'prop1' ) ).to.equal( 'foo' ); + expect( el.getCustomProperty( 'prop2' ) ).to.equal( objectProperty ); + expect( Array.from( el.getCustomProperties() ).length ).to.equal( 2 ); + } ); + } ); + + describe( 'removeCustomProperty', () => { + it( 'should remove existing custom property', () => { + const el = new Element( 'p' ); + + writer.setCustomProperty( el, 'prop1', 'foo' ); + + expect( el.getCustomProperty( 'prop1' ) ).to.equal( 'foo' ); + expect( Array.from( el.getCustomProperties() ).length ).to.equal( 1 ); + + writer.removeCustomProperty( el, 'prop1' ); + + expect( el.getCustomProperty( 'prop1' ) ).to.undefined; + expect( Array.from( el.getCustomProperties() ).length ).to.equal( 0 ); + } ); + + it( 'should have no effect if custom property does not exists', () => { + const el = new Element( 'h1' ); + + writer.removeCustomProperty( el, 'prop1' ); + + expect( el.getCustomProperty( 'prop1' ) ).to.undefined; + expect( Array.from( el.getCustomProperties() ).length ).to.equal( 0 ); + } ); } ); } ); From d0ea328a1d7e9ed14a51b00de00839ea76b3aefd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 27 Aug 2018 11:09:04 +0200 Subject: [PATCH 5/7] Renamed 'RawWriter' to 'UpcastWriter'. --- src/view/{rawwriter.js => upcastwriter.js} | 6 +++--- tests/view/{rawwriter.js => upcastwriter.js} | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) rename src/view/{rawwriter.js => upcastwriter.js} (97%) rename tests/view/{rawwriter.js => upcastwriter.js} (99%) diff --git a/src/view/rawwriter.js b/src/view/upcastwriter.js similarity index 97% rename from src/view/rawwriter.js rename to src/view/upcastwriter.js index 68b3f9383..a8ad45bd0 100644 --- a/src/view/rawwriter.js +++ b/src/view/upcastwriter.js @@ -4,18 +4,18 @@ */ /** - * @module module:engine/view/rawwriter + * @module module:engine/view/upcastwriter */ import Element from './element'; /** - * View raw writer class. Provides set of methods used to properly manipulate nodes attached to + * View upcast writer class. Provides set of methods used to properly manipulate nodes attached to * {@link module:engine/view/view~View view instance}. It should be only used to manipulate non-semantic view * (view created from HTML string). For view which was downcasted from the {@link module:engine/model/model~Model model} * see {@link module:engine/view/writer~Writer writer}. */ -export default class RawWriter { +export default class UpcastWriter { /** * Clones provided element. * diff --git a/tests/view/rawwriter.js b/tests/view/upcastwriter.js similarity index 99% rename from tests/view/rawwriter.js rename to tests/view/upcastwriter.js index f6c7eb319..33c9e4f9b 100644 --- a/tests/view/rawwriter.js +++ b/tests/view/upcastwriter.js @@ -4,14 +4,14 @@ */ import Element from '../../src/view/element'; -import RawWriter from '../../src/view/rawwriter'; +import UpcastWriter from '../../src/view/upcastwriter'; import HtmlDataProcessor from '../../src/dataprocessor/htmldataprocessor'; -describe( 'RawWriter', () => { +describe( 'UpcastWriter', () => { let writer, view, dataprocessor; before( () => { - writer = new RawWriter(); + writer = new UpcastWriter(); dataprocessor = new HtmlDataProcessor(); } ); From 9cc372950dc114479bfda8e0f813d69054e1ebe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 27 Aug 2018 15:47:06 +0200 Subject: [PATCH 6/7] Docs rewording. --- src/view/upcastwriter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/upcastwriter.js b/src/view/upcastwriter.js index a8ad45bd0..6dbf0c458 100644 --- a/src/view/upcastwriter.js +++ b/src/view/upcastwriter.js @@ -13,7 +13,7 @@ import Element from './element'; * View upcast writer class. Provides set of methods used to properly manipulate nodes attached to * {@link module:engine/view/view~View view instance}. It should be only used to manipulate non-semantic view * (view created from HTML string). For view which was downcasted from the {@link module:engine/model/model~Model model} - * see {@link module:engine/view/writer~Writer writer}. + * see {@link module:engine/view/downcastwriter~DowncastWriter downcast writer}. */ export default class UpcastWriter { /** From ac4b0cb5469377ed7bcabab4c770cd8cd639f17a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Fri, 31 Aug 2018 09:31:17 +0200 Subject: [PATCH 7/7] Params order refactoring. --- src/view/upcastwriter.js | 83 ++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/src/view/upcastwriter.js b/src/view/upcastwriter.js index 6dbf0c458..2085c4b09 100644 --- a/src/view/upcastwriter.js +++ b/src/view/upcastwriter.js @@ -34,13 +34,13 @@ export default class UpcastWriter { * and sets the parent of these nodes to this element. * * @see module:engine/view/element~Element#_appendChild + * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} element Element * to which items will be appended. - * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. - * @fires module:engine/view/node~Node#change + * @fires module:engine/view/node~Node#event:change * @returns {Number} Number of appended nodes. */ - appendChild( element, items ) { + appendChild( items, element ) { return element._appendChild( items ); } @@ -49,14 +49,14 @@ export default class UpcastWriter { * this element. * * @see module:engine/view/element~Element#_insertChild + * @param {Number} index Offset at which nodes should be inserted. + * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} element Element * to which items will be inserted. - * @param {Number} index Position where nodes should be inserted. - * @param {module:engine/view/item~Item|Iterable.} items Items to be inserted. - * @fires module:engine/view/node~Node#change + * @fires module:engine/view/node~Node#event:change * @returns {Number} Number of inserted nodes. */ - insertChild( element, index, items ) { + insertChild( index, items, element ) { return element._insertChild( index, items ); } @@ -64,14 +64,14 @@ export default class UpcastWriter { * Removes number of child nodes starting at the given index and set the parent of these nodes to `null`. * * @see module:engine/view/element~Element#_removeChildren + * @param {Number} index Offset from which nodes will be removed. + * @param {Number} howMany Number of nodes to remove. * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} element Element - * from which children will be removed. - * @param {Number} index Number of the first node to remove. - * @param {Number} [howMany=1] Number of nodes to remove. - * @fires module:engine/view/node~Node#change - * @returns {Array.} The array of removed nodes. + * which children will be removed. + * @fires module:engine/view/node~Node#event:change + * @returns {Array.} The array containing removed nodes. */ - removeChildren( element, index, howMany = 1 ) { + removeChildren( index, howMany, element ) { return element._removeChildren( index, howMany ); } @@ -79,10 +79,11 @@ export default class UpcastWriter { * Removes given element from the view structure. Will not have effect on detached elements. * * @param {module:engine/view/element~Element} element Element which will be removed. - * @returns {Array.} The array of removed nodes. + * @returns {Array.} The array containing removed nodes. */ remove( element ) { const parent = element.parent; + if ( parent ) { return this.removeChildren( parent, parent.getChildIndex( element ) ); } @@ -94,7 +95,7 @@ export default class UpcastWriter { * Replaces given element with the new one in the view structure. Will not have effect on detached elements. * * @param {module:engine/view/element~Element} oldElement Element which will be replaced. - * @param {module:engine/view/element~Element} newElement Element which will inserted in the place of the old element. + * @param {module:engine/view/element~Element} newElement Element which will be inserted in the place of the old element. * @returns {Boolean} Whether old element was successfully replaced. */ replace( oldElement, newElement ) { @@ -113,33 +114,33 @@ export default class UpcastWriter { } /** - * Renames element by creating a copy of renamed element but with changed name and then moving contents of the + * Renames element by creating a copy of a given element but with its name changed and then moving contents of the * old element to the new one. * * Since this function creates a new element and removes the given one, the new element is returned to keep reference. * + * @param {String} newName New element name. * @param {module:engine/view/element~Element} element Element to be renamed. - * @param {String} newName New name for element. * @returns {module:engine/view/element~Element|null} New element or null if the old element * was not replaced (happens for detached elements). */ - rename( element, newName ) { + rename( newName, element ) { const newElement = new Element( newName, element.getAttributes(), element.getChildren() ); return this.replace( element, newElement ) ? newElement : null; } /** - * Adds or overwrite element's attribute with a specified key and value. + * Adds or overwrites element's attribute with a specified key and value. * * writer.setAttribute( linkElement, 'href', 'http://ckeditor.com' ); * * @see module:engine/view/element~Element#_setAttribute - * @param {module:engine/view/element~Element} element * @param {String} key Attribute key. * @param {String} value Attribute value. + * @param {module:engine/view/element~Element} element Element for which attribute will be set. */ - setAttribute( element, key, value ) { + setAttribute( key, value, element ) { element._setAttribute( key, value ); } @@ -149,10 +150,10 @@ export default class UpcastWriter { * writer.removeAttribute( linkElement, 'href' ); * * @see module:engine/view/element~Element#_removeAttribute - * @param {module:engine/view/element~Element} element * @param {String} key Attribute key. + * @param {module:engine/view/element~Element} element Element from which attribute will be removed. */ - removeAttribute( element, key ) { + removeAttribute( key, element ) { element._removeAttribute( key ); } @@ -163,10 +164,10 @@ export default class UpcastWriter { * writer.addClass( linkElement, [ 'foo', 'bar' ] ); * * @see module:engine/view/element~Element#_addClass - * @param {module:engine/view/element~Element} element - * @param {Array.|String} className + * @param {Array.|String} className Single class name or array of class names which will be added. + * @param {module:engine/view/element~Element} element Element for which class will be added. */ - addClass( element, className ) { + addClass( className, element ) { element._addClass( className ); } @@ -177,10 +178,10 @@ export default class UpcastWriter { * writer.removeClass( linkElement, [ 'foo', 'bar' ] ); * * @see module:engine/view/element~Element#_removeClass - * @param {module:engine/view/element~Element} element - * @param {Array.|String} className + * @param {Array.|String} className Single class name or array of class names which will be removed. + * @param {module:engine/view/element~Element} element Element from which class will be removed. */ - removeClass( element, className ) { + removeClass( className, element ) { element._removeClass( className ); } @@ -194,11 +195,11 @@ export default class UpcastWriter { * } ); * * @see module:engine/view/element~Element#_setStyle - * @param {module:engine/view/element~Element} element * @param {String|Object} property Property name or object with key - value pairs. * @param {String} [value] Value to set. This parameter is ignored if object is provided as the first parameter. + * @param {module:engine/view/element~Element} element Element for which style will be added. */ - setStyle( element, property, value ) { + setStyle( property, value, element ) { element._setStyle( property, value ); } @@ -209,10 +210,10 @@ export default class UpcastWriter { * writer.removeStyle( element, [ 'color', 'border-top' ] ); // Removes both 'color' and 'border-top' styles. * * @see module:engine/view/element~Element#_removeStyle - * @param {module:engine/view/element~Element} element - * @param {Array.|String} property + * @param {Array.|String} property Style property name or names to be removed. + * @param {module:engine/view/element~Element} element Element from which style will be removed. */ - removeStyle( element, property ) { + removeStyle( property, element ) { element._removeStyle( property ); } @@ -221,11 +222,11 @@ export default class UpcastWriter { * so they can be used to add special data to elements. * * @see module:engine/view/element~Element#_setCustomProperty - * @param {module:engine/view/element~Element} element - * @param {String|Symbol} key - * @param {*} value + * @param {String|Symbol} key Custom property name/key. + * @param {*} value Custom property value to be stored. + * @param {module:engine/view/element~Element} element Element for which custom property will be set. */ - setCustomProperty( element, key, value ) { + setCustomProperty( key, value, element ) { element._setCustomProperty( key, value ); } @@ -233,11 +234,11 @@ export default class UpcastWriter { * Removes a custom property stored under the given key. * * @see module:engine/view/element~Element#_removeCustomProperty - * @param {module:engine/view/element~Element} element - * @param {String|Symbol} key + * @param {String|Symbol} key Name/key of the custom property to be removed. + * @param {module:engine/view/element~Element} element Element from which the custom property will be removed. * @returns {Boolean} Returns true if property was removed. */ - removeCustomProperty( element, key ) { + removeCustomProperty( key, element ) { return element._removeCustomProperty( key ); } }