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

Commit

Permalink
Merge pull request #17 from ckeditor/t/ckeditor5-engine/1102
Browse files Browse the repository at this point in the history 
Internal: Use `id` property of HighlightDescriptor.
  • Loading branch information
Piotr Jasiun authored Aug 25, 2017
2 parents 6982e42 + 3408af6 commit 6e96e6b
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 98 deletions.
125 changes: 64 additions & 61 deletions src/highlightstack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,98 +35,101 @@ export default class HighlightStack {
*/
add( descriptor ) {
const stack = this._stack;
let i = 0;

// Find correct place to insert descriptor in the stack.
while ( stack[ i ] && shouldABeBeforeB( stack[ i ], descriptor ) ) {
i++;
// Save top descriptor and insert new one. If top is changed - fire event.
const oldTop = stack[ 0 ];
this._insertDescriptor( descriptor );
const newTop = stack[ 0 ];

// When new object is at the top and stores different information.
if ( oldTop !== newTop && !compareDescriptors( oldTop, newTop ) ) {
this.fire( 'change:top', {
oldDescriptor: oldTop,
newDescriptor: newTop
} );
}
}

stack.splice( i, 0, descriptor );

// New element at the stack top.
if ( i === 0 ) {
const data = {
newDescriptor: descriptor
};

// If old descriptor is present it was pushed down the stack.
if ( stack[ 1 ] ) {
const oldDescriptor = stack[ 1 ];

// New descriptor on the top is same as previous one - do not fire any event.
if ( compareDescriptors( descriptor, oldDescriptor ) ) {
return;
}
/**
* Removes highlight descriptor from the stack.
*
* @fires change:top
* @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} descriptor
*/
remove( descriptor ) {
const stack = this._stack;

data.oldDescriptor = oldDescriptor;
}
const oldTop = stack[ 0 ];
this._removeDescriptor( descriptor );
const newTop = stack[ 0 ];

this.fire( 'change:top', data );
// When new object is at the top and stores different information.
if ( oldTop !== newTop && !compareDescriptors( oldTop, newTop ) ) {
this.fire( 'change:top', {
oldDescriptor: oldTop,
newDescriptor: newTop
} );
}
}

/**
* Removes highlight descriptor from the stack.
* Inserts given descriptor in correct place in the stack. It also takes care about updating information when
* descriptor with same id is already present.
*
* @fires change:top
* @private
* @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} descriptor
*/
remove( descriptor ) {
_insertDescriptor( descriptor ) {
const stack = this._stack;
const length = stack.length;
const index = stack.findIndex( item => item.id === descriptor.id );

if ( length === 0 ) {
// Inserting exact same descriptor - do nothing.
if ( compareDescriptors( descriptor, stack[ index ] ) ) {
return;
}

// If descriptor with same id but with different information is on the stack - remove it.
if ( index > -1 ) {
stack.splice( index, 1 );
}

// Find correct place to insert descriptor in the stack.
// It have different information (for example priority) so it must be re-inserted in correct place.
let i = 0;

while ( stack[ i ] && !compareDescriptors( descriptor, stack[ i ] ) ) {
while ( stack[ i ] && shouldABeBeforeB( stack[ i ], descriptor ) ) {
i++;

// Descriptor not found.
if ( i >= stack.length ) {
return;
}
}

stack.splice( i, 1 );

// Element from stack top was removed - fire `change:top` event with new first element. It might be `undefined`
// which informs that no descriptor is currently at the top.
if ( i === 0 ) {
const data = {
oldDescriptor: descriptor
};

if ( stack[ 0 ] ) {
const newDescriptor = stack[ 0 ];

// New descriptor on the top is same as removed one - do not fire any event.
if ( compareDescriptors( descriptor, newDescriptor ) ) {
return;
}
stack.splice( i, 0, descriptor );
}

data.newDescriptor = newDescriptor;
}
/**
* Removes descriptor with given id from the stack.
*
* @private
* @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} descriptor
*/
_removeDescriptor( descriptor ) {
const stack = this._stack;
const index = stack.findIndex( item => item.id === descriptor.id );

this.fire( 'change:top', data );
// If descriptor with same id is on the list - remove it.
if ( index > -1 ) {
stack.splice( index, 1 );
}
}
}

mix( HighlightStack, EmitterMixin );

// Compares two highlight descriptors by priority and CSS class names. Returns `true` when both descriptors are
// considered equal.
// Compares two descriptors by checking their priority and class list.
//
// @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} descriptorA
// @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} descriptorB
// @returns {Boolean}
function compareDescriptors( descriptorA, descriptorB ) {
return descriptorA.priority == descriptorB.priority &&
classesToString( descriptorA.class ) == classesToString( descriptorB.class );
// @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} a
// @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} b
// @returns {Boolean} Returns true if both descriptors are defined and have same priority and classes.
function compareDescriptors( a, b ) {
return a && b && a.priority == b.priority && classesToString( a.class ) == classesToString( b.class );
}

// Checks whenever first descriptor should be placed in the stack before second one.
Expand Down
61 changes: 39 additions & 22 deletions tests/highlightstack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe( 'HighlightStack', () => {

it( 'should fire event when new descriptor is provided to an empty stack', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-id' };

stack.on( 'change:top', spy );
stack.add( descriptor );
Expand All @@ -26,8 +26,8 @@ describe( 'HighlightStack', () => {

it( 'should fire event when new top element has changed', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 11, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 11, class: 'css-class', id: 'descriptor-2' };

stack.on( 'change:top', spy );
stack.add( descriptor );
Expand All @@ -38,10 +38,27 @@ describe( 'HighlightStack', () => {
expect( spy.secondCall.args[ 1 ].oldDescriptor ).to.equal( descriptor );
} );

it( 'should fire event when top element has updated', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 11, class: 'css-class', id: 'descriptor-1' };

stack.on( 'change:top', spy );
stack.add( descriptor );
stack.add( secondDescriptor );

sinon.assert.calledTwice( spy );
expect( spy.firstCall.args[ 1 ].newDescriptor ).to.equal( descriptor );
expect( spy.firstCall.args[ 1 ].oldDescriptor ).to.be.undefined;

expect( spy.secondCall.args[ 1 ].newDescriptor ).to.equal( secondDescriptor );
expect( spy.secondCall.args[ 1 ].oldDescriptor ).to.equal( descriptor );
} );

it( 'should not fire event when element with lower priority was added', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 9, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 9, class: 'css-class', id: 'descriptor-2' };

stack.on( 'change:top', spy );
stack.add( descriptor );
Expand All @@ -54,8 +71,8 @@ describe( 'HighlightStack', () => {

it( 'should fire event when top element was removed', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 11, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 11, class: 'css-class', id: 'descriptor-2' };

stack.add( descriptor );
stack.add( secondDescriptor );
Expand All @@ -71,8 +88,8 @@ describe( 'HighlightStack', () => {

it( 'should not fire event when other than top element is removed', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 11, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 11, class: 'css-class', id: 'descriptor-2' };

stack.add( descriptor );
stack.add( secondDescriptor );
Expand All @@ -86,8 +103,8 @@ describe( 'HighlightStack', () => {

it( 'should not fire event when same descriptor is added', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 10, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };

stack.on( 'change:top', spy );
stack.add( descriptor );
Expand All @@ -110,8 +127,8 @@ describe( 'HighlightStack', () => {

it( 'should not fire when trying to remove descriptor which is not present', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 12, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 12, class: 'css-class', id: 'descriptor-2' };

stack.add( descriptor );
stack.on( 'change:top', spy );
Expand All @@ -122,7 +139,7 @@ describe( 'HighlightStack', () => {

it( 'should fire event when last element from stack was removed', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor' };

stack.add( descriptor );
stack.on( 'change:top', spy );
Expand All @@ -135,8 +152,8 @@ describe( 'HighlightStack', () => {

it( 'should not fire event when new top descriptor is same as previous', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class' };
const secondDescriptor = { priority: 10, class: 'css-class' };
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { priority: 10, class: 'css-class', id: 'descriptor-2' };

stack.add( descriptor );
stack.add( secondDescriptor );
Expand All @@ -148,9 +165,9 @@ describe( 'HighlightStack', () => {

it( 'should sort by class when priorities are the same', () => {
const spy = sinon.spy();
const descriptorA = { priority: 10, class: 'css-a' };
const descriptorB = { priority: 10, class: 'css-b' };
const descriptorC = { priority: 10, class: 'css-c' };
const descriptorA = { priority: 10, class: 'css-a', id: 'descriptor-1' };
const descriptorB = { priority: 10, class: 'css-b', id: 'descriptor-2' };
const descriptorC = { priority: 10, class: 'css-c', id: 'descriptor-3' };

stack.on( 'change:top', spy );
stack.add( descriptorB );
Expand All @@ -167,9 +184,9 @@ describe( 'HighlightStack', () => {

it( 'should sort by class when priorities are the same - array of CSS classes', () => {
const spy = sinon.spy();
const descriptorA = { priority: 10, class: [ 'css-a', 'css-z' ] };
const descriptorB = { priority: 10, class: [ 'css-a', 'css-y' ] };
const descriptorC = { priority: 10, class: 'css-c' };
const descriptorA = { priority: 10, class: [ 'css-a', 'css-z' ], id: 'descriptor-1' };
const descriptorB = { priority: 10, class: [ 'css-a', 'css-y' ], id: 'descriptor-2' };
const descriptorC = { priority: 10, class: 'css-c', id: 'descriptor-3' };

stack.on( 'change:top', spy );
stack.add( descriptorB );
Expand Down
30 changes: 15 additions & 15 deletions tests/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ describe( 'widget utils', () => {
expect( typeof set ).to.equal( 'function' );
expect( typeof remove ).to.equal( 'function' );

set( element, { priority: 1, class: 'highlight' } );
set( element, { priority: 1, class: 'highlight', id: 'highlight' } );
expect( element.hasClass( 'highlight' ) ).to.be.true;

remove( element, { priority: 1, class: 'highlight' } );
remove( element, { priority: 1, class: 'highlight', id: 'highlight' } );
expect( element.hasClass( 'highlight' ) ).to.be.false;
} );

Expand All @@ -75,11 +75,11 @@ describe( 'widget utils', () => {
expect( typeof set ).to.equal( 'function' );
expect( typeof remove ).to.equal( 'function' );

set( element, { priority: 1, class: [ 'highlight', 'foo' ] } );
set( element, { priority: 1, class: [ 'highlight', 'foo' ], id: 'highlight' } );
expect( element.hasClass( 'highlight' ) ).to.be.true;
expect( element.hasClass( 'foo' ) ).to.be.true;

remove( element, { priority: 1, class: [ 'foo', 'highlight' ] } );
remove( element, { priority: 1, class: [ 'foo', 'highlight' ], id: 'highlight' } );
expect( element.hasClass( 'highlight' ) ).to.be.false;
expect( element.hasClass( 'foo' ) ).to.be.false;
} );
Expand Down Expand Up @@ -174,7 +174,7 @@ describe( 'widget utils', () => {
} );

it( 'should call highlight methods when descriptor is added and removed', () => {
const descriptor = { priority: 10, class: 'highlight' };
const descriptor = { priority: 10, class: 'highlight', id: 'highlight' };

set( element, descriptor );
remove( element, descriptor );
Expand All @@ -187,8 +187,8 @@ describe( 'widget utils', () => {
} );

it( 'should call highlight methods when next descriptor is added', () => {
const descriptor = { priority: 10, class: 'highlight' };
const secondDescriptor = { priority: 11, class: 'highlight' };
const descriptor = { priority: 10, class: 'highlight', id: 'highlight-1' };
const secondDescriptor = { priority: 11, class: 'highlight', id: 'highlight-2' };

set( element, descriptor );
set( element, secondDescriptor );
Expand All @@ -199,8 +199,8 @@ describe( 'widget utils', () => {
} );

it( 'should not call highlight methods when descriptor with lower priority is added', () => {
const descriptor = { priority: 10, class: 'highlight' };
const secondDescriptor = { priority: 9, class: 'highlight' };
const descriptor = { priority: 10, class: 'highlight', id: 'highlight-1' };
const secondDescriptor = { priority: 9, class: 'highlight', id: 'highlight-2' };

set( element, descriptor );
set( element, secondDescriptor );
Expand All @@ -210,8 +210,8 @@ describe( 'widget utils', () => {
} );

it( 'should call highlight methods when descriptor is removed changing active descriptor', () => {
const descriptor = { priority: 10, class: 'highlight' };
const secondDescriptor = { priority: 11, class: 'highlight' };
const descriptor = { priority: 10, class: 'highlight', id: 'highlight-1' };
const secondDescriptor = { priority: 11, class: 'highlight', id: 'highlight-2' };

set( element, descriptor );
set( element, secondDescriptor );
Expand All @@ -228,8 +228,8 @@ describe( 'widget utils', () => {
} );

it( 'should call highlight methods when descriptor is removed not changing active descriptor', () => {
const descriptor = { priority: 10, class: 'highlight' };
const secondDescriptor = { priority: 9, class: 'highlight' };
const descriptor = { priority: 10, class: 'highlight', id: 'highlight-1' };
const secondDescriptor = { priority: 9, class: 'highlight', id: 'highlight-2' };

set( element, descriptor );
set( element, secondDescriptor );
Expand All @@ -242,8 +242,8 @@ describe( 'widget utils', () => {
} );

it( 'should call highlight methods - CSS class array', () => {
const descriptor = { priority: 10, class: [ 'highlight', 'a' ] };
const secondDescriptor = { priority: 10, class: [ 'highlight', 'b' ] };
const descriptor = { priority: 10, class: [ 'highlight', 'a' ], id: 'highlight-1' };
const secondDescriptor = { priority: 10, class: [ 'highlight', 'b' ], id: 'highlight-2' };

set( element, descriptor );
set( element, secondDescriptor );
Expand Down

0 comments on commit 6e96e6b

Please sign in to comment.