소스 검색

Merge branch 'i/6575' into i/6449

Aleksander Nowodzinski 5 년 전
부모
커밋
717a2dff3a
2개의 변경된 파일82개의 추가작업 그리고 2개의 파일을 삭제
  1. 25 2
      packages/ckeditor5-ui/src/toolbar/toolbarview.js
  2. 57 0
      packages/ckeditor5-ui/tests/toolbar/toolbarview.js

+ 25 - 2
packages/ckeditor5-ui/src/toolbar/toolbarview.js

@@ -525,6 +525,15 @@ class DynamicGrouping {
 		 */
 		this.cachedPadding = null;
 
+		/**
+		 * A flag indicating that an items grouping update has been queued (e.g. due to the toolbar being visible)
+		 * and should be executed immediately the next time the toolbar shows up.
+		 *
+		 * @readonly
+		 * @member {Boolean}
+		 */
+		this.shouldUpdateGroupingOnNextResize = false;
+
 		// Only those items that were not grouped are visible to the user.
 		view.itemsView.children.bindTo( this.ungroupedItems ).using( item => item );
 
@@ -614,11 +623,23 @@ class DynamicGrouping {
 		// Do no grouping–related geometry analysis when the toolbar is detached from visible DOM,
 		// for instance before #render(), or after render but without a parent or a parent detached
 		// from DOM. DOMRects won't work anyway and there will be tons of warning in the console and
-		// nothing else.
+		// nothing else. This happens, for instance, when the toolbar is detached from DOM and
+		// some logic adds or removes its #items.
 		if ( !this.viewElement.ownerDocument.body.contains( this.viewElement ) ) {
 			return;
 		}
 
+		// Do not update grouping when the element is invisible. Such toolbar has DOMRect filled with zeros
+		// and that would cause all items to be grouped. Instead, queue the grouping so it runs next time
+		// the toolbar is visible (the next ResizeObserver callback execution). This is handy because
+		// the grouping could be caused by increasing the #maxWidth when the toolbar was invisible and the next
+		// time it shows up, some items could actually be ungrouped (https://github.com/ckeditor/ckeditor5/issues/6575).
+		if ( !this.viewElement.offsetParent ) {
+			this.shouldUpdateGroupingOnNextResize = true;
+
+			return;
+		}
+
 		let wereItemsGrouped;
 
 		// Group #items as long as some wrap to the next row. This will happen, for instance,
@@ -701,7 +722,9 @@ class DynamicGrouping {
 
 		// TODO: Consider debounce.
 		this.resizeObserver = new ResizeObserver( this.viewElement, entry => {
-			if ( !previousWidth || previousWidth !== entry.contentRect.width ) {
+			if ( !previousWidth || previousWidth !== entry.contentRect.width || this.shouldUpdateGroupingOnNextResize ) {
+				this.shouldUpdateGroupingOnNextResize = false;
+
 				this._updateGrouping();
 
 				previousWidth = entry.contentRect.width;

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

@@ -760,6 +760,63 @@ describe( 'ToolbarView', () => {
 
 				sinon.assert.calledOnce( view._behavior._updateGrouping );
 			} );
+
+			it( 'does not update the state of grouped items if invisible', () => {
+				view.element.style.width = '500px';
+				view.element.style.height = '200px';
+
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+
+				expect( ungroupedItems ).to.have.length( 5 );
+				expect( groupedItems ).to.have.length( 0 );
+
+				view.element.style.display = 'none';
+				view.maxWidth = '100px';
+
+				expect( ungroupedItems ).to.have.length( 5 );
+				expect( groupedItems ).to.have.length( 0 );
+			} );
+
+			it( 'should queue the update of the grouped items state when invisible (and execute it when visible again)', () => {
+				view.maxWidth = '200px';
+
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+
+				expect( ungroupedItems ).to.have.length( 1 );
+				expect( groupedItems ).to.have.length( 4 );
+
+				view.element.style.display = 'none';
+
+				resizeCallback( [ {
+					target: view.element,
+					contentRect: new Rect( view.element )
+				} ] );
+
+				// Response to this change will be queued.
+				view.maxWidth = '500px';
+
+				expect( ungroupedItems ).to.have.length( 1 );
+				expect( groupedItems ).to.have.length( 4 );
+
+				// The queued items state should happen after that.
+				view.element.style.display = 'flex';
+
+				resizeCallback( [ {
+					target: view.element,
+					contentRect: new Rect( view.element )
+				} ] );
+
+				expect( ungroupedItems ).to.have.length( 5 );
+				expect( groupedItems ).to.have.length( 0 );
+			} );
 		} );
 
 		describe( 'destroy()', () => {