#8689: The editor placeholder should not disappear until started typing

packages/ckeditor5-engine/src/view/placeholder.js

@@ -29,9 +29,10 @@ const documentPlaceholders = new WeakMap();
  * in the passed `element` but in one of its children (selected automatically, i.e. a first empty child element).
  * Useful when attaching placeholders to elements that can host other elements (not just text), for instance,
  * editable root elements.
+ * @param {Boolean} [options.hideOnFocus=false] If set `true`, the placeholder will be hidden when the host element is focused.
  */
 export function enablePlaceholder( options ) {
-	const { view, element, text, isDirectHost = true } = options;
+	const { view, element, text, isDirectHost = true, hideOnFocus = false } = options;
 	const doc = view.document;
 
 	// Use a single a single post fixer per—document to update all placeholders.
@@ -46,7 +47,8 @@ export function enablePlaceholder( options ) {
 	// Store information about the element placeholder under its document.
 	documentPlaceholders.get( doc ).set( element, {
 		text,
-		isDirectHost
+		isDirectHost,
+		hideOnFocus
 	} );
 
 	// Update the placeholders right away.
@@ -140,34 +142,40 @@ export function hidePlaceholder( writer, element ) {
  * {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`} in that case or make
  * sure the correct element is passed to the helper.
  *
- * @param {module:engine/view/element~Element} element
+ * @param {module:engine/view/element~Element} element Element that holds the placeholder.
+ * @param {Boolean} ignoreDocumentFocus Focusing the element will keep the placeholder visible.
  * @returns {Boolean}
  */
-export function needsPlaceholder( element ) {
+export function needsPlaceholder( element, ignoreDocumentFocus ) {
 	if ( !element.isAttached() ) {
 		return false;
 	}
 
-	// The element is empty only as long as it contains nothing but uiElements.
-	const isEmptyish = !Array.from( element.getChildren() )
+	// Anything but uiElement(s) counts as content.
+	const hasContent = Array.from( element.getChildren() )
 		.some( element => !element.is( 'uiElement' ) );
 
+	if ( hasContent ) {
+		return false;
+	}
+
+	// Skip the focus check and make the placeholder visible already regardless of document focus state.
+	if ( ignoreDocumentFocus ) {
+		return true;
+	}
+
 	const doc = element.document;
 
-	// If the element is empty and the document is blurred.
-	if ( !doc.isFocused && isEmptyish ) {
+	// If the document is blurred.
+	if ( !doc.isFocused ) {
 		return true;
 	}
 
 	const viewSelection = doc.selection;
 	const selectionAnchor = viewSelection.anchor;
 
 	// If document is focused and the element is empty but the selection is not anchored inside it.
-	if ( isEmptyish && selectionAnchor && selectionAnchor.parent !== element ) {
-		return true;
-	}
-
-	return false;
+	return selectionAnchor && selectionAnchor.parent !== element;
 }
 
 // Updates all placeholders associated with a document in a post–fixer callback.
@@ -221,7 +229,7 @@ function updatePlaceholder( writer, element, config ) {
 		wasViewModified = true;
 	}
 
-	if ( needsPlaceholder( hostElement ) ) {
+	if ( needsPlaceholder( hostElement, !config.hideOnFocus ) ) {
 		if ( showPlaceholder( writer, hostElement ) ) {
 			wasViewModified = true;
 		}

packages/ckeditor5-engine/tests/view/placeholder.js

@@ -95,7 +95,7 @@ describe( 'placeholder', () => {
 			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'if element has selection inside but document is blurred should contain placeholder CSS class', () => {
@@ -132,7 +132,7 @@ describe( 'placeholder', () => {
 				writer.setSelection( ViewRange._createIn( element ) );
 			} );
 
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'should change placeholder settings when called twice', () => {
@@ -208,10 +208,10 @@ describe( 'placeholder', () => {
 			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
 			expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
-			expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'should update placeholder before rendering', () => {
@@ -233,7 +233,7 @@ describe( 'placeholder', () => {
 			} );
 
 			// After rendering - placeholder should be invisible since selection is moved there.
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
 		it( 'should not set attributes/class when multiple children (isDirectHost=false)', () => {
@@ -399,6 +399,15 @@ describe( 'placeholder', () => {
 			expect( needsPlaceholder( element ) ).to.be.true;
 		} );
 
+		it( 'should return false if element has content other than UI elements', () => {
+			setData( view, '<p>{moo}<ui:span></ui:span></p>' );
+			viewDocument.isFocused = true;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.false;
+		} );
+
 		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;
@@ -416,5 +425,14 @@ describe( 'placeholder', () => {
 
 			expect( needsPlaceholder( element ) ).to.be.true;
 		} );
+
+		it( 'should return true if we want to keep placeholder when element is focused', () => {
+			setData( view, '<p><ui:span></ui:span></p>' );
+			viewDocument.isFocused = true;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element, true ) ).to.be.true;
+		} );
 	} );
 } );

packages/ckeditor5-engine/theme/placeholder.css

@@ -7,6 +7,7 @@
 .ck.ck-placeholder,
 .ck .ck-placeholder {
 	&::before {
+		position: absolute;
 		content: attr(data-placeholder);
 
 		/* See ckeditor/ckeditor5#469. */

packages/ckeditor5-heading/src/title.js

@@ -369,7 +369,8 @@ export default class Title extends Plugin {
 			}
 
 			// Then we need to display placeholder if it is needed.
-			if ( needsPlaceholder( body ) && viewRoot.childCount === 2 && body.name === 'p' ) {
+			// See: https://github.com/ckeditor/ckeditor5/issues/8689.
+			if ( needsPlaceholder( body, true ) && viewRoot.childCount === 2 && body.name === 'p' ) {
 				hasChanged = showPlaceholder( writer, body ) ? true : hasChanged;
 			// Or hide if it is not needed.
 			} else {

packages/ckeditor5-image/src/imagecaption/utils.js

@@ -25,7 +25,9 @@ export function captionElementCreator( view, placeholderText ) {
 		enablePlaceholder( {
 			view,
 			element: editable,
-			text: placeholderText
+			text: placeholderText,
+			// See: https://github.com/ckeditor/ckeditor5/issues/8689.
+			hideOnFocus: true
 		} );
 
 		return toWidgetEditable( editable, writer );

packages/ckeditor5-image/tests/imagecaption/imagecaptionediting.js

@@ -430,6 +430,39 @@ describe( 'ImageCaptionEditing', () => {
 			);
 		} );
 
+		it( 'should hide placeholder when figcaption is focused', () => {
+			setModelData( model, '<paragraph>[]foo</paragraph><image src=""><caption></caption></image>' );
+
+			expect( getViewData( view ) ).to.equal(
+				'<p>{}foo</p>' +
+				'<figure class="ck-widget image" contenteditable="false">' +
+					'<img src=""></img>' +
+					'<figcaption class="ck-editor__editable ck-editor__nested-editable ck-hidden ck-placeholder" ' +
+						'contenteditable="true" data-placeholder="Enter image caption">' +
+					'</figcaption>' +
+				'</figure>'
+			);
+
+			const caption = doc.getRoot().getNodeByPath( [ 1, 0 ] );
+
+			editor.editing.view.document.isFocused = true;
+			editor.focus();
+
+			model.change( writer => {
+				writer.setSelection( writer.createRangeIn( caption ) );
+			} );
+
+			expect( getViewData( view ) ).to.equal(
+				'<p>foo</p>' +
+				'<figure class="ck-widget image" contenteditable="false">' +
+					'<img src=""></img>' +
+					'<figcaption class="ck-editor__editable ck-editor__nested-editable ck-editor__nested-editable_focused" ' +
+						'contenteditable="true" data-placeholder="Enter image caption">[]' +
+					'</figcaption>' +
+				'</figure>'
+			);
+		} );
+
 		it( 'should not add additional figcaption if one is already present', () => {
 			setModelData( model, '<paragraph>foo</paragraph>[<image src=""><caption>foo bar</caption></image>]' );
 

packages/ckeditor5-image/tests/imagecaption/utils.js

@@ -27,7 +27,7 @@ describe( 'image captioning utils', () => {
 		} );
 	} );
 
-	describe( 'captionElementCreator', () => {
+	describe( 'captionElementCreator()', () => {
 		it( 'should create figcatpion editable element', () => {
 			expect( element ).to.be.instanceOf( ViewEditableElement );
 			expect( element.name ).to.equal( 'figcaption' );
@@ -39,7 +39,7 @@ describe( 'image captioning utils', () => {
 		} );
 	} );
 
-	describe( 'isCaptionEditable', () => {
+	describe( 'isCaption()', () => {
 		it( 'should return true for elements created with creator', () => {
 			expect( isCaption( element ) ).to.be.true;
 		} );
@@ -51,7 +51,7 @@ describe( 'image captioning utils', () => {
 		} );
 	} );
 
-	describe( 'getCaptionFromImage', () => {
+	describe( 'getCaptionFromImage()', () => {
 		it( 'should return caption element from image element', () => {
 			const dummy = new ModelElement( 'dummy' );
 			const caption = new ModelElement( 'caption' );
@@ -67,7 +67,7 @@ describe( 'image captioning utils', () => {
 		} );
 	} );
 
-	describe( 'matchImageCaption', () => {
+	describe( 'matchImageCaption()', () => {
 		it( 'should return null for element that is not a figcaption', () => {
 			const element = new ViewElement( document, 'div' );
 

packages/ckeditor5-image/theme/image.css

@@ -25,3 +25,12 @@
 		min-width: 50px;
 	}
 }
+
+/*
+ * Since the caption placeholder for images disappears when focused, it does not require special treatment
+ * and can go with a position that follows text alignment of an .image out-of-the-box (center by default).
+ * See https://github.com/ckeditor/ckeditor5/issues/8689.
+ */
+.ck.ck-editor__editable .image > figcaption.ck-placeholder::before {
+	position: static;
+}