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

Commit 6e0d063

Browse files
author
Piotr Jasiun
authored
Merge pull request #216 from ckeditor/t/215
Feature: Introduced skipping items when binding collections. Closes #215. Closes https://github.com/ckeditor/ckeditor5-ui/issues/92.
2 parents cc96644 + 24db40a commit 6e0d063

File tree

2 files changed

+267
-9
lines changed

2 files changed

+267
-9
lines changed

‎src/collection.js

+108-6
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ export default class Collection {
8080
*/
8181
this._bindToInternalToExternalMap = new WeakMap();
8282

83+
/**
84+
* Stores indexes of skipped items from bound external collection.
85+
*
86+
* @private
87+
* @member {Array}
88+
*/
89+
this._skippedIndexesFromExternal = [];
90+
8391
/**
8492
* A collection instance this collection is bound to as a result
8593
* of calling {@link #bindTo} method.
@@ -271,7 +279,7 @@ export default class Collection {
271279
this._bindToInternalToExternalMap.delete( item );
272280
this._bindToExternalToInternalMap.delete( externalItem );
273281

274-
this.fire( 'remove', item );
282+
this.fire( 'remove', item, index );
275283

276284
return item;
277285
}
@@ -402,9 +410,28 @@ export default class Collection {
402410
* console.log( target.get( 0 ).value ); // 'foo'
403411
* console.log( target.get( 1 ).value ); // 'bar'
404412
*
413+
* It's possible to skip specified items by returning falsy value:
414+
*
415+
* const source = new Collection();
416+
* const target = new Collection();
417+
*
418+
* target.bindTo( source ).using( item => {
419+
* if ( item.hidden ) {
420+
* return null;
421+
* }
422+
*
423+
* return item;
424+
* } );
425+
*
426+
* source.add( { hidden: true } );
427+
* source.add( { hidden: false } );
428+
*
429+
* console.log( source.length ); // 2
430+
* console.log( target.length ); // 1
431+
*
405432
* **Note**: {@link #clear} can be used to break the binding.
406433
*
407-
* @param {module:utils/collection~Collection} collection A collection to be bound.
434+
* @param {module:utils/collection~Collection} externalCollection A collection to be bound.
408435
* @returns {Object}
409436
* @returns {module:utils/collection~Collection#bindTo#as} return.as
410437
* @returns {module:utils/collection~Collection#bindTo#using} return.using
@@ -476,28 +503,102 @@ export default class Collection {
476503
} else {
477504
const item = factory( externalItem );
478505

506+
// When there is no item we need to remember skipped index first and then we can skip this item.
507+
if ( !item ) {
508+
this._skippedIndexesFromExternal.push( index );
509+
510+
return;
511+
}
512+
513+
// Lets try to put item at the same index as index in external collection
514+
// but when there are a skipped items in one or both collections we need to recalculate this index.
515+
let finalIndex = index;
516+
517+
// When we try to insert item after some skipped items from external collection we need
518+
// to include this skipped items and decrease index.
519+
//
520+
// For the following example:
521+
// external -> [ 'A', 'B - skipped for internal', 'C - skipped for internal' ]
522+
// internal -> [ A ]
523+
//
524+
// Another item is been added at the end of external collection:
525+
// external.add( 'D' )
526+
// external -> [ 'A', 'B - skipped for internal', 'C - skipped for internal', 'D' ]
527+
//
528+
// We can't just add 'D' to internal at the same index as index in external because
529+
// this will produce empty indexes what is invalid:
530+
// internal -> [ 'A', empty, empty, 'D' ]
531+
//
532+
// So we need to include skipped items and decrease index
533+
// internal -> [ 'A', 'D' ]
534+
for ( const skipped of this._skippedIndexesFromExternal ) {
535+
if ( index > skipped ) {
536+
finalIndex--;
537+
}
538+
}
539+
540+
// We need to take into consideration that external collection could skip some items from
541+
// internal collection.
542+
//
543+
// For the following example:
544+
// internal -> [ 'A', 'B - skipped for external', 'C - skipped for external' ]
545+
// external -> [ A ]
546+
//
547+
// Another item is been added at the end of external collection:
548+
// external.add( 'D' )
549+
// external -> [ 'A', 'D' ]
550+
//
551+
// We need to include skipped items and place new item after them:
552+
// internal -> [ 'A', 'B - skipped for external', 'C - skipped for external', 'D' ]
553+
for ( const skipped of externalCollection._skippedIndexesFromExternal ) {
554+
if ( finalIndex >= skipped ) {
555+
finalIndex++;
556+
}
557+
}
558+
479559
this._bindToExternalToInternalMap.set( externalItem, item );
480560
this._bindToInternalToExternalMap.set( item, externalItem );
481-
482-
this.add( item, index );
561+
this.add( item, finalIndex );
562+
563+
// After adding new element to internal collection we need update indexes
564+
// of skipped items in external collection.
565+
for ( let i = 0; i < externalCollection._skippedIndexesFromExternal.length; i++ ) {
566+
if ( finalIndex <= externalCollection._skippedIndexesFromExternal[ i ] ) {
567+
externalCollection._skippedIndexesFromExternal[ i ]++;
568+
}
569+
}
483570
}
484571
};
485572

486573
// Load the initial content of the collection.
487574
for ( const externalItem of externalCollection ) {
488-
addItem( null, externalItem );
575+
addItem( null, externalItem, externalCollection.getIndex( externalItem ) );
489576
}
490577

491578
// Synchronize the with collection as new items are added.
492579
this.listenTo( externalCollection, 'add', addItem );
493580

494581
// Synchronize the with collection as new items are removed.
495-
this.listenTo( externalCollection, 'remove', ( evt, externalItem ) => {
582+
this.listenTo( externalCollection, 'remove', ( evt, externalItem, index ) => {
496583
const item = this._bindToExternalToInternalMap.get( externalItem );
497584

498585
if ( item ) {
499586
this.remove( item );
500587
}
588+
589+
// After removing element from external collection we need update/remove indexes
590+
// of skipped items in internal collection.
591+
this._skippedIndexesFromExternal = this._skippedIndexesFromExternal.reduce( ( result, skipped ) => {
592+
if ( index < skipped ) {
593+
result.push( skipped - 1 );
594+
}
595+
596+
if ( index > skipped ) {
597+
result.push( skipped );
598+
}
599+
600+
return result;
601+
}, [] );
501602
} );
502603
}
503604

@@ -520,6 +621,7 @@ export default class Collection {
520621
*
521622
* @event remove
522623
* @param {Object} item The removed item.
624+
* @param {Number} index Index from which item was removed.
523625
*/
524626
}
525627

