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

Commit

Permalink
Using HighlihtDescriptor.id to compare and insert descriptors on High…
Browse files Browse the repository at this point in the history 
…lightStack.
  • Loading branch information
@szymonkups
szymonkups committed Aug 24, 2017
1 parent 6703024 commit 3408af6
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 67 deletions.
89 changes: 45 additions & 44 deletions src/highlightstack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default class HighlightStack {
const newTop = stack[ 0 ];

// When new object is at the top and stores different information.
if ( oldTop !== newTop && !cmp( oldTop, newTop ) ) {
if ( oldTop !== newTop && !compareDescriptors( oldTop, newTop ) ) {
this.fire( 'change:top', {
oldDescriptor: oldTop,
newDescriptor: newTop
Expand All @@ -58,51 +58,43 @@ export default class HighlightStack {
*/
remove( descriptor ) {
const stack = this._stack;
const length = stack.length;

if ( length === 0 ) {
return;
}

let i = 0;

while ( stack[ i ] && !compareDescriptors( descriptor, stack[ i ] ) ) {
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 ];
data.newDescriptor = newDescriptor;
}
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
} );
}
}

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

// If descriptor with same id is on the list - remove it.
// 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 ] && shouldABeBeforeB( stack[ i ], descriptor ) ) {
Expand All @@ -112,25 +104,34 @@ export default class HighlightStack {
stack.splice( i, 0, descriptor );
}

/**
* 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 );

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

mix( HighlightStack, EmitterMixin );

function cmp( a, b ) {
return a && b && a.id == b.id && classesToString( a.class ) == classesToString( b.class ) && a.priority == b.priority;
}

// 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.id == descriptorB.id;
// @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.
//
// @param {module:engine/conversion/buildmodelconverter~HighlightDescriptor} a
Expand Down
33 changes: 25 additions & 8 deletions tests/highlightstack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,23 @@ 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', id: 'descriptor-1' };
Expand Down Expand Up @@ -133,10 +150,10 @@ describe( 'HighlightStack', () => {
expect( spy.firstCall.args[ 1 ].oldDescriptor ).to.equal( descriptor );
} );

it.only( 'should not fire event when new top descriptor is same as previous', () => {
it( 'should not fire event when new top descriptor is same as previous', () => {
const spy = sinon.spy();
const descriptor = { priority: 10, class: 'css-class', id: 'descriptor-1' };
const secondDescriptor = { 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 3408af6

Please sign in to comment.