Merge pull request #7795 from ckeditor/i/6444-toolbar-reposition-on-r…

…egroup

Fix (ui): Balloon toolbar should reposition and ungroup items correctly when the window resizes. Closes #6444.

packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.js

@@ -165,6 +165,15 @@ export default class BalloonToolbar extends Plugin {
 				} );
 			} );
 		}
+
+		// Listen to the toolbar view and whenever it changes its geometry due to some items being
+		// grouped or ungrouped, update the position of the balloon because a shorter/longer toolbar
+		// means the balloon could be pointing at the wrong place. Once updated, the balloon will point
+		// at the right selection in the content again.
+		// https://github.com/ckeditor/ckeditor5/issues/6444
+		this.listenTo( this.toolbarView, 'groupedItemsUpdate', () => {
+			this._updatePosition();
+		} );
 	}
 
 	/**
@@ -236,7 +245,7 @@ export default class BalloonToolbar extends Plugin {
 
 		// Update the toolbar position when the editor ui should be refreshed.
 		this.listenTo( this.editor.ui, 'update', () => {
-			this._balloon.updatePosition( this._getBalloonPositionData() );
+			this._updatePosition();
 		} );
 
 		// Add the toolbar to the common editor contextual balloon.
@@ -300,6 +309,18 @@ export default class BalloonToolbar extends Plugin {
 		};
 	}
 
+	/**
+	 * Updates the position of the {@link #_balloon} to make up for changes:
+	 *
+	 * * in the geometry of the selection it is attached to (e.g. the selection moved in the viewport or expanded or shrunk),
+	 * * or the geometry of the balloon toolbar itself (e.g. the toolbar has grouped or ungrouped some items and it is shorter or longer).
+	 *
+	 * @private
+	 */
+	_updatePosition() {
+		this._balloon.updatePosition( this._getBalloonPositionData() );
+	}
+
 	/**
 	 * @inheritDoc
 	 */

packages/ckeditor5-ui/src/toolbar/toolbarview.js

@@ -304,6 +304,19 @@ export default class ToolbarView extends View {
 			}
 		} ).filter( item => item !== undefined ) );
 	}