‎tests/collection.js

+159-3
Original file line numberDiff line numberDiff line change
@@ -448,9 +448,9 @@ describe( 'Collection', () => {
448448
collection.remove( 'bom' ); // by id
449449

450450
sinon.assert.calledThrice( spy );
451-
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item1 );
452-
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item2 );
453-
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3 );
451+
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item1, 0 );
452+
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item2, 1 );
453+
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3, 0 );
454454
} );
455455

456456
it( 'should throw an error on invalid index', () => {
@@ -767,6 +767,34 @@ describe( 'Collection', () => {
767767
expect( collection.get( 0 ).value ).to.equal( 'foo' );
768768
expect( collection.get( 1 ).value ).to.equal( 'bar' );
769769
} );
770+
771+
it( 'skips when there is no item', () => {
772+
// Add before collection is bound.
773+
items.add( { value: 1, skip: true } );
774+
775+
expect( collection ).to.have.length( 0 );
776+
777+
collection.bindTo( items ).using( item => {
778+
if ( item.skip ) {
779+
return null;
780+
}
781+
782+
return item;
783+
} );
784+
785+
// Still 0 because initial item was skipped.
786+
expect( collection ).to.have.length( 0 );
787+
788+
items.add( { value: 2, skip: false } );
789+
items.add( { value: 3, skip: true } );
790+
items.add( { value: 4, skip: false } );
791+
792+
expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 4 ] );
793+
794+
items.add( { value: 5, skip: false }, 2 );
795+
796+
expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 5, 4 ] );
797+
} );
770798
} );
771799

772800
describe( 'property name', () => {
@@ -802,6 +830,25 @@ describe( 'Collection', () => {
802830
items.remove( 0 );
803831
expect( collection ).to.have.length( 0 );
804832
} );
833+
834+
it( 'skips when there is no item', () => {
835+
items.add( { prop: null } );
836+
837+
collection.bindTo( items ).using( 'prop' );
838+
839+
// Still 0 because initial item was skipped.
840+
expect( collection ).to.have.length( 0 );
841+
842+
items.add( { prop: { value: 2, skip: false } } );
843+
items.add( { prop: null } );
844+
items.add( { prop: { value: 4, skip: false } } );
845+
846+
expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 4 ] );
847+
848+
items.add( { prop: { value: 5 } }, 2 );
849+
850+
expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 5, 4 ] );
851+
} );
805852
} );
806853
} );
807854

