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

Fix: BalloonPanelView should react to all #scroll events in DOM when the target is not related to any HTMLElement.

Aleksander Nowodzinski 8 лет назад
Родитель
Сommit
9d7f01b0dc

+ 13 - 5
packages/ckeditor5-ui/src/panel/balloon/balloonpanelview.js

@@ -251,19 +251,27 @@ export default class BalloonPanelView extends View {
 		this.attachTo( options );
 
 		const limiter = options.limiter || defaultLimiterElement;
-		let target = null;
+		let targetElement = null;
 
 		// We need to take HTMLElement related to the target if it is possible.
 		if ( isElement( options.target ) ) {
-			target = options.target;
+			targetElement = options.target;
 		} else if ( isRange( options.target ) ) {
-			target = options.target.commonAncestorContainer;
+			targetElement = options.target.commonAncestorContainer;
 		}
 
 		// Then we need to listen on scroll event of eny element in the document.
 		this.listenTo( global.document, 'scroll', ( evt, domEvt ) => {
-			// We need to update position if scrolled element contains related to the balloon elements.
-			if ( ( target && domEvt.target.contains( target ) ) || domEvt.target.contains( limiter ) ) {
+			const scrollTarget = domEvt.target;
+
+			// The position needs to be updated if the positioning target is within the scrolled element.
+			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 );
+
+			// The positioning target can be a Rect, object etc.. There's no way to optimize the listener then.
+			if ( isWithinScrollTarget || isLimiterWithinScrollTarget || !targetElement ) {
 				this.attachTo( options );
 			}
 		}, { useCapture: true } );

+ 12 - 1
packages/ckeditor5-ui/tests/panel/balloon/balloonpanelview.js

@@ -590,7 +590,7 @@ describe( 'BalloonPanelView', () => {
 				sinon.assert.calledTwice( attachToSpy );
 			} );
 
-			it( 'should work for rect as a target', () => {
+			it( 'should work for a Rect as a target', () => {
 				// Just check if this normally works without errors.
 				const rect = {};
 
@@ -602,6 +602,17 @@ describe( 'BalloonPanelView', () => {
 
 				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 = {};
+
+				view.pin( { target: rect } );
+				sinon.assert.calledOnce( attachToSpy );
+
+				notRelatedElement.dispatchEvent( new Event( 'scroll' ) );
+				sinon.assert.calledTwice( attachToSpy );
+			} );
 		} );
 
 		describe( 'unpin()', () => {