Browse Source

Changed: `.ck` class should not be added to elements inside editor editable.

Szymon Cofalik 7 years ago
parent
commit
010f943581

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

@@ -36,11 +36,7 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
 	// Store information about element with placeholder.
 	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' )
+		checkFunction
 	} );
 
 	// Update view right away.
@@ -58,12 +54,6 @@ export function detachPlaceholder( view, element ) {
 
 	view.change( writer => {
 		if ( documentPlaceholders.has( doc ) ) {
-			const info = documentPlaceholders.get( doc ).get( element );
-
-			if ( info.removeCkClass ) {
-				writer.removeClass( 'ck', element );
-			}
-
 			documentPlaceholders.get( doc ).delete( element );
 		}
 
@@ -133,7 +123,7 @@ function updateSinglePlaceholder( writer, element, info ) {
 	// If element is empty and editor is blurred.
 	if ( !document.isFocused && isEmptyish ) {
 		if ( !element.hasClass( 'ck-placeholder' ) ) {
-			writer.addClass( [ 'ck', 'ck-placeholder' ], element );
+			writer.addClass( 'ck-placeholder', element );
 			changed = true;
 		}
 
@@ -143,7 +133,7 @@ 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-placeholder' ) ) {
-			writer.addClass( [ 'ck', 'ck-placeholder' ], element );
+			writer.addClass( 'ck-placeholder', element );
 			changed = true;
 		}
 	} else {

+ 19 - 45
packages/ckeditor5-engine/tests/view/placeholder.js

@@ -27,7 +27,7 @@ describe( 'placeholder', () => {
 			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;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'if element has children set only data attribute', () => {
@@ -37,7 +37,7 @@ describe( 'placeholder', () => {
 			attachPlaceholder( view, element, 'foo bar baz' );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'if element has only ui elements, set CSS class and data attribute', () => {
@@ -47,7 +47,7 @@ describe( 'placeholder', () => {
 			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;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'if element has selection inside set only data attribute', () => {
@@ -57,7 +57,7 @@ describe( 'placeholder', () => {
 			attachPlaceholder( view, element, 'foo bar baz' );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'if element has selection inside but document is blurred should contain placeholder CSS class', () => {
@@ -68,7 +68,7 @@ describe( 'placeholder', () => {
 			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;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'use check function if one is provided', () => {
@@ -81,11 +81,11 @@ describe( 'placeholder', () => {
 
 			sinon.assert.called( spy );
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
 			result = false;
 			view.render();
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'should remove CSS class if selection is moved inside', () => {
@@ -95,13 +95,13 @@ describe( 'placeholder', () => {
 			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;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
 			view.change( writer => {
 				writer.setSelection( ViewRange.createIn( element ) );
 			} );
 
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'should change placeholder settings when called twice', () => {
@@ -112,7 +112,7 @@ describe( 'placeholder', () => {
 			attachPlaceholder( view, element, 'new text' );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'new text' );
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'should not throw when element is no longer in document', () => {
@@ -140,10 +140,10 @@ describe( 'placeholder', () => {
 			attachPlaceholder( secondView, secondElement, 'second placeholder' );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
 			expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
-			expect( secondElement.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+			expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.true;
 
 			// Move selection to the elements with placeholders.
 			view.change( writer => {
@@ -155,10 +155,10 @@ describe( 'placeholder', () => {
 			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 
 			expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
-			expect( secondElement.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'should update placeholder before rendering', () => {
@@ -172,11 +172,11 @@ describe( 'placeholder', () => {
 
 				// Here we are before rendering - placeholder is visible in first element;
 				expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-				expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
+				expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 			} );
 
 			// After rendering - placeholder should be invisible since selection is moved there.
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 	} );
 
@@ -188,12 +188,12 @@ describe( 'placeholder', () => {
 			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;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
 			detachPlaceholder( view, element );
 
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'should not blow up when called on element without placeholder', () => {
@@ -203,33 +203,7 @@ describe( 'placeholder', () => {
 			detachPlaceholder( view, element );
 
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
-			expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( '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;
-	} );
 } );

+ 6 - 4
packages/ckeditor5-engine/theme/placeholder.css

@@ -3,9 +3,11 @@
  * For licensing, see LICENSE.md.
  */
 
-.ck.ck-placeholder::before {
-	content: attr( data-placeholder );
+.ck.ck-placeholder, .ck .ck-placeholder {
+	&::before {
+		 content: attr(data-placeholder);
 
-	/* See ckeditor/ckeditor5#469. */
-	pointer-events: none;
+		 /* See ckeditor/ckeditor5#469. */
+		 pointer-events: none;
+	 }
 }