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

Fix: The value of the AttributeCommand is taken from the first allowed node.

Oskar Wróbel 7 лет назад
Родитель
Сommit
d26fa2b0c2

+ 37 - 2
packages/ckeditor5-basic-styles/src/attributecommand.js

@@ -41,7 +41,7 @@ export default class AttributeCommand extends Command {
 		 * Flag indicating whether the command is active. The command is active when the
 		 * {@link module:engine/model/selection~Selection#hasAttribute selection has the attribute} which means that:
 		 *
-		 * * If the selection is not empty – That it starts in a text (or another node) which has the attribute set.
+		 * * If the selection is not empty – That the first node in the selection that allows the attribute has the attribute set.
 		 * * If the selection is empty – That the selection has the attribute itself (which means that newly typed
 		 * text will have this attribute, too).
 		 *
@@ -58,7 +58,7 @@ export default class AttributeCommand extends Command {
 		const model = this.editor.model;
 		const doc = model.document;
 
-		this.value = doc.selection.hasAttribute( this.attributeKey );
+		this.value = this._getValueFromFirstAllowedNode();
 		this.isEnabled = model.schema.checkAttributeInSelection( doc.selection, this.attributeKey );
 	}
 
@@ -108,4 +108,39 @@ export default class AttributeCommand extends Command {
 			}
 		} );
 	}
+
+	/**
+	 * Checks the attribute value of the first node in the selection that allows the attribute.
+	 * For the collapsed selection returns the selection attribute.
+	 *
+	 * **Note** Selection direction is taken into consideration.
+	 *
+	 * @private
+	 * @returns {Boolean} The attribute value.
+	 */
+	_getValueFromFirstAllowedNode() {
+		const model = this.editor.model;
+		const schema = model.schema;
+		const selection = model.document.selection;
+
+		if ( selection.isCollapsed ) {
+			return selection.hasAttribute( this.attributeKey );
+		}
+
+		const ranges = Array.from( selection.getRanges() );
+
+		if ( selection.isBackward ) {
+			ranges.reverse();
+		}
+
+		for ( const range of ranges ) {
+			for ( const item of range.getItems( { direction: selection.isBackward ? 'backward' : 'forward' } ) ) {
+				if ( schema.checkAttribute( item, this.attributeKey ) ) {
+					return item.hasAttribute( this.attributeKey );
+				}
+			}
+		}
+
+		return false;
+	}
 }

+ 52 - 5
packages/ckeditor5-basic-styles/tests/attributecommand.js

@@ -49,7 +49,7 @@ describe( 'AttributeCommand', () => {
 	} );
 
 	describe( 'value', () => {
-		it( 'is true when selection has the attribute', () => {
+		it( 'is true when collapsed selection has the attribute', () => {
 			model.change( writer => {
 				writer.setSelectionAttribute( attrKey, true );
 			} );
@@ -57,7 +57,7 @@ describe( 'AttributeCommand', () => {
 			expect( command.value ).to.be.true;
 		} );
 
-		it( 'is false when selection does not have the attribute', () => {
+		it( 'is false when collapsed selection does not have the attribute', () => {
 			model.change( writer => {
 				writer.setSelectionAttribute( attrKey, true );
 			} );
@@ -69,8 +69,55 @@ describe( 'AttributeCommand', () => {
 			expect( command.value ).to.be.false;
 		} );
 
-		// See https://github.com/ckeditor/ckeditor5-core/issues/73#issuecomment-311572827.
-		it( 'is false when selection contains object with nested editable', () => {
+		it( 'is true when the first item that allows attribute has the attribute set - forward #1', () => {
+			setData( model, '<p><$text bold="true">fo[o</$text></p><h1>b]ar</h1>' );
+
+			expect( command.value ).to.be.true;
+		} );
+
+		it( 'is true when the first item that allows attribute has the attribute set - forward #2', () => {
+			setData( model, '<h1>fo[o</h1><p><$text bold="true">f</$text>o]o</p>' );
+
+			expect( command.value ).to.be.true;
+		} );
+
+		it( 'is false when the first item that allows attribute has not the attribute set - forward #1', () => {
+			setData( model, '<p>b[a<$text bold="true">r</$text></p><h1>fo]o</h1>' );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'is false when the first item that allows attribute has not the attribute set - forward #2', () => {
+			setData( model, '<h1>fo[o</h1><p>b<$text bold="true">r</$text>r]</p>' );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'is true when the first item that allows attribute has the attribute set - backward #1', () => {
+			setData( model, '<h1>fo[o</h1><p>b<$text bold="true">ar]</$text></p>', { lastRangeBackward: true } );
+
+			expect( command.value ).to.be.true;
+		} );
+
+		it( 'is true when the first item that allows attribute has the attribute set - backward #2', () => {
+			setData( model, '<p>b[a<$text bold="true">r</$text></p><h1>fo]o</h1>', { lastRangeBackward: true } );
+
+			expect( command.value ).to.be.true;
+		} );
+
+		it( 'is false when the first item that allows attribute has not the attribute set - backward #1', () => {
+			setData( model, '<h1>fo[o</h1><p>b<$text bold="true">a</$text>r]</p>', { lastRangeBackward: true } );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'is false when the first item that allows attribute has not the attribute set - backward #2', () => {
+			setData( model, '<p>f[<$text bold="true">o</$text>o</p><h1>ba]r</h1>', { lastRangeBackward: true } );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'is true when the first item that allows attribute has the attribute set - object with nested editable', () => {
 			model.schema.register( 'caption', {
 				allowContentOf: '$block',
 				allowIn: 'img',
@@ -85,7 +132,7 @@ describe( 'AttributeCommand', () => {
 
 			expect( command.value ).to.be.false;
 			command.execute();
-			expect( command.value ).to.be.false;
+			expect( command.value ).to.be.true;
 
 			expect( getData( model ) ).to.equal(
 				'<p>[<img><caption><$text bold="true">Some caption inside the image.</$text></caption></img>]</p>'