Selaa lähdekoodia

Merge pull request #7656 from ckeditor/i/6194-collection-getall

Other (ui): Improved toolbar rendering time when multiple items are added or removed at once (e.g. during editor initialization). Closes #6194.

Fix (ui): Removing the first hidden (grouped) toolbar button should not throw an exception. Closes #7655.
Aleksander Nowodzinski 5 vuotta sitten
vanhempi
commit
266dfda77f

+ 26 - 22
packages/ckeditor5-ui/src/toolbar/toolbarview.js

@@ -276,11 +276,11 @@ export default class ToolbarView extends View {
 	 * @param {module:ui/componentfactory~ComponentFactory} factory A factory producing toolbar items.
 	 */
 	fillFromConfig( config, factory ) {
-		config.map( name => {
+		this.items.addMany( config.map( name => {
 			if ( name == '|' ) {
-				this.items.add( new ToolbarSeparatorView() );
+				return new ToolbarSeparatorView();
 			} else if ( factory.has( name ) ) {
-				this.items.add( factory.create( name ) );
+				return factory.create( name );
 			} else {
 				/**
 				 * There was a problem processing the configuration of the toolbar. The item with the given
@@ -302,7 +302,7 @@ export default class ToolbarView extends View {
 				console.warn( attachLinkToDocumentation(
 					'toolbarview-item-unavailable: The requested toolbar item is unavailable.' ), { name } );
 			}
-		} );
+		} ).filter( item => item !== undefined ) );
 	}
 }
 
@@ -545,32 +545,36 @@ class DynamicGrouping {
 		view.children.on( 'add', this._updateFocusCycleableItems.bind( this ) );
 		view.children.on( 'remove', this._updateFocusCycleableItems.bind( this ) );
 
-		// ToolbarView#items is dynamic. When an item is added, it should be automatically
+		// ToolbarView#items is dynamic. When an item is added or removed, it should be automatically
 		// represented in either grouped or ungrouped items at the right index.
 		// In other words #items == concat( #ungroupedItems, #groupedItems )
 		// (in length and order).
-		view.items.on( 'add', ( evt, item, index ) => {
-			if ( index > this.ungroupedItems.length ) {
-				this.groupedItems.add( item, index - this.ungroupedItems.length );
-			} else {
-				this.ungroupedItems.add( item, index );
+		view.items.on( 'change', ( evt, changeData ) => {
+			const index = changeData.index;
+
+			// Removing.
+			for ( const removedItem of changeData.removed ) {
+				if ( index >= this.ungroupedItems.length ) {
+					this.groupedItems.remove( removedItem );
+				} else {
+					this.ungroupedItems.remove( removedItem );
+				}
 			}
 
-			// When a new ungrouped item joins in and lands in #ungroupedItems, there's a chance it causes
-			// the toolbar to overflow.
-			this._updateGrouping();
-		} );
+			// Adding.
+			for ( let currentIndex = index; currentIndex < index + changeData.added.length; currentIndex++ ) {
+				const addedItem = changeData.added[ currentIndex - index ];
 
-		// When an item is removed from ToolbarView#items, it should be automatically
-		// removed from either grouped or ungrouped items.
-		view.items.on( 'remove', ( evt, item, index ) => {
-			if ( index > this.ungroupedItems.length ) {
-				this.groupedItems.remove( item );
-			} else {
-				this.ungroupedItems.remove( item );
+				if ( currentIndex > this.ungroupedItems.length ) {
+					this.groupedItems.add( addedItem, currentIndex - this.ungroupedItems.length );
+				} else {
+					this.ungroupedItems.add( addedItem, currentIndex );
+				}
 			}
 
-			// Whether removed from grouped or ungrouped items, there is a chance
+			// When new ungrouped items join in and land in #ungroupedItems, there's a chance it causes
+			// the toolbar to overflow.
+			// Consequently if removed from grouped or ungrouped items, there is a chance
 			// some new space is available and we could do some ungrouping.
 			this._updateGrouping();
 		} );

+ 11 - 0
packages/ckeditor5-ui/tests/toolbar/toolbarview.js

@@ -575,6 +575,17 @@ describe( 'ToolbarView', () => {
 
 				expect( ungroupedItems.map( i => i ) ).to.have.ordered.members( [ itemB, itemD ] );
 			} );
+
+			it( 'doesn\'t throw when removing the first of grouped items', () => { // (#7655)
+				const items = [ focusable(), focusable(), focusable(), focusable() ];
+				view.element.style.width = '200px';
+				view.items.addMany( items );
+
+				view.items.remove( 1 );
+
+				expect( ungroupedItems.map( i => i ) ).to.have.ordered.members( [ items[ 0 ] ] );
+				expect( groupedItems.map( i => i ) ).to.have.ordered.members( [ items[ 2 ], items[ 3 ] ] );
+			} );
 		} );
 
 		it( 'groups items that overflow into the dropdown', () => {