Skip to content

Commit

Permalink
Merge pull request #14753 from ckeditor/ck/14743-enablePlaceholder-AP…
Browse files Browse the repository at this point in the history
…I-should-remain-backward-compatible-for-some-time

Fix (engine): Made the `enablePlaceholder()` API to remain backward compatible for the deprecation period. It will be removed in the future. Closes #14743.
  • Loading branch information
mlewand authored and pomek committed Aug 8, 2023
1 parent 17b86b1 commit 048ed18
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 5 deletions.
35 changes: 33 additions & 2 deletions packages/ckeditor5-engine/src/view/placeholder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import type EditableElement from './editableelement';
import type Element from './element';
import type View from './view';

import type { ObservableChangeEvent } from '@ckeditor/ckeditor5-utils';
import { logWarning, type ObservableChangeEvent } from '@ckeditor/ckeditor5-utils';

// Each document stores information about its placeholder elements and check functions.
const documentPlaceholders = new WeakMap<Document, Map<Element, PlaceholderConfig>>();

let hasDisplayedPlaceholderDeprecationWarning = false;

/**
* A helper that enables a placeholder on the provided view element (also updates its visibility).
* The placeholder is a CSS pseudo–element (with a text content) attached to the element.
Expand All @@ -35,12 +37,15 @@ const documentPlaceholders = new WeakMap<Document, Map<Element, PlaceholderConfi
* 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 options.text Placeholder text. It's **deprecated** and will be removed soon. Use
* {@link module:engine/view/placeholder~PlaceholderableElement#placeholder `options.element.placeholder`} instead.
* @param options.keepOnFocus If set `true`, the placeholder stay visible when the host element is focused.
*/
export function enablePlaceholder( { view, element, isDirectHost = true, keepOnFocus = false }: {
export function enablePlaceholder( { view, element, text, isDirectHost = true, keepOnFocus = false }: {
view: View;
element: PlaceholderableElement | EditableElement;
isDirectHost?: boolean;
text?: string;
keepOnFocus?: boolean;
} ): void {
const doc = view.document;
Expand All @@ -67,6 +72,12 @@ export function enablePlaceholder( { view, element, isDirectHost = true, keepOnF

if ( element.placeholder ) {
setPlaceholder( element.placeholder );
} else if ( text ) {
setPlaceholder( text );
}

if ( text ) {
showPlaceholderTextDeprecationWarning();
}

function setPlaceholder( text: string ) {
Expand Down Expand Up @@ -299,6 +310,26 @@ function getChildPlaceholderHostSubstitute( parent: Element ): Element | null {
return null;
}

/**
* Displays a deprecation warning message in the console, but only once per page load.
*/
function showPlaceholderTextDeprecationWarning() {
if ( !hasDisplayedPlaceholderDeprecationWarning ) {
/**
* The "text" option in the {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`}
* function is deprecated and will be removed soon.
*
* See the {@glink updating/guides/update-to-39#view-element-placeholder Migration to v39} guide for
* more information on how to apply this change.
*
* @error enableplaceholder-deprecated-text-option
*/
logWarning( 'enableplaceholder-deprecated-text-option' );
}

hasDisplayedPlaceholderDeprecationWarning = true;
}

/**
* Configuration of the placeholder.
*/
Expand Down
55 changes: 52 additions & 3 deletions packages/ckeditor5-engine/tests/view/placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals console */

import {
enablePlaceholder,
disablePlaceholder,
Expand Down Expand Up @@ -618,8 +620,7 @@ describe( 'placeholder', () => {

enablePlaceholder( {
view,
element: viewRoot,
text: 'foo bar baz'
element: viewRoot
} );
viewRoot.placeholder = 'new placeholder';

Expand All @@ -632,12 +633,60 @@ describe( 'placeholder', () => {
enablePlaceholder( {
view,
element: viewRoot,
text: 'foo bar baz',
isDirectHost: false
} );
viewRoot.placeholder = 'new placeholder';

expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'new placeholder' );
} );

it( 'should through warning once if "text" is used as argument', () => {
sinon.stub( console, 'warn' );

setData( view, '<div></div><div>{another div}</div>' );

enablePlaceholder( {
view,
element: viewRoot,
isDirectHost: false,
text: 'foo bar'
} );

enablePlaceholder( {
view,
element: viewRoot,
isDirectHost: false,
text: 'foo bar baz'
} );

sinon.assert.calledOnce( console.warn );
expect( console.warn.calledWith( sinon.match( /^enableplaceholder-deprecated-text-option/ ) ) ).to.be.true;
} );

it( 'should set placeholder using "text" argument', () => {
setData( view, '<div></div><div>{another div}</div>' );

enablePlaceholder( {
view,
element: viewRoot,
text: 'placeholder'
} );

expect( viewRoot.getAttribute( 'data-placeholder' ) ).to.equal( 'placeholder' );
} );

it( 'should prefer element\'s placeholder value over text parameter', () => {
setData( view, '<div></div><div>{another div}</div>' );

enablePlaceholder( {
view,
element: viewRoot,
text: 'foo'
} );

viewRoot.placeholder = 'bar';

expect( viewRoot.getAttribute( 'data-placeholder' ) ).to.equal( 'bar' );
} );
} );
} );

0 comments on commit 048ed18

Please sign in to comment.