Merge pull request #17 from ckeditor/t/16

Fix: Paragraph command should correctly update its `value` and `isEnabled` properties. Closes #16.

src/paragraphcommand.js

@@ -8,6 +8,8 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command/command';
+import Position from '@ckeditor/ckeditor5-engine/src/model/position';
+import first from '@ckeditor/ckeditor5-utils/src/first';
 
 /**
  * The paragraph command.
@@ -34,7 +36,10 @@ export default class ParagraphCommand extends Command {
 		this.set( 'value', false );
 
 		// Update current value each time changes are done on document.
-		this.listenTo( editor.document, 'changesDone', () => this._updateValue() );
+		this.listenTo( editor.document, 'changesDone', () => {
+			this.refreshValue();
+			this.refreshState();
+		} );
 	}
 
 	/**
@@ -65,14 +70,22 @@ export default class ParagraphCommand extends Command {
 
 	/**
 	 * Updates command's {@link #value value} based on current selection.
-	 *
-	 * @private
 	 */
-	_updateValue() {
-		const block = this.editor.document.selection.getSelectedBlocks().next().value;
+	refreshValue() {
+		const block = first( this.editor.document.selection.getSelectedBlocks() );
+
+		this.value = !!block && block.is( 'paragraph' );
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	_checkEnabled() {
+		const block = first( this.editor.document.selection.getSelectedBlocks() );
 
-		if ( block ) {
-			this.value = block.is( 'paragraph' );
-		}
+		return !!block && this.editor.document.schema.check( {
+			name: 'paragraph',
+			inside: Position.createBefore( block )
+		} );
 	}
 }

tests/paragraphcommand.js

@@ -23,6 +23,10 @@ describe( 'ParagraphCommand', () => {
 			editor.commands.set( 'paragraph', command );
 			schema.registerItem( 'paragraph', '$block' );
 			schema.registerItem( 'heading1', '$block' );
+
+			schema.registerItem( 'notBlock' );
+			schema.allow( { name: 'notBlock', inside: '$root' } );
+			schema.allow( { name: '$text', inside: 'notBlock' } );
 		} );
 	} );
 
@@ -32,8 +36,6 @@ describe( 'ParagraphCommand', () => {
 
 	describe( 'value', () => {
 		it( 'has default value', () => {
-			setData( document, '' );
-
 			expect( command.value ).to.be.false;
 		} );
 
@@ -61,6 +63,35 @@ describe( 'ParagraphCommand', () => {
 			setData( document, '<paragraph>[bar</paragraph><heading1>foo]</heading1>' );
 			expect( command.value ).to.be.true;
 		} );
+
+		it( 'has proper value when inside non-block element', () => {
+			setData( document, '<notBlock>[foo]</notBlock>' );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'has proper value when moved from block to element that is not a block', () => {
+			setData( document, '<paragraph>[foo]</paragraph><notBlock>foo</notBlock>' );
+			const element = document.getRoot().getChild( 1 );
+
+			document.enqueueChanges( () => {
+				document.selection.setRanges( [ Range.createIn( element ) ] );
+			} );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'should be refreshed after calling refreshValue()', () => {
+			setData( document, '<paragraph>[foo]</paragraph><notBlock>foo</notBlock>' );
+			const element = document.getRoot().getChild( 1 );
+
+			// Purposely not putting it in `document.enqueueChanges` to update command manually.
+			document.selection.setRanges( [ Range.createIn( element ) ] );
+
+			expect( command.value ).to.be.true;
+			command.refreshValue();
+			expect( command.value ).to.be.false;
+		} );
 	} );
 
 	describe( '_doExecute', () => {
@@ -147,4 +178,24 @@ describe( 'ParagraphCommand', () => {
 			} );
 		} );
 	} );
+
+	describe( 'isEnabled', () => {
+		it( 'should be enabled when inside another block', () => {
+			setData( document, '<heading1>f{}oo</heading1>' );
+
+			expect( command.isEnabled ).to.be.true;
+		} );
+
+		it( 'should be disabled if inside non-block', () => {
+			setData( document, '<notBlock>f{}oo</notBlock>' );
+
+			expect( command.isEnabled ).to.be.false;
+		} );
+
+		it( 'should be disabled if selection is placed on non-block element', () => {
+			setData( document, '[<notBlock>foo</notBlock>]' );
+
+			expect( command.isEnabled ).to.be.false;
+		} );
+	} );
 } );