8
0
Просмотр исходного кода

Merge pull request #288 from ckeditor/t/260

Feature: Allowed `BalloonPanelView` position limiter defined as a function. Made `ContextualBalloon` position limiter configurable via `#positionLimiter` property. Closes #260.
Oskar Wróbel 8 лет назад
Родитель
Сommit
ef5c57b202

+ 28 - 12
packages/ckeditor5-ui/src/panel/balloon/balloonpanelview.js

@@ -253,15 +253,8 @@ export default class BalloonPanelView extends View {
 	_startPinning( options ) {
 		this.attachTo( options );
 
-		const limiter = options.limiter || defaultLimiterElement;
-		let targetElement = null;
-
-		// We need to take HTMLElement related to the target if it is possible.
-		if ( isElement( options.target ) ) {
-			targetElement = options.target;
-		} else if ( isRange( options.target ) ) {
-			targetElement = options.target.commonAncestorContainer;
-		}
+		const targetElement = getDomElement( options.target );
+		const limiterElement = options.limiter ? getDomElement( options.limiter ) : defaultLimiterElement;
 
 		// Then we need to listen on scroll event of eny element in the document.
 		this.listenTo( global.document, 'scroll', ( evt, domEvt ) => {
@@ -271,10 +264,11 @@ export default class BalloonPanelView extends View {
 			const isWithinScrollTarget = targetElement && scrollTarget.contains( targetElement );
 
 			// The position needs to be updated if the positioning limiter is within the scrolled element.
-			const isLimiterWithinScrollTarget = scrollTarget.contains( limiter );
+			const isLimiterWithinScrollTarget = limiterElement && scrollTarget.contains( limiterElement );
 
-			// The positioning target can be a Rect, object etc.. There's no way to optimize the listener then.
-			if ( isWithinScrollTarget || isLimiterWithinScrollTarget || !targetElement ) {
+			// The positioning target and/or limiter can be a Rect, object etc..
+			// There's no way to optimize the listener then.
+			if ( isWithinScrollTarget || isLimiterWithinScrollTarget || !targetElement || !limiterElement ) {
 				this.attachTo( options );
 			}
 		}, { useCapture: true } );
@@ -296,6 +290,28 @@ export default class BalloonPanelView extends View {
 	}
 }
 
+// Returns the DOM element for given object or null, if there's none,
+// e.g. when passed object is a Rect instance or so.
+//
+// @private
+// @param {*} object
+// @returns {HTMLElement|null}
+function getDomElement( object ) {
+	if ( isElement( object ) ) {
+		return object;
+	}
+
+	if ( isRange( object ) ) {
+		return object.commonAncestorContainer;
+	}
+
+	if ( typeof object == 'function' ) {
+		return getDomElement( object() );
+	}
+
+	return null;
+}
+
 /**
  * A horizontal offset of the arrow tip from the edge of the balloon. Controlled by CSS.
  *

+ 34 - 1
packages/ckeditor5-ui/src/panel/balloon/contextualballoon.js

@@ -10,6 +10,7 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import BalloonPanelView from './balloonpanelview';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
+import first from '@ckeditor/ckeditor5-utils/src/first';
 
 /**
  * Provides the common contextual balloon panel for the editor.
@@ -47,6 +48,28 @@ export default class ContextualBalloon extends Plugin {
 		 */
 		this.view = new BalloonPanelView();
 
+		/**
+		 * The {@link module:utils/dom/position~Options#limiter position limiter}
+		 * for the {@link #view}, used when no `limiter` has been passed into {@link #add}
+		 * or {@link #updatePosition}.
+		 *
+		 * By default, a function, which obtains the farthest DOM
+		 * {@link module:engine/view/rooteditableelement~RootEditableElement}
+		 * of the {@link module:engine/view/document~Document#selection}.
+		 *
+		 * @member {module:utils/dom/position~Options#limiter} #positionLimiter
+		 */
+		this.positionLimiter = () => {
+			const view = this.editor.editing.view;
+			const editableElement = view.selection.editableElement;
+
+			if ( editableElement ) {
+				return view.domConverter.mapViewToDom( editableElement.root );
+			}
+
+			return null;
+		};
+
 		/**
 		 * Stack of the views injected into the balloon. Last one in the stack is displayed
 		 * as a content of {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon#view}.
@@ -196,6 +219,16 @@ export default class ContextualBalloon extends Plugin {
 	 * @returns {module:utils/dom/position~Options}
 	 */
 	_getBalloonPosition() {
-		return this._stack.values().next().value.position;
+		let position = first( this._stack.values() ).position;
+
+		// Use the default limiter if none has been specified.
+		if ( position && !position.limiter ) {
+			// Don't modify the original options object.
+			position = Object.assign( {}, position, {
+				limiter: this.positionLimiter
+			} );
+		}
+
+		return position;
 	}
 }

+ 6 - 4
packages/ckeditor5-ui/src/toolbar/contextual/contextualtoolbar.js

@@ -41,12 +41,14 @@ export default class ContextualToolbar extends Plugin {
 	 * @inheritDoc
 	 */
 	init() {
+		const editor = this.editor;
+
 		/**
 		 * The toolbar view displayed in the balloon.
 		 *
 		 * @member {module:ui/toolbar/toolbarview~ToolbarView}
 		 */
-		this.toolbarView = new ToolbarView( this.editor.locale );
+		this.toolbarView = new ToolbarView( editor.locale );
 
 		Template.extend( this.toolbarView.template, {
 			attributes: {
@@ -63,7 +65,7 @@ export default class ContextualToolbar extends Plugin {
 		 * @private
 		 * @member {module:ui/panel/balloon/contextualballoon~ContextualBalloon}
 		 */
-		this._balloon = this.editor.plugins.get( ContextualBalloon );
+		this._balloon = editor.plugins.get( ContextualBalloon );
 
 		/**
 		 * Fires {@link #event:_selectionChangeDebounced} event using `lodash#debounce`.
@@ -196,7 +198,8 @@ export default class ContextualToolbar extends Plugin {
 	 * @returns {module:utils/dom/position~Options}
 	 */
 	_getBalloonPositionData() {
-		const editingView = this.editor.editing.view;
+		const editor = this.editor;
+		const editingView = editor.editing.view;
 
 		// Get direction of the selection.
 		const isBackward = editingView.selection.isBackward;
@@ -213,7 +216,6 @@ export default class ContextualToolbar extends Plugin {
 				// Select the proper range rect depending on the direction of the selection.
 				return rangeRects[ isBackward ? 0 : rangeRects.length - 1 ];
 			},
-			limiter: this.editor.ui.view.editable.element,
 			positions: getBalloonPositions( isBackward )
 		};
 	}

+ 27 - 0
packages/ckeditor5-ui/tests/panel/balloon/balloonpanelview.js

@@ -614,6 +614,22 @@ describe( 'BalloonPanelView', () => {
 				sinon.assert.calledTwice( attachToSpy );
 			} );
 
+			it( 'should work for a function as a target/limiter', () => {
+				// Just check if this normally works without errors.
+				const rect = {};
+
+				view.pin( {
+					target() { return rect; },
+					limiter() { return limiter; }
+				} );
+
+				sinon.assert.calledOnce( attachToSpy );
+
+				limiter.dispatchEvent( new Event( 'scroll' ) );
+
+				sinon.assert.calledTwice( attachToSpy );
+			} );
+
 			// https://github.com/ckeditor/ckeditor5-ui/issues/227
 			it( 'should react to #scroll from anywhere when the target is not an HTMLElement or Range', () => {
 				const rect = {};
@@ -624,6 +640,17 @@ describe( 'BalloonPanelView', () => {
 				notRelatedElement.dispatchEvent( new Event( 'scroll' ) );
 				sinon.assert.calledTwice( attachToSpy );
 			} );
+
+			// https://github.com/ckeditor/ckeditor5-ui/issues/260
+			it( 'should react to #scroll from anywhere when the limiter is not an HTMLElement` or Range', () => {
+				const rect = {};
+
+				view.pin( { target, limiter: rect } );
+				sinon.assert.calledOnce( attachToSpy );
+
+				notRelatedElement.dispatchEvent( new Event( 'scroll' ) );
+				sinon.assert.calledTwice( attachToSpy );
+			} );
 		} );
 
 		describe( 'unpin()', () => {

+ 194 - 22
packages/ckeditor5-ui/tests/panel/balloon/contextualballoon.js

@@ -9,6 +9,11 @@ import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview';
 import View from '../../../src/view';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
+import ViewEditableElement from '@ckeditor/ckeditor5-engine/src/view/editableelement';
+import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
+import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 /* global document, Event */
 
@@ -21,7 +26,7 @@ describe( 'ContextualBalloon', () => {
 
 		return ClassicTestEditor
 			.create( editorElement, {
-				plugins: [ ContextualBalloon ]
+				plugins: [ Paragraph, ContextualBalloon ]
 			} )
 			.then( newEditor => {
 				editor = newEditor;
@@ -38,7 +43,9 @@ describe( 'ContextualBalloon', () => {
 				// Add viewA to the pane and init viewB.
 				balloon.add( {
 					view: viewA,
-					position: { target: 'fake' }
+					position: {
+						target: 'fake'
+					}
 				} );
 
 				viewB.init();
@@ -65,6 +72,51 @@ describe( 'ContextualBalloon', () => {
 			expect( balloon.view ).to.instanceof( BalloonPanelView );
 		} );
 
+		describe( 'positionLimiter', () => {
+			let doc, viewDocument, root;
+
+			beforeEach( () => {
+				doc = editor.document;
+				viewDocument = editor.editing.view;
+				root = viewDocument.getRoot();
+			} );
+
+			it( 'obtains the root of the selection', () => {
+				setModelData( doc, '<paragraph>[]bar</paragraph>' );
+
+				expect( balloon.positionLimiter() ).to.equal( viewDocument.domConverter.mapViewToDom( root ) );
+			} );
+
+			it( 'does not fail if selection has no #editableElement', () => {
+				sinon.stub( viewDocument.selection, 'editableElement' ).value( null );
+
+				expect( balloon.positionLimiter() ).to.equal( null );
+			} );
+
+			it( 'obtains the farthest root of the selection (nested editable)', () => {
+				doc.schema.registerItem( 'widget' );
+				doc.schema.registerItem( 'nestededitable' );
+
+				doc.schema.objects.add( 'widget' );
+
+				doc.schema.allow( { name: 'widget', inside: '$root' } );
+				doc.schema.allow( { name: 'nestededitable', inside: 'widget' } );
+				doc.schema.allow( { name: '$inline', inside: 'nestededitable' } );
+
+				buildModelConverter().for( editor.data.modelToView, editor.editing.modelToView )
+					.fromElement( 'widget' )
+					.toElement( () => new ViewContainerElement( 'figure', { contenteditable: 'false' } ) );
+
+				buildModelConverter().for( editor.data.modelToView, editor.editing.modelToView )
+					.fromElement( 'nestededitable' )
+					.toElement( () => new ViewEditableElement( 'figcaption', { contenteditable: 'true' } ) );
+
+				setModelData( doc, '<widget><nestededitable>[]foo</nestededitable></widget>' );
+
+				expect( balloon.positionLimiter() ).to.equal( viewDocument.domConverter.mapViewToDom( root ) );
+			} );
+		} );
+
 		it( 'should add balloon panel view to editor `body` collection', () => {
 			expect( editor.ui.view.body.getIndex( balloon.view ) ).to.above( -1 );
 		} );
@@ -74,7 +126,10 @@ describe( 'ContextualBalloon', () => {
 
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' }
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				}
 			} );
 
 			balloon.view.element.dispatchEvent( new Event( 'focus' ) );
@@ -91,7 +146,10 @@ describe( 'ContextualBalloon', () => {
 		it( 'should return true when given view is in stack but is not visible', () => {
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' }
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				}
 			} );
 
 			expect( balloon.visibleView ).to.equal( viewB );
@@ -108,7 +166,65 @@ describe( 'ContextualBalloon', () => {
 			expect( balloon.view.content.length ).to.equal( 1 );
 			expect( balloon.view.content.get( 0 ) ).to.deep.equal( viewA );
 			expect( balloon.view.pin.calledOnce ).to.true;
-			expect( balloon.view.pin.firstCall.args[ 0 ] ).to.deep.equal( { target: 'fake' } );
+			sinon.assert.calledWithMatch( balloon.view.pin.firstCall, {
+				target: 'fake',
+				limiter: balloon.positionLimiter
+			} );
+		} );
+
+		it( 'should use a provided limiter instead of #positionLimiter', () => {
+			balloon.remove( viewA );
+			balloon.view.pin.resetHistory();
+
+			balloon.add( {
+				view: viewB,
+				position: {
+					target: 'foo',
+					limiter: 'customLimiter'
+				}
+			} );
+
+			sinon.assert.calledWithMatch( balloon.view.pin, {
+				target: 'foo',
+				limiter: 'customLimiter'
+			} );
+		} );
+
+		it( 'should use a custom #positionLimiter', () => {
+			balloon.remove( viewA );
+			balloon.view.pin.resetHistory();
+			balloon.positionLimiter = 'customLimiter';
+
+			balloon.add( {
+				view: viewB,
+				position: {
+					target: 'foo',
+				}
+			} );
+
+			sinon.assert.calledWithMatch( balloon.view.pin, {
+				target: 'foo',
+				limiter: 'customLimiter'
+			} );
+		} );
+
+		it( 'should not alter the view data if no limiter is provided and the #positionLimiter is used', () => {
+			const data = {
+				view: viewB,
+				position: {
+					target: 'foo',
+				}
+			};
+
+			balloon.remove( viewA );
+			balloon.add( data );
+
+			expect( data ).to.deep.equal( {
+				view: viewB,
+				position: {
+					target: 'foo'
+				}
+			} );
 		} );
 
 		it( 'should pin balloon to the target element', () => {
@@ -119,7 +235,10 @@ describe( 'ContextualBalloon', () => {
 			expect( () => {
 				balloon.add( {
 					view: viewA,
-					position: { target: 'fake' }
+					position: {
+						target: 'fake',
+						limiter: balloon.positionLimiter
+					}
 				} );
 			} ).to.throw( CKEditorError, /^contextualballoon-add-view-exist/ );
 		} );
@@ -127,7 +246,10 @@ describe( 'ContextualBalloon', () => {
 		it( 'should add multiple views to he stack and display last one', () => {
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' }
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				}
 			} );
 
 			expect( balloon.view.content.length ).to.equal( 1 );
@@ -142,12 +264,14 @@ describe( 'ContextualBalloon', () => {
 
 			expect( balloon.view.pin.calledTwice ).to.true;
 
-			expect( balloon.view.pin.firstCall.args[ 0 ] ).to.deep.equal( {
-				target: 'fake'
+			sinon.assert.calledWithMatch( balloon.view.pin.firstCall, {
+				target: 'fake',
+				limiter: balloon.positionLimiter
 			} );
 
-			expect( balloon.view.pin.secondCall.args[ 0 ] ).to.deep.equal( {
-				target: 'fake'
+			sinon.assert.calledWithMatch( balloon.view.pin.secondCall, {
+				target: 'fake',
+				limiter: balloon.positionLimiter
 			} );
 		} );
 
@@ -156,7 +280,10 @@ describe( 'ContextualBalloon', () => {
 
 			balloon.add( {
 				view,
-				position: { target: 'fake' },
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				},
 				balloonClassName: 'foo'
 			} );
 
@@ -164,7 +291,10 @@ describe( 'ContextualBalloon', () => {
 
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' },
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				},
 				balloonClassName: 'bar'
 			} );
 
@@ -180,7 +310,10 @@ describe( 'ContextualBalloon', () => {
 		it( 'should return data of currently visible view when there is more than one in the stack', () => {
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' }
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				}
 			} );
 
 			expect( balloon.visibleView ).to.equal( viewB );
@@ -205,7 +338,10 @@ describe( 'ContextualBalloon', () => {
 		it( 'should remove given view and set preceding in the stack as visible when removed view was visible', () => {
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' }
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				}
 			} );
 
 			balloon.remove( viewB );
@@ -216,7 +352,10 @@ describe( 'ContextualBalloon', () => {
 		it( 'should remove given view from the stack when view is not visible', () => {
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' }
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				}
 			} );
 
 			balloon.remove( viewA );
@@ -235,13 +374,19 @@ describe( 'ContextualBalloon', () => {
 
 			balloon.add( {
 				view,
-				position: { target: 'fake' },
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				},
 				balloonClassName: 'foo'
 			} );
 
 			balloon.add( {
 				view: viewB,
-				position: { target: 'fake' },
+				position: {
+					target: 'fake',
+					limiter: balloon.positionLimiter
+				},
 				balloonClassName: 'bar'
 			} );
 
@@ -265,7 +410,10 @@ describe( 'ContextualBalloon', () => {
 			balloon.updatePosition();
 
 			expect( balloon.view.attachTo.calledOnce );
-			expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'fake' } );
+			sinon.assert.calledWithMatch( balloon.view.attachTo.firstCall, {
+				target: 'fake',
+				limiter: balloon.positionLimiter
+			} );
 		} );
 
 		it( 'should set given position to the currently visible view and use position from the first view in the stack #1', () => {
@@ -274,7 +422,10 @@ describe( 'ContextualBalloon', () => {
 			balloon.updatePosition( { target: 'new' } );
 
 			expect( balloon.view.attachTo.calledOnce );
-			expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'new' } );
+			sinon.assert.calledWithMatch( balloon.view.attachTo.firstCall, {
+				target: 'new',
+				limiter: balloon.positionLimiter
+			} );
 		} );
 
 		it( 'should set given position to the currently visible view and use position from the first view in the stack #2', () => {
@@ -290,14 +441,35 @@ describe( 'ContextualBalloon', () => {
 			balloon.updatePosition( { target: 'new' } );
 
 			expect( balloon.view.attachTo.calledOnce );
-			expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'fake' } );
+			sinon.assert.calledWithMatch( balloon.view.attachTo.firstCall, {
+				target: 'fake',
+				limiter: balloon.positionLimiter
+			} );
 
 			balloon.remove( viewA );
 
 			balloon.updatePosition();
 
 			expect( balloon.view.attachTo.calledTwice );
-			expect( balloon.view.attachTo.secondCall.args[ 0 ] ).to.deep.equal( { target: 'new' } );
+			sinon.assert.calledWithMatch( balloon.view.attachTo.secondCall, {
+				target: 'new',
+				limiter: balloon.positionLimiter
+			} );
+		} );
+
+		it( 'should use a given position limiter instead of the default one', () => {
+			balloon.view.attachTo.reset();
+
+			balloon.updatePosition( {
+				target: 'new',
+				limiter: 'customLimiter'
+			} );
+
+			expect( balloon.view.attachTo.calledOnce );
+			sinon.assert.calledWithMatch( balloon.view.attachTo.firstCall, {
+				target: 'new',
+				limiter: 'customLimiter'
+			} );
 		} );
 
 		it( 'should throw an error when there is no given view in the stack', () => {

+ 0 - 2
packages/ckeditor5-ui/tests/toolbar/contextual/contextualtoolbar.js

@@ -154,7 +154,6 @@ describe( 'ContextualToolbar', () => {
 				balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container',
 				position: {
 					target: sinon.match.func,
-					limiter: editor.ui.view.editable.element,
 					positions: [
 						defaultPositions.southEastArrowNorth,
 						defaultPositions.southEastArrowNorthEast,
@@ -181,7 +180,6 @@ describe( 'ContextualToolbar', () => {
 				balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container',
 				position: {
 					target: sinon.match.func,
-					limiter: editor.ui.view.editable.element,
 					positions: [
 						defaultPositions.northWestArrowSouth,
 						defaultPositions.northWestArrowSouthWest,