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

Improved placeholder helpers when indirect host. Added tests for placeholder helpers.

Aleksander Nowodzinski 7 лет назад
Родитель
Сommit
ad34950d89

+ 15 - 4
packages/ckeditor5-engine/src/view/placeholder.js

@@ -65,11 +65,17 @@ export function disablePlaceholder( view, element ) {
 	const doc = element.document;
 
 	view.change( writer => {
-		if ( documentPlaceholders.has( doc ) ) {
-			documentPlaceholders.get( doc ).delete( element );
+		if ( !documentPlaceholders.has( doc ) ) {
+			return;
 		}
 
-		hidePlaceholder( writer, element );
+		const placeholders = documentPlaceholders.get( doc );
+		const config = placeholders.get( element );
+
+		writer.removeAttribute( 'data-placeholder', config.hostElement );
+		hidePlaceholder( writer, config.hostElement );
+
+		placeholders.delete( element );
 	} );
 }
 
@@ -206,6 +212,11 @@ function updatePlaceholder( writer, element, config ) {
 		return false;
 	}
 
+	// Cache the host element. It will be necessary for disablePlaceholder() to know
+	// which element should have class and attribute removed because, depending on
+	// the config.isDirectHost value, it could be the element or one of its descendants.
+	config.hostElement = hostElement;
+
 	// This may be necessary when updating the placeholder text to something else.
 	if ( hostElement.getAttribute( 'data-placeholder' ) !== text ) {
 		writer.setAttribute( 'data-placeholder', text, hostElement );
@@ -234,7 +245,7 @@ function getChildPlaceholderHostSubstitute( parent ) {
 	if ( parent.childCount === 1 ) {
 		const firstChild = parent.getChild( 0 );
 
-		if ( firstChild.is( 'element' ) ) {
+		if ( firstChild.is( 'element' ) && !firstChild.is( 'uiElement' ) ) {
 			return firstChild;
 		}
 	}

+ 243 - 35
packages/ckeditor5-engine/tests/view/placeholder.js

@@ -3,7 +3,13 @@
  * For licensing, see LICENSE.md.
  */
 
-import { addPlaceholder, removePlaceholder } from '../../src/view/placeholder';
+import {
+	enablePlaceholder,
+	disablePlaceholder,
+	showPlaceholder,
+	hidePlaceholder,
+	needsPlaceholder
+} from '../../src/view/placeholder';
 import createViewRoot from './_utils/createroot';
 import View from '../../src/view/view';
 import ViewRange from '../../src/view/range';
@@ -19,22 +25,45 @@ describe( 'placeholder', () => {
 		viewDocument.isFocused = true;
 	} );
 
-	describe( 'addPlaceholder', () => {
+	describe( 'enablePlaceholder', () => {
 		it( 'should attach proper CSS class and data attribute', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
+		it( 'should attach proper CSS class and data attribute (isDirectHost=false)', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+		} );
+
 		it( 'if element has children set only data attribute', () => {
 			setData( view, '<div>first div</div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -44,7 +73,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div><ui:span></ui:span><ui:span></ui:span></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -54,7 +87,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div>[]</div><div>another div</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -65,34 +102,25 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 			viewDocument.isFocused = false;
 
-			addPlaceholder( view, element, 'foo bar baz' );
-
-			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
-		} );
-
-		it( 'use check function if one is provided', () => {
-			setData( view, '<div></div><div>{another div}</div>' );
-			const element = viewRoot.getChild( 0 );
-			let result = true;
-			const spy = sinon.spy( () => result );
-
-			addPlaceholder( view, element, 'foo bar baz', spy );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
-			sinon.assert.called( spy );
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
-
-			result = false;
-			view.render();
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'should remove CSS class if selection is moved inside', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -108,8 +136,17 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
-			addPlaceholder( view, element, 'new text' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
+
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'new text'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'new text' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -119,7 +156,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 			setData( view, '<p>paragraph</p>' );
 
 			view.render();
@@ -136,8 +177,17 @@ describe( 'placeholder', () => {
 			setData( secondView, '<div></div><div>{another div}</div>' );
 			const secondElement = secondRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'first placeholder' );
-			addPlaceholder( secondView, secondElement, 'second placeholder' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'first placeholder'
+			} );
+
+			enablePlaceholder( {
+				view: secondView,
+				element: secondElement,
+				text: 'second placeholder'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -165,7 +215,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			view.change( writer => {
 				writer.setSelection( ViewRange._createIn( element ) );
@@ -178,19 +232,53 @@ describe( 'placeholder', () => {
 			// After rendering - placeholder should be invisible since selection is moved there.
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
+
+		it( 'should not set attributes/class when multiple children (isDirectHost=false)', () => {
+			setData( view, '<p></p><p></p>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
+
+		it( 'should not set attributes/class when first child is not element (isDirectHost=false)', () => {
+			setData( view, '<ui:span></ui:span>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
 	} );
 
-	describe( 'removePlaceholder', () => {
+	describe( 'disablePlaceholder', () => {
 		it( 'should remove placeholder from element', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			addPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
-			removePlaceholder( view, element );
+			disablePlaceholder( view, element );
 
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -200,10 +288,130 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			removePlaceholder( view, element );
+			disablePlaceholder( view, element );
 
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
+
+		it( 'should remove placeholder from element (isDirectHost=false)', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+
+			disablePlaceholder( view, viewRoot );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
+	} );
+
+	describe( 'showPlaceholder', () => {
+		it( 'should add the ck-placholder class if an element does not have it', () => {
+			setData( view, '<div></div>' );
+			const element = viewRoot.getChild( 0 );
+
+			const result = view.change( writer => showPlaceholder( writer, element ) );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
+			expect( result ).to.be.true;
+		} );
+
+		it( 'should do nothing if an element already has the ck-placeholder class', () => {
+			setData( view, '<div class="ck-placeholder"></div>' );
+			const element = viewRoot.getChild( 0 );
+			let spy;
+
+			const result = view.change( writer => {
+				spy = sinon.spy( writer, 'addClass' );
+
+				return showPlaceholder( writer, element );
+			} );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
+			expect( result ).to.be.false;
+			sinon.assert.notCalled( spy );
+		} );
+	} );
+
+	describe( 'hidePlaceholder', () => {
+		it( 'should remove the ck-placholder class if an element has it', () => {
+			setData( view, '<div class="ck-placeholder"></div>' );
+			const element = viewRoot.getChild( 0 );
+
+			const result = view.change( writer => hidePlaceholder( writer, element ) );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( result ).to.be.true;
+		} );
+
+		it( 'should do nothing if an element has no ck-placeholder class', () => {
+			setData( view, '<div></div>' );
+			const element = viewRoot.getChild( 0 );
+			let spy;
+
+			const result = view.change( writer => {
+				spy = sinon.spy( writer, 'removeClass' );
+
+				return hidePlaceholder( writer, element );
+			} );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( result ).to.be.false;
+			sinon.assert.notCalled( spy );
+		} );
+	} );
+
+	describe( 'needsPlaceholder', () => {
+		it( 'should return false if element was removed from the document', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+
+			view.change( writer => {
+				writer.remove( element );
+			} );
+
+			expect( needsPlaceholder( element ) ).to.be.false;
+		} );
+
+		it( 'should return true if element is empty and document is blurred', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+		} );
+
+		it( 'should return true if element hosts UI elements only and document is blurred', () => {
+			setData( view, '<p><ui:span></ui:span></p>' );
+			viewDocument.isFocused = false;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+		} );
+
+		it( 'should return true when document is focused but selection anchored somewhere else', () => {
+			setData( view, '<p></p><p>{moo}</p>' );
+			viewDocument.isFocused = true;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+		} );
 	} );
 } );