@@ -1032,6 +1079,115 @@ describe( 'Collection', () => {
10321079
sinon.assert.callCount( spyRemoveA, 2 );
10331080
sinon.assert.callCount( spyRemoveB, 2 );
10341081
} );
1082+
1083+
describe( 'skipping items', () => {
1084+
let collectionA, collectionB;
1085+
1086+
beforeEach( () => {
1087+
collectionA = new Collection();
1088+
collectionB = new Collection();
1089+
1090+
// A<--->B
1091+
collectionA.bindTo( collectionB ).using( item => {
1092+
if ( item.skip ) {
1093+
return null;
1094+
}
1095+
1096+
return item;
1097+
} );
1098+
1099+
collectionB.bindTo( collectionA ).using( item => {
1100+
if ( item.skip ) {
1101+
return null;
1102+
}
1103+
1104+
return item;
1105+
} );
1106+
} );
1107+
1108+
it( 'should add items at the enf of collections when includes skipped items', () => {
1109+
collectionA.add( { v: 'A' } );
1110+
collectionA.add( { v: 'B', skip: true } );
1111+
collectionA.add( { v: 'C', skip: true } );
1112+
collectionA.add( { v: 'D' } );
1113+
1114+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [] );
1115+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
1116+
assertItems( collectionA, [ 'A', 'B', 'C', 'D' ] );
1117+
assertItems( collectionB, [ 'A', 'D' ] );
1118+
1119+
collectionB.add( { v: 'E' } );
1120+
collectionB.add( { v: 'F', skip: true } );
1121+
collectionB.add( { v: 'G' } );
1122+
1123+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3 ] );
1124+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
1125+
assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'G' ] );
1126+
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G' ] );
1127+
} );
1128+
1129+
it( 'should add items between skipped items', () => {
1130+
collectionA.add( { v: 'A' } );
1131+
collectionA.add( { v: 'B', skip: true } );
1132+
collectionA.add( { v: 'C', skip: true } );
1133+
collectionA.add( { v: 'D' } );
1134+
1135+
collectionB.add( { v: 'E' } );
1136+
collectionB.add( { v: 'F', skip: true } );
1137+
collectionB.add( { v: 'G', skip: true } );
1138+
collectionB.add( { v: 'H' } );
1139+
1140+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
1141+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
1142+
assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'H' ] );
1143+
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );
1144+
1145+
collectionA.add( { v: 'I' }, 2 );
1146+
1147+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 4, 5 ] );
1148+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
1149+
assertItems( collectionA, [ 'A', 'B', 'I', 'C', 'D', 'E', 'H' ] );
1150+
assertItems( collectionB, [ 'A', 'I', 'D', 'E', 'F', 'G', 'H' ] );
1151+
1152+
collectionB.add( { v: 'J' }, 5 );
1153+
1154+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 4, 5 ] );
1155+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
1156+
assertItems( collectionA, [ 'A', 'B', 'I', 'C', 'D', 'E', 'J', 'H' ] );
1157+
assertItems( collectionB, [ 'A', 'I', 'D', 'E', 'F', 'J', 'G', 'H' ] );
1158+
} );
1159+
1160+
it( 'should properly remove skipped items and update skipped indexes', () => {
1161+
collectionA.add( { v: 'A' } );
1162+
collectionA.add( { v: 'B', skip: true } );
1163+
collectionA.add( { v: 'C', skip: true } );
1164+
collectionA.add( { v: 'D' } );
1165+
1166+
collectionB.add( { v: 'E' } );
1167+
collectionB.add( { v: 'F', skip: true } );
1168+
collectionB.add( { v: 'G', skip: true } );
1169+
collectionB.add( { v: 'H' } );
1170+
1171+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
1172+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
1173+
assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'H' ] );
1174+
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );
1175+
1176+
collectionA.remove( 2 );
1177+
1178+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
1179+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1 ] );
1180+
assertItems( collectionA, [ 'A', 'B', 'D', 'E', 'H' ] );
1181+
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );
1182+
1183+
collectionB.remove( 3 );
1184+
1185+
expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3 ] );
1186+
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1 ] );
1187+
assertItems( collectionA, [ 'A', 'B', 'D', 'E', 'H' ] );
1188+
assertItems( collectionB, [ 'A', 'D', 'E', 'G', 'H' ] );
1189+
} );
1190+
} );
10351191
} );
10361192
} );
10371193

0 commit comments

Comments
 (0)