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

Commit

Permalink
Merge branch 'master' into t/ckeditor5/742
Browse files Browse the repository at this point in the history
  • Loading branch information
Kamil Piechaczek committed Apr 4, 2018
2 parents bafd635 + bfadb47 commit cbcba89
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 44 deletions.
29 changes: 28 additions & 1 deletion src/model/documentselection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ export default class DocumentSelection {

/**
* Removes an attribute with given key from the selection.
* If the given attribute was set on the selection, fires the {@link module:engine/model/selection~Selection#event:change}
* If the given attribute was set on the selection, fires the {@link module:engine/model/selection~Selection#event:change:range}
* event with removed attribute key.
* Should be used only within the {@link module:engine/model/writer~Writer#removeSelectionAttribute} method.
*
Expand Down Expand Up @@ -451,6 +451,33 @@ export default class DocumentSelection {

mix( DocumentSelection, EmitterMixin );

/**
* Fired when selection range(s) changed.
*
* @event change:range
* @param {Boolean} directChange In case of {@link module:engine/model/selection~Selection} class it is always set
* to `true` which indicates that the selection change was caused by a direct use of selection's API.
* The {@link module:engine/model/documentselection~DocumentSelection}, however, may change because its position
* was directly changed through the {@link module:engine/model/writer~Writer writer} or because its position was
* changed because the structure of the model has been changed (which means an indirect change).
* The indirect change does not occur in case of normal (detached) selections because they are "static" (as "not live")
* which mean that they are not updated once the document changes.
*/

/**
* Fired when selection attribute changed.
*
* @event change:attribute
* @param {Boolean} directChange In case of {@link module:engine/model/selection~Selection} class it is always set
* to `true` which indicates that the selection change was caused by a direct use of selection's API.
* The {@link module:engine/model/documentselection~DocumentSelection}, however, may change because its attributes
* were directly changed through the {@link module:engine/model/writer~Writer writer} or because its position was
* changed in the model and its attributes were refreshed (which means an indirect change).
* The indirect change does not occur in case of normal (detached) selections because they are "static" (as "not live")
* which mean that they are not updated once the document changes.
* @param {Array.<String>} attributeKeys Array containing keys of attributes that changed.
*/

// `LiveSelection` is used internally by {@link module:engine/model/documentselection~DocumentSelection} and shouldn't be used directly.
//
// LiveSelection` is automatically updated upon changes in the {@link module:engine/model/document~Document document}
Expand Down
32 changes: 18 additions & 14 deletions src/model/selection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ export default class Selection {
/**
* Removes an attribute with given key from the selection.
*
* If given attribute was set on the selection, fires the {@link #event:change} event with
* If given attribute was set on the selection, fires the {@link #event:change:range} event with
* removed attribute key.
*
* @fires change:attribute
Expand All @@ -561,7 +561,7 @@ export default class Selection {
/**
* Sets attribute on the selection. If attribute with the same key already is set, it's value is overwritten.
*
* If the attribute value has changed, fires the {@link #event:change} event with
* If the attribute value has changed, fires the {@link #event:change:range} event with
* the attribute key.
*
* @fires change:attribute
Expand Down Expand Up @@ -725,25 +725,29 @@ export default class Selection {
}

/**
* @event change
*/

/**
* Fired whenever selection ranges are changed.
* Fired when selection range(s) changed.
*
* @event change:range
* @param {Boolean} directChange Specifies whether the range change was caused by direct usage of `Selection` API (`true`)
* or by changes done to {@link module:engine/model/document~Document model document}
* using {@link module:engine/model/batch~Batch Batch} API (`false`).
* @param {Boolean} directChange In case of {@link module:engine/model/selection~Selection} class it is always set
* to `true` which indicates that the selection change was caused by a direct use of selection's API.
* The {@link module:engine/model/documentselection~DocumentSelection}, however, may change because its position
* was directly changed through the {@link module:engine/model/writer~Writer writer} or because its position was
* changed because the structure of the model has been changed (which means an indirect change).
* The indirect change does not occur in case of normal (detached) selections because they are "static" (as "not live")
* which mean that they are not updated once the document changes.
*/

/**
* Fired whenever selection attributes are changed.
* Fired when selection attribute changed.
*
* @event change:attribute
* @param {Boolean} directChange Specifies whether the attributes changed by direct usage of the Selection API (`true`)
* or by changes done to the {@link module:engine/model/document~Document model document}
* using the {@link module:engine/model/batch~Batch Batch} API (`false`).
* @param {Boolean} directChange In case of {@link module:engine/model/selection~Selection} class it is always set
* to `true` which indicates that the selection change was caused by a direct use of selection's API.
* The {@link module:engine/model/documentselection~DocumentSelection}, however, may change because its attributes
* were directly changed through the {@link module:engine/model/writer~Writer writer} or because its position was
* changed in the model and its attributes were refreshed (which means an indirect change).
* The indirect change does not occur in case of normal (detached) selections because they are "static" (as "not live")
* which mean that they are not updated once the document changes.
* @param {Array.<String>} attributeKeys Array containing keys of attributes that changed.
*/
}
Expand Down
18 changes: 9 additions & 9 deletions src/view/placeholder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function detachPlaceholder( view, element ) {
}

view.change( writer => {
writer.removeClass( 'ck-placeholder', element );
writer.removeClass( [ 'ck', 'ck-placeholder' ], element );
writer.removeAttribute( 'data-placeholder', element );
} );
}
Expand Down Expand Up @@ -105,8 +105,8 @@ function updateSinglePlaceholder( writer, element, info ) {

// If checkFunction is provided and returns false - remove placeholder.
if ( checkFunction && !checkFunction() ) {
if ( element.hasClass( 'ck-placeholder' ) ) {
writer.removeClass( 'ck-placeholder', element );
if ( element.hasClass( 'ck', 'ck-placeholder' ) ) {
writer.removeClass( [ 'ck', 'ck-placeholder' ], element );
changed = true;
}

Expand All @@ -119,8 +119,8 @@ 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-placeholder', element );
if ( !element.hasClass( 'ck', 'ck-placeholder' ) ) {
writer.addClass( [ 'ck', 'ck-placeholder' ], element );
changed = true;
}

Expand All @@ -129,13 +129,13 @@ 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-placeholder', element );
if ( !element.hasClass( 'ck', 'ck-placeholder' ) ) {
writer.addClass( [ 'ck', 'ck-placeholder' ], element );
changed = true;
}
} else {
if ( element.hasClass( 'ck-placeholder' ) ) {
writer.removeClass( 'ck-placeholder', element );
if ( element.hasClass( 'ck', 'ck-placeholder' ) ) {
writer.removeClass( [ 'ck', 'ck-placeholder' ], element );
changed = true;
}
}
Expand Down
38 changes: 19 additions & 19 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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;

result = false;
view.render();
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;

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

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

expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.true;
expect( secondElement.hasClass( 'ck', '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-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;

expect( secondElement.getAttribute( 'data-placeholder' ) ).to.equal( 'second placeholder' );
expect( secondElement.hasClass( 'ck-placeholder' ) ).to.be.false;
expect( secondElement.hasClass( 'ck', '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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;
} );

// After rendering - placeholder should be invisible since selection is moved there.
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', '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-placeholder' ) ).to.be.true;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.true;

detachPlaceholder( view, element );

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

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

expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
expect( element.hasClass( 'ck', 'ck-placeholder' ) ).to.be.false;
} );
} );
} );
2 changes: 1 addition & 1 deletion theme/placeholder.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md.
*/

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

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

0 comments on commit cbcba89

Please sign in to comment.