Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Changed: .ck class should not be added to elements inside editor ed…
Browse files Browse the repository at this point in the history 
…itable.
  • Loading branch information
@scofalik
scofalik committed Apr 5, 2018
1 parent f386410 commit 53276a1
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 62 deletions.
16 changes: 3 additions & 13 deletions src/view/placeholder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,7 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
// Store information about element with placeholder.
documentPlaceholders.get( document ).set( element, {
placeholderText,
checkFunction,

// Whether to remove the ck class when detaching the placeholder.
// https://github.com/ckeditor/ckeditor5-image/issues/198#issuecomment-377542222
removeCkClass: !element.hasClass( 'ck' )
checkFunction
} );

// Update view right away.
Expand All @@ -58,12 +54,6 @@ export function detachPlaceholder( view, element ) {

view.change( writer => {
if ( documentPlaceholders.has( doc ) ) {
const info = documentPlaceholders.get( doc ).get( element );

if ( info.removeCkClass ) {
writer.removeClass( 'ck', element );
}

documentPlaceholders.get( doc ).delete( element );
}

Expand Down Expand Up @@ -133,7 +123,7 @@ function updateSinglePlaceholder( writer, element, info ) {
// If element is empty and editor is blurred.
if ( !document.isFocused && isEmptyish ) {
if ( !element.hasClass( 'ck-placeholder' ) ) {
writer.addClass( [ 'ck', 'ck-placeholder' ], element );
writer.addClass( 'ck-placeholder', element );
changed = true;
}

Expand All @@ -143,7 +133,7 @@ function updateSinglePlaceholder( writer, element, info ) {
// It there are no child elements and selection is not placed inside element.
if ( isEmptyish && anchor && anchor.parent !== element ) {
if ( !element.hasClass( 'ck-placeholder' ) ) {
writer.addClass( [ 'ck', 'ck-placeholder' ], element );
writer.addClass( 'ck-placeholder', element );
changed = true;
}
} else {
Expand Down
64 changes: 19 additions & 45 deletions tests/view/placeholder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

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

it( 'if element has children set only data attribute', () => {
Expand All @@ -37,7 +37,7 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );

it( 'if element has only ui elements, set CSS class and data attribute', () => {
Expand All @@ -47,7 +47,7 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

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

it( 'if element has selection inside set only data attribute', () => {
Expand All @@ -57,7 +57,7 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );

it( 'if element has selection inside but document is blurred should contain placeholder CSS class', () => {
Expand All @@ -68,7 +68,7 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

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

it( 'use check function if one is provided', () => {
Expand All @@ -81,11 +81,11 @@ describe( 'placeholder', () => {

sinon.assert.called( spy );
expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;

result = false;
view.render();
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );

it( 'should remove CSS class if selection is moved inside', () => {
Expand All @@ -95,13 +95,13 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;

view.change( writer => {
writer.setSelection( ViewRange.createIn( element ) );
} );

expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );

it( 'should change placeholder settings when called twice', () => {
Expand All @@ -112,7 +112,7 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'new text' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'new text' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
} );

it( 'should not throw when element is no longer in document', () => {
Expand Down Expand Up @@ -140,10 +140,10 @@ describe( 'placeholder', () => {
attachPlaceholder( secondView, secondElement, 'second placeholder' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;

expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
expect( secondElement.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.true;

// Move selection to the elements with placeholders.
view.change( writer => {
Expand All @@ -155,10 +155,10 @@ describe( 'placeholder', () => {
} );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;

expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
expect( secondElement.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.false;
} );

it( 'should update placeholder before rendering', () => {
Expand All @@ -172,11 +172,11 @@ describe( 'placeholder', () => {

// 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', 'ck-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
} );

// After rendering - placeholder should be invisible since selection is moved there.
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );
} );

Expand All @@ -188,12 +188,12 @@ describe( 'placeholder', () => {
attachPlaceholder( view, element, 'foo bar baz' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;

detachPlaceholder( view, element );

expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );

it( 'should not blow up when called on element without placeholder', () => {
Expand All @@ -203,33 +203,7 @@ describe( 'placeholder', () => {
detachPlaceholder( view, element );

expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
} );
} );

it( 'should remove ck class when it was added by placeholder', () => {
setData( view, '<div></div><div>{another div}</div>' );
const element = viewRoot.getChild( 0 );

attachPlaceholder( view, element, 'foo bar baz' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;

detachPlaceholder( view, element );
expect( !element.hasClass( 'ck' ) ).to.be.true;
} );

it( 'should not remove ck class when it was applied already', () => {
setData( view, '<div class="ck"></div><div>{another div}</div>' );
const element = viewRoot.getChild( 0 );

attachPlaceholder( view, element, 'foo bar baz' );

expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;

detachPlaceholder( view, element );
expect( element.hasClass( 'ck' ) ).to.be.true;
} );
} );
10 changes: 6 additions & 4 deletions theme/placeholder.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
* For licensing, see LICENSE.md.
*/

.ck.ck-placeholder::before {
content: attr( data-placeholder );
.ck.ck-placeholder, .ck .ck-placeholder {
&::before {
content: attr(data-placeholder);

/* See ckeditor/ckeditor5#469. */
pointer-events: none;
/* See ckeditor/ckeditor5#469. */
pointer-events: none;
}
}

0 comments on commit 53276a1

Please sign in to comment.