Przeglądaj źródła

Merge pull request #1391 from ckeditor/t/ckeditor5-image/198

Internal: Placeholder is removing `ck` CSS class only if it was added by that placeholder. Closes https://github.com/ckeditor/ckeditor5-image/issues/198.
Piotrek Koszuliński 7 lat temu
rodzic
commit
1701f4c7b9

+ 26 - 13
packages/ckeditor5-engine/src/view/placeholder.js

@@ -34,7 +34,14 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
 	}
 
 	// Store information about element with placeholder.
-	documentPlaceholders.get( document ).set( element, { placeholderText, checkFunction } );
+	documentPlaceholders.get( document ).set( element, {
+		placeholderText,
+		checkFunction,
+
+		// Whether to remove the ck class when detaching the placeholder.
+		// https://github.com/ckeditor/ckeditor5-image/issues/198#issuecomment-377542222
+		removeCkClass: !element.hasClass( 'ck' )
+	} );
 
 	// Update view right away.
 	view.render();
@@ -47,14 +54,20 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
  * @param {module:engine/view/element~Element} element
  */
 export function detachPlaceholder( view, element ) {
-	const document = element.document;
-
-	if ( documentPlaceholders.has( document ) ) {
-		documentPlaceholders.get( document ).delete( element );
-	}
+	const doc = element.document;
 
 	view.change( writer => {
-		writer.removeClass( [ 'ck', 'ck-placeholder' ], element );
+		if ( documentPlaceholders.has( doc ) ) {
+			const info = documentPlaceholders.get( doc ).get( element );
+
+			if ( info.removeCkClass ) {
+				writer.removeClass( 'ck', element );
+			}
+
+			documentPlaceholders.get( doc ).delete( element );
+		}
+
+		writer.removeClass( 'ck-placeholder', element );
 		writer.removeAttribute( 'data-placeholder', element );
 	} );
 }
@@ -105,8 +118,8 @@ function updateSinglePlaceholder( writer, element, info ) {
 
 	// If checkFunction is provided and returns false - remove placeholder.
 	if ( checkFunction && !checkFunction() ) {
-		if ( element.hasClass( 'ck', 'ck-placeholder' ) ) {
-			writer.removeClass( [ 'ck', 'ck-placeholder' ], element );
+		if ( element.hasClass( 'ck-placeholder' ) ) {
+			writer.removeClass( 'ck-placeholder', element );
 			changed = true;
 		}
 
@@ -119,7 +132,7 @@ function updateSinglePlaceholder( writer, element, info ) {
 
 	// If element is empty and editor is blurred.
 	if ( !document.isFocused && isEmptyish ) {
-		if ( !element.hasClass( 'ck', 'ck-placeholder' ) ) {
+		if ( !element.hasClass( 'ck-placeholder' ) ) {
 			writer.addClass( [ 'ck', 'ck-placeholder' ], element );
 			changed = true;
 		}
@@ -129,13 +142,13 @@ function updateSinglePlaceholder( writer, element, info ) {
 
 	// It there are no child elements and selection is not placed inside element.
 	if ( isEmptyish && anchor && anchor.parent !== element ) {
-		if ( !element.hasClass( 'ck', 'ck-placeholder' ) ) {
+		if ( !element.hasClass( 'ck-placeholder' ) ) {
 			writer.addClass( [ 'ck', 'ck-placeholder' ], element );
 			changed = true;
 		}
 	} else {
-		if ( element.hasClass( 'ck', 'ck-placeholder' ) ) {
-			writer.removeClass( [ 'ck', 'ck-placeholder' ], element );
+		if ( element.hasClass( 'ck-placeholder' ) ) {
+			writer.removeClass( 'ck-placeholder', element );
 			changed = true;
 		}
 	}

+ 26 - 0
packages/ckeditor5-engine/tests/view/placeholder.js

@@ -206,4 +206,30 @@ describe( 'placeholder', () => {
 			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
 		} );
 	} );
+
+	it( 'should remove ck class when it was added by placeholder', () => {
+		setData( view, '<div></div><div>{another div}</div>' );
+		const element = viewRoot.getChild( 0 );
+
+		attachPlaceholder( view, element, 'foo bar baz' );
+
+		expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+		expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+
+		detachPlaceholder( view, element );
+		expect( !element.hasClass( 'ck' ) ).to.be.true;
+	} );
+
+	it( 'should not remove ck class when it was applied already', () => {
+		setData( view, '<div class="ck"></div><div>{another div}</div>' );
+		const element = viewRoot.getChild( 0 );
+
+		attachPlaceholder( view, element, 'foo bar baz' );
+
+		expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+		expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+
+		detachPlaceholder( view, element );
+		expect( element.hasClass( 'ck' ) ).to.be.true;
+	} );
 } );