T/ckeditor5/479: Made it possible to use addPlaceholder() on the root editable

[oleq]

Suggested merge commit message (convention)

Feature: Moved the root element DOM attributes management from the UI to the engine. Made it possible to use addPlaceholder() (now enablePlaceholder()) on the root editable. Introduced the View.detachDomRoot() method. Implemented additional placeholder helpers (showPlaceholder(), hidePlaceholder(), needsPlaceholder()) (see ckeditor/ckeditor5#479). Closes ckeditor/ckeditor5#4034.

BREAKING CHANGE: The attachPlaceholder() has been renamed to enablePlaceholder().
BREAKING CHANGE: enablePlaceholder() accepts a configuration object instead of separate parameters.
BREAKING CHANGE: The detachPlaceholder() has been renamed to disablePlaceholder().

---

Additional information

Part of the constellation.

[pjasiun]

I was looking for usage of this function but I found none (only tests). If it is not needed maybe the change in this function is not needed too.

[oleq]

AFAIR it is used in Letters, isn't it?

[pjasiun]

It is a little too magic for me. get...Element function does not return an element, but a function which returns element. In fact, it also decides if the placeholder should be added based on the child could, so can be used instead of checkFunction. Even if it let us integrate placeholders with a few lines of code, the time one will need to spent to understand how it works is not worth it. I think this logic should be some outside in the EditorUI class.

[pjasiun]

I think such code in the editor UI would be more readable:

let elementWithPlaceholder;
const viewRoot = view.document.getRoot();

model.document.on( 'change', () => {
	const firstRootChild = viewRoot.getChild( 1 );

	// Remove placeholder when is not needed.
	if ( viewRoot.childCount != 1 || !firstRootChild || firstRootChild.is( 'element' ) ) {
		if ( elementWithPlaceholder ) {
			removePlaceholder( view, elementWithPlaceholder );
			elementWithPlaceholder = undefined;
		}

		return;
	}

	// Add placeholder only if firstRootChild element changed.
	if ( elementWithPlaceholder !== firstRootChild ) {
		addPlaceholder( view, firstRootChild, label );
		elementWithPlaceholder = firstRootChild;
	}
}, { priority: 'low' } );

Then it can be changed to util.

[oleq]

I have mixed feelings about this piece of code you proposed. Where would you like to keep it?

[coveralls]

Coverage increased (+0.008%) to 100.0% when pulling 0e49518 on t/ckeditor5/479 into 4fba713 on master.

[Reinmar]

During a F2F talk we discussed such a simplified API:

export function hidePlaceholder() {}

export function showPlaceholder() {}

export function needsPlaceholder() {}

export function enablePlaceholder( view, element, isParagraphLikeElement=true ) {
  const whereNeeded = isParagraphLikeElement ? element : element.getChild( 0 );

  view.registerPostFixer( () =>
    if ( needsPlaceholder( whereNeeded ) ) {
      showPlaceholder( view, whereNeeded );
    } else {
      hidePlaceholder( view, whereNeeded );
    }
  } );
}

That's a pseudo code of how it could look. The idea is that the "whether the placeholder should be added" is limited to two supported cases – directly in the passed element or in its first child if it's a ContainerElement.

If anyone needs to support more tricky cases like displaying the placeholder in the second element in the root, they can use needsPlaceholder() and show/hidePlaceholder() functions. Ideally, they shouldn't need to write much more code.

[Reinmar]

One more thing – we discussed moving this code to src/utils. I'm not sure this is necessary now, because it won't use model for anything (we considered an option where it works on the model), but it's a possibility.

[pjasiun]

That's a pseudo code of how it could look.

It is a very PSEUDO code ;) It is not as simple as:

const whereNeeded = isParagraphLikeElement ? element : element.getChild( 0 );

If it is not a paragraph-like element we need to follow changes in the parent. We need to remove the placeholder if the element split and there is more than one child, it might happen that we will have to remove the placeholder from a different than the first element (for instant some content is added at the beginning of the document, before the child with the placeholder).

Also, needsPlaceholder most probably also will need isParagraphLikeElement flag, because it should use a very different check if it checks paragraph and if it checks root element. Or we could limit needsPlaceholder to paragraph-like elements.

One more thing – we discussed moving this code to src/utils. I'm not sure this is necessary now, because it won't use model for anything (we considered an option where it works on the model), but it's a possibility.

Agree, it could stay in the view, where it is now.