Skip to content

Commit

Permalink
Merge branch 'feat/better-placeholder-behavior' of git://github.com/h…
Browse files Browse the repository at this point in the history
…uacnlee/ckeditor5 into huacnlee-feat/better-placeholder-behavior
  • Loading branch information
oleq committed Dec 10, 2020
2 parents 0ad1d6a + a75f9df commit 6719bf0
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 17 deletions.
14 changes: 2 additions & 12 deletions packages/ckeditor5-engine/src/view/placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,8 @@ export function needsPlaceholder( element ) {
const isEmptyish = !Array.from( element.getChildren() )
.some( element => !element.is( 'uiElement' ) );

const doc = element.document;

// If the element is empty and the document is blurred.
if ( !doc.isFocused && isEmptyish ) {
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 ) {
// If the element, keep placeholder
if ( isEmptyish ) {
return true;
}

Expand Down
10 changes: 5 additions & 5 deletions packages/ckeditor5-engine/tests/view/placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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)', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-engine/theme/placeholder.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
.ck.ck-placeholder,
.ck .ck-placeholder {
&::before {
position: absolute;
content: attr(data-placeholder);

/* See ckeditor/ckeditor5#469. */
Expand Down

0 comments on commit 6719bf0

Please sign in to comment.