Merge pull request #8867 from ckeditor/i/8689-smarter-editor-placeholder

Fix (engine): The editor placeholder should not disappear until started typing. Closes #8689.
Fix (image): Image caption placeholder is now hidden when focused. See #8689.
Fix (heading): In the title plugin, the body placeholder is visible even when the body section is focused. See #8689.
Fix: Editor will show the placeholder even when focused. See #8689.

docs/_snippets/examples/multi-root-editor.js

@@ -305,7 +305,8 @@ class MultirootEditorUI extends EditorUI {
 					view: editingView,
 					element: editingRoot,
 					text: placeholderText,
-					isDirectHost: false
+					isDirectHost: false,
+					keepOnFocus: true
 				} );
 			}
 		}

docs/framework/guides/custom-editor-creator.md

@@ -295,7 +295,8 @@ class MultirootEditorUI extends EditorUI {
 					view: editingView,
 					element: editingRoot,
 					text: placeholderText,
-					isDirectHost: false
+					isDirectHost: false,
+					keepOnFocus: true
 				} );
 			}
 		}

packages/ckeditor5-editor-balloon/src/ballooneditorui.js

@@ -134,7 +134,8 @@ export default class BalloonEditorUI extends EditorUI {
 				view: editingView,
 				element: editingRoot,
 				text: placeholderText,
-				isDirectHost: false
+				isDirectHost: false,
+				keepOnFocus: true
 			} );
 		}
 	}

packages/ckeditor5-editor-classic/src/classiceditorui.js

@@ -176,7 +176,8 @@ export default class ClassicEditorUI extends EditorUI {
 				view: editingView,
 				element: editingRoot,
 				text: placeholderText,
-				isDirectHost: false
+				isDirectHost: false,
+				keepOnFocus: true
 			} );
 		}
 	}

packages/ckeditor5-editor-decoupled/src/decouplededitorui.js

@@ -134,7 +134,8 @@ export default class DecoupledEditorUI extends EditorUI {
 				view: editingView,
 				element: editingRoot,
 				text: placeholderText,
-				isDirectHost: false
+				isDirectHost: false,
+				keepOnFocus: true
 			} );
 		}
 	}

packages/ckeditor5-editor-inline/src/inlineeditorui.js

@@ -170,7 +170,8 @@ export default class InlineEditorUI extends EditorUI {
 				view: editingView,
 				element: editingRoot,
 				text: placeholderText,
-				isDirectHost: false
+				isDirectHost: false,
+				keepOnFocus: true
 			} );
 		}
 	}

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.keepOnFocus=false] If set `true`, the placeholder stay visible when the host element is focused.
  */
 export function enablePlaceholder( options ) {
-	const { view, element, text, isDirectHost = true } = options;
+	const { view, element, text, isDirectHost = true, keepOnFocus = 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,
+		keepOnFocus
 	} );
 
 	// 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} keepOnFocus Focusing the element will keep the placeholder visible.
  * @returns {Boolean}
  */
-export function needsPlaceholder( element ) {
+export function needsPlaceholder( element, keepOnFocus ) {
 	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 ( keepOnFocus ) {
+		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.keepOnFocus ) ) {
 		if ( showPlaceholder( writer, hostElement ) ) {
 			wasViewModified = true;
 		}

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

@@ -265,6 +265,58 @@ describe( 'placeholder', () => {
 			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
 			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
+
+		it( 'should keep the placeholder visible when the host element is focused (keepOnFocus = true)', () => {
+			setData( view, '<div></div><div>{another div}</div>' );
+			const element = viewRoot.getChild( 0 );
+
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz',
+				keepOnFocus: true
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.true;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+
+			view.change( writer => {
+				writer.setSelection( ViewRange._createIn( element ) );
+
+				// 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-placeholder' ) ).to.be.true;
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.true;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+		} );
+
+		it( 'should hide the placeholder when the host element is focused (keepOnFocus = false)', () => {
+			setData( view, '<div></div><div>{another div}</div>' );
+			const element = viewRoot.getChild( 0 );
+
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+				// Defaults: keepOnFocus = false
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.true;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+
+			view.change( writer => {
+				writer.setSelection( ViewRange._createIn( element ) );
+
+				// 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-placeholder' ) ).to.be.true;
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.true;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
 	} );
 
 	describe( 'disablePlaceholder', () => {
@@ -399,6 +451,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 +477,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

@@ -340,7 +340,8 @@ export default class Title extends Plugin {
 			enablePlaceholder( {
 				view,
 				element: conversionApi.mapper.toViewElement( data.item ),
-				text: titlePlaceholder
+				text: titlePlaceholder,
+				keepOnFocus: true
 			} );
 		} );
 
@@ -368,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/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;
+}