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

Commit

Permalink
Merge pull request #93 from ckeditor/t/92
Browse files Browse the repository at this point in the history
Fix: Improved balloon positioning when there is more than one stack in the rotator.
  • Loading branch information
Mgsy authored Jul 23, 2019
2 parents b6ef95e + cec3fc7 commit 763c9ba
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 1 deletion.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
},
"devDependencies": {
"@ckeditor/ckeditor5-basic-styles": "^11.1.3",
"@ckeditor/ckeditor5-block-quote": "^11.1.2",
"@ckeditor/ckeditor5-clipboard": "^12.0.1",
"@ckeditor/ckeditor5-editor-balloon": "^12.2.1",
"@ckeditor/ckeditor5-editor-classic": "^12.1.3",
Expand Down
13 changes: 13 additions & 0 deletions src/widgettoolbarrepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ export default class WidgetToolbarRepository extends Plugin {
*/
_hideToolbar( toolbarDefinition ) {
this._balloon.remove( toolbarDefinition.view );
this.stopListening( this._balloon, 'change:visibleView' );
}

/**
Expand All @@ -209,6 +210,18 @@ export default class WidgetToolbarRepository extends Plugin {
position: getBalloonPositionData( this.editor, relatedElement ),
balloonClassName: toolbarDefinition.balloonClassName,
} );

// Update toolbar position each time stack with toolbar view is switched to visible.
// This is in a case target element has changed when toolbar was in invisible stack
// e.g. target image was wrapped by a block quote.
// See https://github.com/ckeditor/ckeditor5-widget/issues/92.
this.listenTo( this._balloon, 'change:visibleView', () => {
for ( const definition of this._toolbarDefinitions.values() ) {
if ( this._isToolbarVisible( definition ) ) {
this._updateToolbarsVisibility( definition );
}
}
} );
}
}

Expand Down
84 changes: 83 additions & 1 deletion tests/widgettoolbarrepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import BalloonEditor from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor'
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote';
import Widget from '../src/widget';
import WidgetToolbarRepository from '../src/widgettoolbarrepository';
import { isWidget, toWidget } from '../src/utils';
Expand All @@ -31,7 +32,7 @@ describe( 'WidgetToolbarRepository', () => {

return ClassicTestEditor
.create( editorElement, {
plugins: [ Paragraph, FakeButton, WidgetToolbarRepository, FakeWidget, FakeChildWidget ],
plugins: [ Paragraph, FakeButton, WidgetToolbarRepository, FakeWidget, FakeChildWidget, BlockQuote ],
fake: {
toolbar: [ 'fake_button' ]
}
Expand Down Expand Up @@ -331,6 +332,87 @@ describe( 'WidgetToolbarRepository', () => {
expect( updatePositionSpy.firstCall.args[ 0 ].position.target ).to.equal(
view.domConverter.viewToDom( fakeChildViewElement ) );
} );

it( 'should not update balloon position when toolbar is in not visible stack', () => {
const customView = new View();

sinon.spy( balloon.view, 'pin' );

widgetToolbarRepository.register( 'fake', {
items: editor.config.get( 'fake.toolbar' ),
getRelatedElement: getSelectedFakeWidget
} );

setData( model,
'<paragraph>foo</paragraph>' +
'[<fake-widget></fake-widget>]'
);

balloon.add( {
stackId: 'custom',
view: customView,
position: { target: {} }
} );

balloon.showStack( 'custom' );

const fakeWidgetToolbarView = widgetToolbarRepository._toolbarDefinitions.get( 'fake' ).view;

expect( balloon.visibleView ).to.equal( customView );
expect( balloon.hasView( fakeWidgetToolbarView ) ).to.equal( true );

const spy = testUtils.sinon.spy( balloon, 'updatePosition' );

editor.ui.fire( 'update' );

sinon.assert.notCalled( spy );
} );

it( 'should update balloon position when stack with toolbar is switched in rotator to visible', () => {
const view = editor.editing.view;
const customView = new View();

sinon.spy( balloon.view, 'pin' );

widgetToolbarRepository.register( 'fake', {
items: editor.config.get( 'fake.toolbar' ),
getRelatedElement: getSelectedFakeWidget
} );

setData( model,
'<paragraph>foo</paragraph>' +
'[<fake-widget></fake-widget>]'
);

const fakeViewElement = view.document.getRoot().getChild( 1 );
const fakeDomElement = editor.editing.view.domConverter.mapViewToDom( fakeViewElement );
const fakeWidgetToolbarView = widgetToolbarRepository._toolbarDefinitions.get( 'fake' ).view;

expect( balloon.view.pin.lastCall.args[ 0 ].target ).to.equal( fakeDomElement );

balloon.add( {
stackId: 'custom',
view: customView,
position: { target: {} }
} );

balloon.showStack( 'custom' );

expect( balloon.visibleView ).to.equal( customView );
expect( balloon.hasView( fakeWidgetToolbarView ) ).to.equal( true );

editor.execute( 'blockQuote' );
balloon.showStack( 'main' );

expect( balloon.visibleView ).to.equal( fakeWidgetToolbarView );
expect( balloon.hasView( customView ) ).to.equal( true );
expect( balloon.view.pin.lastCall.args[ 0 ].target ).to.not.equal( fakeDomElement );

const newFakeViewElement = view.document.getRoot().getChild( 1 ).getChild( 0 );
const newFakeDomElement = editor.editing.view.domConverter.mapViewToDom( newFakeViewElement );

expect( balloon.view.pin.lastCall.args[ 0 ].target ).to.equal( newFakeDomElement );
} );
} );
} );

Expand Down

0 comments on commit 763c9ba

Please sign in to comment.