diff --git a/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.js b/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.js index 131c486791c..2aee62de736 100644 --- a/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.js +++ b/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 */ diff --git a/packages/ckeditor5-ui/src/toolbar/toolbarview.js b/packages/ckeditor5-ui/src/toolbar/toolbarview.js index a289cfd97ee..e690f354f03 100644 --- a/packages/ckeditor5-ui/src/toolbar/toolbarview.js +++ b/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' ); + } } /** diff --git a/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js b/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js index 7fc472fee74..d8109e3ae44 100644 --- a/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js +++ b/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, 'b[a]r' ); + + 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, 'b[a]r' ); diff --git a/packages/ckeditor5-ui/tests/toolbar/toolbarview.js b/packages/ckeditor5-ui/tests/toolbar/toolbarview.js index 963d5532c9f..032900920c8 100644 --- a/packages/ckeditor5-ui/tests/toolbar/toolbarview.js +++ b/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()', () => { diff --git a/packages/ckeditor5-utils/src/dom/resizeobserver.js b/packages/ckeditor5-utils/src/dom/resizeobserver.js index d3f0af2207d..31491ab6cfb 100644 --- a/packages/ckeditor5-utils/src/dom/resizeobserver.js +++ b/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 ) { diff --git a/packages/ckeditor5-utils/tests/dom/resizeobserver.js b/packages/ckeditor5-utils/tests/dom/resizeobserver.js index 6f65c854341..a9cabe9c69f 100644 --- a/packages/ckeditor5-utils/tests/dom/resizeobserver.js +++ b/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();