8
0
Просмотр исходного кода

Writer keeps consitent hierarchy when wrapping.

Szymon Kupś 8 лет назад
Родитель
Сommit
8eb75dfb76

+ 38 - 3
packages/ckeditor5-engine/src/view/element.js

@@ -673,11 +673,35 @@ export default class Element extends Node {
 		yield* this._customProperties.entries();
 	}
 
+	/**
+	 * Returns identity string based on element's name, styles, classes and other attributes.
+	 * Two elements that {@link #isSimilar are similar} will have same identity string.
+	 * It has following format:
+	 *
+	 *		"name|classes(class1,class2,class3)|styles(style1=val1,style2=val2)|attributes(attr1=val1,attr2=val2)"
+ 	 *
+	 * For example:
+	 *
+	 *		const element = new ViewElement( 'foo' );
+	 *		element.setAttribute( 'banana', '10' );
+	 *		element.setAttribute( 'apple', '20' );
+	 *		element.setStyle( 'color', 'red' );
+	 *		element.setStyle( 'border-color', 'white' );
+	 *		element.addClass( 'baz' );
+	 *
+	 *		// returns "foo|classes(baz)|styles(border-color=white,color=red)|attributes(apple=20,banana=10)"
+	 *		element.getIdentity();
+	 *
+	 * NOTE: Classes, styles and other attributes are sorted alphabetically.
+	 *
+	 * @returns {String}
+	 */
 	getIdentity() {
-		const attributes = Array.from( this._attrs ).map( i => i[ 0 ] + '=' + i[ 1 ] ).sort();
-		const styles = Array.from( this._styles ).map( i => i[ 0 ] + '=' + i[ 1 ] ).sort();
+		const classes = Array.from( this._classes ).sort().join( ',' );
+		const attributes = mapToSortedString( this._attrs );
+		const styles = mapToSortedString( this._styles );
 
-		return [ this.name, ...attributes, ...styles ].join( '|' );
+		return `${ this.name }|classes(${ classes })|styles(${ styles })|attributes(${ attributes })`;
 	}
 
 	/**
@@ -795,3 +819,14 @@ function normalize( nodes ) {
 			return typeof node == 'string' ? new Text( node ) : node;
 		} );
 }
+
+// Returns string representation of povided map in following format:
+//
+//		"mapKey1=value1,mapKey2=value2,mapKey3=value3"
+//
+// NOTE: All map keys should be strings. Key-value pairs will be sorted.
+//
+// @returns {String}
+function mapToSortedString( map ) {
+	return Array.from( map ).map( i => i[ 0 ] + '=' + i[ 1 ] ).sort().join( ',' );
+}

+ 14 - 2
packages/ckeditor5-engine/src/view/writer.js

@@ -933,6 +933,7 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 		const isUI = child.is( 'uiElement' );
 
 		// Wrap text, empty elements, ui elements or attributes with higher or equal priority.
+		// if ( isText || isEmpty || isUI || ( isAttribute && attribute.priority <= child.priority ) ) {
 		if ( isText || isEmpty || isUI || ( isAttribute && shouldABeOutsideB( attribute, child ) ) ) {
 			// Clone attribute.
 			const newAttribute = attribute.clone();
@@ -975,6 +976,14 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 	return Range.createFromParentsAndOffsets( parent, startOffset, parent, endOffset );
 }
 
+// Checks if first {@link module:engine/view/attributeelement~AttributeElement AttributeElement} provided to the function
+// can be wrapped otuside second element. It is done by comparing elements'
+// {@link module:engine/view/attributeelement~AttributeElement#priority priorities}, if both have same priority
+// {@link module:engine/view/element~Element#getIdentity identities} are compared.
+//
+// @param {module:engine/view/attributeelement~AttributeElement} a
+// @param {module:engine/view/attributeelement~AttributeElement} b
+// @returns {Boolean}
 function shouldABeOutsideB( a, b ) {
 	if ( a.priority < b.priority ) {
 		return true;
@@ -982,8 +991,11 @@ function shouldABeOutsideB( a, b ) {
 		return false;
 	}
 
-	// When priorities are equal.
-	console.log( a.getIdentity() );
+	if ( a.name == b.name ) {
+		return true;
+	}
+
+	// When priorities are equal use identities.
 	return a.getIdentity() < b.getIdentity();
 }
 

+ 27 - 6
packages/ckeditor5-engine/tests/view/element.js

@@ -986,7 +986,22 @@ describe( 'Element', () => {
 		it( 'should return only name if no other attributes are present', () => {
 			const el = new Element( 'foo' );
 
-			expect( el.getIdentity() ).to.equal( 'foo' );
+			expect( el.getIdentity() ).to.equal( 'foo|classes()|styles()|attributes()' );
+		} );
+
+		it( 'should return classes in sorted order', () => {
+			const el = new Element( 'fruit' );
+			el.addClass( 'banana', 'lemon', 'apple' );
+
+			expect( el.getIdentity() ).to.equal( 'fruit|classes(apple,banana,lemon)|styles()|attributes()' );
+		} );
+
+		it( 'should return styles in sorted order', () => {
+			const el = new Element( 'foo', {
+				style: 'border: 1px solid red; background-color: red'
+			} );
+
+			expect( el.getIdentity() ).to.equal( 'foo|classes()|styles(background-color=red,border=1px solid red)|attributes()' );
 		} );
 
 		it( 'should return attributes in sorted order', () => {
@@ -996,15 +1011,21 @@ describe( 'Element', () => {
 				b: 3
 			} );
 
-			expect( el.getIdentity() ).to.equal( 'foo|a=1|b=3|d=4' );
+			expect( el.getIdentity() ).to.equal( 'foo|classes()|styles()|attributes(a=1,b=3,d=4)' );
 		} );
 
-		it( 'should return styles in sorted order', () => {
-			const el = new Element( 'foo', {
-				style: 'border: 1px solid red; background-color: red'
+		it( 'should return classes, styles and attributes', () => {
+			const el = new Element( 'baz', {
+				foo: 'one',
+				bar: 'two',
+				style: 'text-align:center;border-radius:10px'
 			} );
 
-			expect( el.getIdentity() ).to.equal( 'foo|background-color=red|border=1px solid red' );
+			el.addClass( 'three', 'two', 'one' );
+
+			expect( el.getIdentity() ).to.equal(
+				'baz|classes(one,three,two)|styles(border-radius=10px,text-align=center)|attributes(bar=two,foo=one)'
+			);
 		} );
 	} );
 } );

+ 22 - 0
packages/ckeditor5-engine/tests/view/writer/wrap.js

@@ -318,5 +318,27 @@ describe( 'writer', () => {
 				wrap( range, new AttributeElement( 'b' ) );
 			} ).to.throw( CKEditorError, 'view-writer-cannot-break-ui-element' );
 		} );
+
+		it( 'should keep stable hierarchy when wrapping with attribute with same priority', () => {
+			test(
+				'<container:p>[<attribute:span>foo</attribute:span>]</container:p>',
+				'<attribute:b></attribute:b>',
+				'<container:p>' +
+					'[<attribute:b view-priority="10">' +
+						'<attribute:span view-priority="10">foo</attribute:span>' +
+					'</attribute:b>]' +
+				'</container:p>'
+			);
+
+			test(
+				'<container:p>[<attribute:b>foo</attribute:b>]</container:p>',
+				'<attribute:span></attribute:span>',
+				'<container:p>' +
+					'[<attribute:b view-priority="10">' +
+						'<attribute:span view-priority="10">foo</attribute:span>' +
+					'</attribute:b>]' +
+				'</container:p>'
+			);
+		} );
 	} );
 } );

+ 5 - 3
packages/ckeditor5-engine/tests/view/writer/wrapposition.js

@@ -101,9 +101,11 @@ describe( 'wrapPosition', () => {
 			'<container:p><attribute:b view-priority="1">foo{}bar</attribute:b></container:p>',
 			'<attribute:u view-priority="1"></attribute:u>',
 			'<container:p>' +
-				'<attribute:b view-priority="1">foo</attribute:b>' +
-				'<attribute:u view-priority="1"><attribute:b view-priority="1">[]</attribute:b></attribute:u>' +
-				'<attribute:b view-priority="1">bar</attribute:b>' +
+				'<attribute:b view-priority="1">' +
+					'foo' +
+					'<attribute:u view-priority="1">[]</attribute:u>' +
+					'bar' +
+				'</attribute:b>' +
 			'</container:p>'
 		);
 	} );