Exposed CKEDITOR.ui.balloonToolbar.reposition #2667
Conversation
|
|
||
| balloonToolbar.reposition( options ); | ||
|
|
||
| assert.areSame( markerElement, spy.args[ 0 ][ 0 ] ); |
There was a problem hiding this comment.
Probably the element is the same, yet tests fail due to the fact that element's wrappers are different. equals should be used here.
| panel.attach( square ); | ||
|
|
||
| CKEDITOR.tools.array.forEach( CKEDITOR.document.find( '.control' ).toArray(), function( input ) { | ||
| input.on( 'change', function() { |
There was a problem hiding this comment.
In newer browsers we could use input instead of change, to dynamically move square when interacting with slider.
plugins/balloontoolbar/plugin.js
Outdated
| this.attach( this._pointedElement, { | ||
| focusElement: false | ||
| } ); | ||
| this.reposition( { focusElement: false } ); |
There was a problem hiding this comment.
You pass parameter to the reposition method here, yet it does not accept any parameters. IMO it should, as it should pass options to the attach.
There was a problem hiding this comment.
I wonder if reposition should really accept options, as balloonToolbar.attach doesn't.
There are two options
show and focusElement, one allows to initially hide balloonToolbarView(balloonPanel) and other to focus some element. I don't really see why this should be done by repositioning method, as it is supposed only to adjust toolbar position when target element moves.
| balloonToolbar.reposition( options ); | ||
|
|
||
| assert.areSame( markerElement, spy.args[ 0 ][ 0 ] ); | ||
| assert.areSame( options, spy.args[ 0 ][ 1 ] ); |
There was a problem hiding this comment.
This statement also can fail as ATM reposition does not support options.
What is the purpose of this pull request?
New Feature
Does your PR contain necessary tests?
All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
What changes did you make?
I've added method to reposition balloon toolbar.
Closes #1496