+
+	/**
+	 * Fired when some toolbar {@link #items} were grouped or ungrouped as a result of some change
+	 * in the toolbar geometry.
+	 *
+	 * **Note**: This event is always fired **once** regardless of the number of items that were be
+	 * grouped or ungrouped at a time.
+	 *
+	 * **Note**: This event is fired only if the items grouping functionality was enabled in
+	 * the first place (see {@link module:ui/toolbar/toolbarview~ToolbarOptions#shouldGroupWhenFull}).
+	 *
+	 * @event groupedItemsUpdate
+	 */
 }
 
 /**
@@ -418,6 +431,14 @@ class DynamicGrouping {
 	 * is added to.
 	 */
 	constructor( view ) {
+		/**
+		 * A toolbar view this behavior belongs to.
+		 *
+		 * @readonly
+		 * @member {module:ui/toolbar~ToolbarView}
+		 */
+		this.view = view;
+
 		/**
 		 * A collection of toolbar children.
 		 *
@@ -644,6 +665,9 @@ class DynamicGrouping {
 			return;
 		}
 
+		// Remember how many items were initially grouped so at the it is possible to figure out if the number
+		// of grouped items has changed. If the number has changed, geometry of the toolbar has also changed.
+		const initialGroupedItemsCount = this.groupedItems.length;
 		let wereItemsGrouped;
 
 		// Group #items as long as some wrap to the next row. This will happen, for instance,
@@ -672,6 +696,10 @@ class DynamicGrouping {
 				this._groupLastItem();
 			}
 		}
+
+		if ( this.groupedItems.length !== initialGroupedItemsCount ) {
+			this.view.fire( 'groupedItemsUpdate' );
+		}
 	}
 
 	/**

packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js

@@ -361,6 +361,18 @@ describe( 'BalloonToolbar', () => {
 			sinon.assert.calledOnce( spy );
 		} );
 
+		it( 'should update the balloon position whenever #toolbarView fires the #groupedItemsUpdate (it changed its geometry)', () => {
+			setData( model, '<paragraph>b[a]r</paragraph>' );
+
+			const spy = sinon.spy( balloon, 'updatePosition' );
+
+			balloonToolbar.show();
+			sinon.assert.notCalled( spy );
+
+			balloonToolbar.toolbarView.fire( 'groupedItemsUpdate' );
+			sinon.assert.calledOnce( spy );
+		} );
+
 		it( 'should not add #toolbarView to the #_balloon more than once', () => {
 			setData( model, '<paragraph>b[a]r</paragraph>' );
 

packages/ckeditor5-ui/tests/toolbar/toolbarview.js

@@ -828,6 +828,47 @@ describe( 'ToolbarView', () => {
 				expect( ungroupedItems ).to.have.length( 5 );
 				expect( groupedItems ).to.have.length( 0 );
 			} );
+
+			it( 'should fire the "groupedItemsUpdate" event on the toolbar when some item is grouped or ungrouped', () => {
+				const updateSpy = sinon.spy();
+
+				view.on( 'groupedItemsUpdate', updateSpy );
+
+				view.element.style.width = '200px';
+
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+
+				resizeCallback( [ {
+					target: view.element,
+					contentRect: new Rect( view.element )
+				} ] );
+
+				sinon.assert.calledOnce( updateSpy );
+
+				// This 10px is not enough to ungroup an item.
+				view.element.style.width = '210px';
+
+				resizeCallback( [ {
+					target: view.element,
+					contentRect: new Rect( view.element )
+				} ] );
+
+				sinon.assert.calledOnce( updateSpy );
+
+				// But this is not enough to ungroup some items.
+				view.element.style.width = '300px';
+
+				resizeCallback( [ {
+					target: view.element,
+					contentRect: new Rect( view.element )
+				} ] );
+
+				sinon.assert.calledTwice( updateSpy );
+			} );
 		} );
 
 		describe( 'destroy()', () => {

packages/ckeditor5-utils/src/dom/resizeobserver.js

@@ -168,12 +168,6 @@ export default class ResizeObserver {
 
 		ResizeObserver._observerInstance = new ObserverConstructor( entries => {
 			for ( const entry of entries ) {
-				// Do not execute callbacks for elements that are invisible.
-				// https://github.com/ckeditor/ckeditor5/issues/6570
-				if ( !entry.target.offsetParent ) {
-					continue;
-				}
-
 				const callbacks = ResizeObserver._getElementCallbacks( entry.target );
 
 				if ( callbacks ) {

packages/ckeditor5-utils/tests/dom/resizeobserver.js

@@ -114,61 +114,6 @@ describe( 'ResizeObserver()', () => {
 			observerA.destroy();
 		} );
 
-		it( 'should not react to resizing of an element if element is invisible', () => {
-			const callbackA = sinon.spy();
-			let resizeCallback;
-
-			testUtils.sinon.stub( global.window, 'ResizeObserver' ).callsFake( callback => {
-				resizeCallback = callback;
-
-				return {
-					observe() {},
-					unobserve() {}
-				};
-			} );
-
-			const observerA = new ResizeObserver( elementA, callbackA );
-
-			elementA.style.display = 'none';
-
-			resizeCallback( [
-				{ target: elementA }
-			] );
-
-			sinon.assert.notCalled( callbackA );
-
-			observerA.destroy();
-		} );
-
-		it( 'should not react to resizing of an element if element\'s parent is invisible', () => {
-			const callbackA = sinon.spy();
-			let resizeCallback;
-
-			testUtils.sinon.stub( global.window, 'ResizeObserver' ).callsFake( callback => {
-				resizeCallback = callback;
-
-				return {
-					observe() {},
-					unobserve() {}
-				};
-			} );
-
-			const observerA = new ResizeObserver( elementA, callbackA );
-			const parent = document.createElement( 'div' );
-			document.body.appendChild( parent );
-			parent.appendChild( elementA );
-			parent.style.display = 'none';
-
-			resizeCallback( [
-				{ target: elementA }
-			] );
-
-			sinon.assert.notCalled( callbackA );
-
-			parent.remove();
-			observerA.destroy();
-		} );
-
 		it( 'should be able to observe the same element along with other observers', () => {
 			const callbackA = sinon.spy();
 			const callbackB = sinon.spy();