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

Fixed: Rect.getIntersectionArea() returns positive area for intersection of rects which has negative width and height (disjoint rects).

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

+ 12 - 3
packages/ckeditor5-utils/src/dom/position.js

@@ -85,8 +85,8 @@ function getPosition( position, targetRect, elementRect ) {
 // @param {utils/dom/rect~Rect} viewportRect A rect of the viewport.
 // @param {utils/dom/rect~Rect} viewportRect A rect of the viewport.
 // @returns {Array} An array containing the name of the position and it's rect.
 // @returns {Array} An array containing the name of the position and it's rect.
 function getBestPosition( positions, targetRect, elementRect, limiterRect, viewportRect ) {
 function getBestPosition( positions, targetRect, elementRect, limiterRect, viewportRect ) {
-	let maxLimiterIntersectArea = -1;
-	let maxViewportIntersectArea = -1;
+	let maxLimiterIntersectArea = 0;
+	let maxViewportIntersectArea = 0;
 	let bestPositionRect;
 	let bestPositionRect;
 	let bestPositionName;
 	let bestPositionName;
 
 
@@ -100,7 +100,16 @@ function getBestPosition( positions, targetRect, elementRect, limiterRect, viewp
 
 
 		if ( limiterRect ) {
 		if ( limiterRect ) {
 			if ( viewportRect ) {
 			if ( viewportRect ) {
-				limiterIntersectArea = limiterRect.getIntersection( viewportRect ).getIntersectionArea( positionRect );
+				// Consider only the part of the limiter which is visible in the viewport. So the limiter is getting limited.
+				const limiterViewportIntersectRect = limiterRect.getIntersection( viewportRect );
+
+				if ( limiterViewportIntersectRect ) {
+					// If the limiter is within the viewport, then check the intersection between that part of the
+					// limiter and actual position.
+					limiterIntersectArea = limiterViewportIntersectRect.getIntersectionArea( positionRect );
+				} else {
+					limiterIntersectArea = 0;
+				}
 			} else {
 			} else {
 				limiterIntersectArea = limiterRect.getIntersectionArea( positionRect );
 				limiterIntersectArea = limiterRect.getIntersectionArea( positionRect );
 			}
 			}

+ 12 - 2
packages/ckeditor5-utils/src/dom/rect.js

@@ -136,7 +136,11 @@ export default class Rect {
 		rect.width = rect.right - rect.left;
 		rect.width = rect.right - rect.left;
 		rect.height = rect.bottom - rect.top;
 		rect.height = rect.bottom - rect.top;
 
 
-		return new Rect( rect );
+		if ( rect.width < 0 || rect.height < 0 ) {
+			return null;
+		} else {
+			return new Rect( rect );
+		}
 	}
 	}
 
 
 	/**
 	/**
@@ -146,7 +150,13 @@ export default class Rect {
 	 * @returns {Number} Area of intersection.
 	 * @returns {Number} Area of intersection.
 	 */
 	 */
 	getIntersectionArea( anotherRect ) {
 	getIntersectionArea( anotherRect ) {
-		return this.getIntersection( anotherRect ).getArea();
+		const rect = this.getIntersection( anotherRect );
+
+		if ( rect ) {
+			return rect.getArea();
+		} else {
+			return 0;
+		}
 	}
 	}
 
 
 	/**
 	/**

+ 26 - 1
packages/ckeditor5-utils/tests/dom/position.js

@@ -217,7 +217,7 @@ describe( 'getOptimalPosition', () => {
 			} );
 			} );
 		} );
 		} );
 
 
-		it( 'should return the very first coordinates no fitting position with a positive intersection has been found', () => {
+		it( 'should return the very first coordinates if no fitting position with a positive intersection has been found', () => {
 			assertPosition( {
 			assertPosition( {
 				element, target, limiter,
 				element, target, limiter,
 				positions: [
 				positions: [
@@ -234,6 +234,27 @@ describe( 'getOptimalPosition', () => {
 				name: 'no-intersect-position'
 				name: 'no-intersect-position'
 			} );
 			} );
 		} );
 		} );
+
+		it( 'should return the very first coordinates if limiter does not fit into the viewport', () => {
+			stubElementRect( limiter, {
+				top: -100,
+				right: -80,
+				bottom: -80,
+				left: -100,
+				width: 20,
+				height: 20
+			} );
+
+			assertPosition( {
+				element, target, limiter,
+				positions: [ attachRight, attachTop ],
+				fitInViewport: true
+			}, {
+				top: 100,
+				left: 10,
+				name: 'right'
+			} );
+		} );
 	} );
 	} );
 } );
 } );
 
 
@@ -298,6 +319,10 @@ function stubWindowScroll( x, y ) {
 }
 }
 
 
 function stubElementRect( element, rect ) {
 function stubElementRect( element, rect ) {
+	if ( element.getBoundingClientRect.restore ) {
+		element.getBoundingClientRect.restore();
+	}
+
 	testUtils.sinon.stub( element, 'getBoundingClientRect' ).returns( rect );
 	testUtils.sinon.stub( element, 'getBoundingClientRect' ).returns( rect );
 }
 }
 
 

+ 1 - 10
packages/ckeditor5-utils/tests/dom/rect.js

@@ -228,16 +228,7 @@ describe( 'Rect', () => {
 				height: 100
 				height: 100
 			} );
 			} );
 
 
-			const insersect = rectA.getIntersection( rectB );
-
-			assertRect( insersect, {
-				top: 100,
-				right: 100,
-				bottom: 100,
-				left: 200,
-				width: -100,
-				height: 0
-			} );
+			expect( rectA.getIntersection( rectB ) ).to.be.null;
 		} );
 		} );
 	} );
 	} );