Skip to content

Commit

Permalink
Merge branch 'master' into i/9048
Browse files Browse the repository at this point in the history
  • Loading branch information
oleq committed Feb 18, 2021
2 parents 6ccc4c7 + af51165 commit 89a27e6
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 14 deletions.
50 changes: 36 additions & 14 deletions packages/ckeditor5-engine/src/view/placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export function enablePlaceholder( options ) {
documentPlaceholders.get( doc ).set( element, {
text,
isDirectHost,
keepOnFocus
keepOnFocus,
hostElement: isDirectHost ? element : null
} );

// Update the placeholders right away.
Expand Down Expand Up @@ -186,9 +187,42 @@ export function needsPlaceholder( element, keepOnFocus ) {
// @returns {Boolean} True if any changes were made to the view document.
function updateDocumentPlaceholders( doc, writer ) {
const placeholders = documentPlaceholders.get( doc );
const directHostElements = [];
let wasViewModified = false;

// First set placeholders on the direct hosts.
for ( const [ element, config ] of placeholders ) {
if ( config.isDirectHost ) {
directHostElements.push( element );

if ( updatePlaceholder( writer, element, config ) ) {
wasViewModified = true;
}
}
}

// Then set placeholders on the indirect hosts but only on those that does not already have an direct host placeholder.
for ( const [ element, config ] of placeholders ) {
if ( config.isDirectHost ) {
continue;
}

const hostElement = getChildPlaceholderHostSubstitute( element );

// When not a direct host, it could happen that there is no child element
// capable of displaying a placeholder.
if ( !hostElement ) {
continue;
}

// Don't override placeholder if the host element already has some direct placeholder.
if ( directHostElements.includes( hostElement ) ) {
continue;
}

// Update the host element (used for setting and removing the placeholder).
config.hostElement = hostElement;

if ( updatePlaceholder( writer, element, config ) ) {
wasViewModified = true;
}
Expand All @@ -207,22 +241,10 @@ function updateDocumentPlaceholders( doc, writer ) {
// @param {Boolean} config.isDirectHost
// @returns {Boolean} True if any changes were made to the view document.
function updatePlaceholder( writer, element, config ) {
const { text, isDirectHost } = config;
const { text, isDirectHost, hostElement } = config;

const hostElement = isDirectHost ? element : getChildPlaceholderHostSubstitute( element );
let wasViewModified = false;

// When not a direct host, it could happen that there is no child element
// capable of displaying a placeholder.
if ( !hostElement ) {
return false;
}

// Cache the host element. It will be necessary for disablePlaceholder() to know
// which element should have class and attribute removed because, depending on
// the config.isDirectHost value, it could be the element or one of its descendants.
config.hostElement = hostElement;

// This may be necessary when updating the placeholder text to something else.
if ( hostElement.getAttribute( 'data-placeholder' ) !== text ) {
writer.setAttribute( 'data-placeholder', text, hostElement );
Expand Down
41 changes: 41 additions & 0 deletions packages/ckeditor5-engine/tests/view/placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,47 @@ describe( 'placeholder', () => {
expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
} );

// https://github.com/ckeditor/ckeditor5/issues/9046
it( 'should set attribute for the direct placeholder even if there is also indirect one (isDirectHost=false)', () => {
setData( view, '<p></p>' );
viewDocument.isFocused = false;

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

enablePlaceholder( {
view,
element: viewRoot.getChild( 0 ),
text: 'bar',
isDirectHost: true
} );

expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'bar' );
expect( viewRoot.getChild( 0 ).isEmpty ).to.be.true;
expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
} );

it( 'should not trigger infinite post-fixers loop (isDirectHost=false)', () => {
setData( view, '<p></p>' );
viewDocument.isFocused = false;

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

view.forceRender();

expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
} );

it( 'should not set class when there is no children (isDirectHost=false)', () => {
setData( view, '<p></p><p>foobar</p>' );
viewDocument.isFocused = false;
Expand Down

0 comments on commit 89a27e6

Please sign in to comment.