Przeglądaj źródła

Merge pull request #329 from ckeditor/i/6181

Fix: The `getOptimalPosition()` helper should prefer positions that fit inside the viewport even though there are some others that fit better into the limiter. Closes ckeditor/ckeditor5#6181.
Aleksander Nowodzinski 5 lat temu
rodzic
commit
ea3f1f7749

+ 175 - 79
packages/ckeditor5-utils/src/dom/position.js

@@ -94,54 +94,33 @@ export function getOptimalPosition( { element, target, positions, limiter, fitIn
 	const elementRect = new Rect( element );
 	const targetRect = new Rect( target );
 
-	let bestPosition;
-	let name;
+	let bestPositionRect;
+	let bestPositionName;
 
 	// If there are no limits, just grab the very first position and be done with that drama.
 	if ( !limiter && !fitInViewport ) {
-		[ name, bestPosition ] = getPosition( positions[ 0 ], targetRect, elementRect );
+		[ bestPositionName, bestPositionRect ] = getPositionNameAndRect( positions[ 0 ], targetRect, elementRect );
 	} else {
 		const limiterRect = limiter && new Rect( limiter ).getVisible();
 		const viewportRect = fitInViewport && new Rect( global.window );
+		const bestPosition = getBestPositionNameAndRect( positions, { targetRect, elementRect, limiterRect, viewportRect } );
 
-		[ name, bestPosition ] =
-			getBestPosition( positions, targetRect, elementRect, limiterRect, viewportRect ) ||
-			// If there's no best position found, i.e. when all intersections have no area because
-			// rects have no width or height, then just use the first available position.
-			getPosition( positions[ 0 ], targetRect, elementRect );
+		// If there's no best position found, i.e. when all intersections have no area because
+		// rects have no width or height, then just use the first available position.
+		[ bestPositionName, bestPositionRect ] = bestPosition || getPositionNameAndRect( positions[ 0 ], targetRect, elementRect );
 	}
 
-	let { left, top } = getAbsoluteRectCoordinates( bestPosition );
+	let absoluteRectCoordinates = getAbsoluteRectCoordinates( bestPositionRect );
 
 	if ( positionedElementAncestor ) {
-		const ancestorPosition = getAbsoluteRectCoordinates( new Rect( positionedElementAncestor ) );
-		const ancestorBorderWidths = getBorderWidths( positionedElementAncestor );
-
-		// (https://github.com/ckeditor/ckeditor5-ui-default/issues/126)
-		// If there's some positioned ancestor of the panel, then its `Rect` must be taken into
-		// consideration. `Rect` is always relative to the viewport while `position: absolute` works
-		// with respect to that positioned ancestor.
-		left -= ancestorPosition.left;
-		top -= ancestorPosition.top;
-
-		// (https://github.com/ckeditor/ckeditor5-utils/issues/139)
-		// If there's some positioned ancestor of the panel, not only its position must be taken into
-		// consideration (see above) but also its internal scrolls. Scroll have an impact here because `Rect`
-		// is relative to the viewport (it doesn't care about scrolling), while `position: absolute`
-		// must compensate that scrolling.
-		left += positionedElementAncestor.scrollLeft;
-		top += positionedElementAncestor.scrollTop;
-
-		// (https://github.com/ckeditor/ckeditor5-utils/issues/139)
-		// If there's some positioned ancestor of the panel, then its `Rect` includes its CSS `borderWidth`
-		// while `position: absolute` positioning does not consider it.
-		// E.g. `{ position: absolute, top: 0, left: 0 }` means upper left corner of the element,
-		// not upper-left corner of its border.
-		left -= ancestorBorderWidths.left;
-		top -= ancestorBorderWidths.top;
+		absoluteRectCoordinates = shiftRectCoordinatesDueToPositionedAncestor( absoluteRectCoordinates, positionedElementAncestor );
 	}
 
-	return { left, top, name };
+	return {
+		left: absoluteRectCoordinates.left,
+		top: absoluteRectCoordinates.top,
+		name: bestPositionName
+	};
 }
 
 // For given position function, returns a corresponding `Rect` instance.
@@ -151,7 +130,7 @@ export function getOptimalPosition( { element, target, positions, limiter, fitIn
 // @param {utils/dom/rect~Rect} targetRect A rect of the target.
 // @param {utils/dom/rect~Rect} elementRect A rect of positioned element.
 // @returns {Array} An array containing position name and its Rect.
-function getPosition( position, targetRect, elementRect ) {
+function getPositionNameAndRect( position, targetRect, elementRect ) {
 	const { left, top, name } = position( targetRect, elementRect );
 
 	return [ name, elementRect.clone().moveTo( left, top ) ];
@@ -161,26 +140,74 @@ function getPosition( position, targetRect, elementRect ) {
 // fit of the `elementRect` into the `limiterRect` and `viewportRect`.
 //
 // @private
-// @param {module:utils/dom/position~Options#positions} positions Functions returning
-// {@link module:utils/dom/position~Position} to be checked, in the order of preference.
-// @param {utils/dom/rect~Rect} targetRect A rect of the {@link module:utils/dom/position~Options#target}.
-// @param {utils/dom/rect~Rect} elementRect A rect of positioned {@link module:utils/dom/position~Options#element}.
-// @param {utils/dom/rect~Rect} limiterRect A rect of the {@link module:utils/dom/position~Options#limiter}.
-// @param {utils/dom/rect~Rect} viewportRect A rect of the viewport.
+//
+// @param {Object} options
+// @param {module:utils/dom/position~Options#positions} positions Functions returning {@link module:utils/dom/position~Position}
+// to be checked, in the order of preference.
+// @param {Object} options
+// @param {utils/dom/rect~Rect} options.targetRect A rect of the {@link module:utils/dom/position~Options#target}.
+// @param {utils/dom/rect~Rect} options.elementRect A rect of positioned {@link module:utils/dom/position~Options#element}.
+// @param {utils/dom/rect~Rect} options.limiterRect A rect of the {@link module:utils/dom/position~Options#limiter}.
+// @param {utils/dom/rect~Rect} options.viewportRect A rect of the viewport.
+//
 // @returns {Array} An array containing the name of the position and it's rect.
-function getBestPosition( positions, targetRect, elementRect, limiterRect, viewportRect ) {
-	let maxLimiterIntersectArea = 0;
-	let maxViewportIntersectArea = 0;
-	let bestPositionRect;
-	let bestPositionName;
+function getBestPositionNameAndRect( positions, options ) {
+	const { elementRect, viewportRect } = options;
 
 	// This is when element is fully visible.
 	const elementRectArea = elementRect.getArea();
 
-	positions.some( position => {
-		const [ positionName, positionRect ] = getPosition( position, targetRect, elementRect );
-		let limiterIntersectArea;
-		let viewportIntersectArea;
+	// Let's calculate intersection areas for positions. It will end early if best match is found.
+	const processedPositions = processPositionsToAreas( positions, options );
+
+	// First let's check all positions that fully fit in the viewport.
+	if ( viewportRect ) {
+		const processedPositionsInViewport = processedPositions.filter( ( { viewportIntersectArea } ) => {
+			return viewportIntersectArea === elementRectArea;
+		} );
+
+		// Try to find best position from those which fit completely in viewport.
+		const bestPositionData = getBestOfProcessedPositions( processedPositionsInViewport, elementRectArea );
+
+		if ( bestPositionData ) {
+			return bestPositionData;
+		}
+	}
+
+	// Either there is no viewportRect or there is no position that fits completely in the viewport.
+	return getBestOfProcessedPositions( processedPositions, elementRectArea );
+}
+
+// For a given array of positioning functions, calculates intersection areas for them.
+//
+// Note: If some position fully fits into the `limiterRect`, it will be returned early, without further consideration
+// of other positions.
+//
+// @private
+//
+// @param {module:utils/dom/position~Options#positions} positions Functions returning {@link module:utils/dom/position~Position}
+// to be checked, in the order of preference.
+// @param {Object} options
+// @param {utils/dom/rect~Rect} options.targetRect A rect of the {@link module:utils/dom/position~Options#target}.
+// @param {utils/dom/rect~Rect} options.elementRect A rect of positioned {@link module:utils/dom/position~Options#element}.
+// @param {utils/dom/rect~Rect} options.limiterRect A rect of the {@link module:utils/dom/position~Options#limiter}.
+// @param {utils/dom/rect~Rect} options.viewportRect A rect of the viewport.
+//
+// @returns {Array.<Object>} Array of positions with calculated intersection areas. Each item is an object containing:
+// * {String} positionName Name of position.
+// * {utils/dom/rect~Rect} positionRect Rect of position.
+// * {Number} limiterIntersectArea Area of intersection of the position with limiter part that is in the viewport.
+// * {Number} viewportIntersectArea Area of intersection of the position with viewport.
+function processPositionsToAreas( positions, { targetRect, elementRect, limiterRect, viewportRect } ) {
+	const processedPositions = [];
+
+	// This is when element is fully visible.
+	const elementRectArea = elementRect.getArea();
+
+	for ( const position of positions ) {
+		const [ positionName, positionRect ] = getPositionNameAndRect( position, targetRect, elementRect );
+		let limiterIntersectArea = 0;
+		let viewportIntersectArea = 0;
 
 		if ( limiterRect ) {
 			if ( viewportRect ) {
@@ -191,8 +218,6 @@ function getBestPosition( positions, targetRect, elementRect, limiterRect, viewp
 					// 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 {
 				limiterIntersectArea = limiterRect.getIntersectionArea( positionRect );
@@ -203,42 +228,113 @@ function getBestPosition( positions, targetRect, elementRect, limiterRect, viewp
 			viewportIntersectArea = viewportRect.getIntersectionArea( positionRect );
 		}
 
-		// The only criterion: intersection with the viewport.
-		if ( viewportRect && !limiterRect ) {
-			if ( viewportIntersectArea > maxViewportIntersectArea ) {
-				setBestPosition();
-			}
-		}
-		// The only criterion: intersection with the limiter.
-		else if ( !viewportRect && limiterRect ) {
-			if ( limiterIntersectArea > maxLimiterIntersectArea ) {
-				setBestPosition();
-			}
+		const processedPosition = {
+			positionName,
+			positionRect,
+			limiterIntersectArea,
+			viewportIntersectArea
+		};
+
+		// If a such position is found that element is fully contained by the limiter then, obviously,
+		// there will be no better one, so finishing.
+		if ( limiterIntersectArea === elementRectArea ) {
+			return [ processedPosition ];
 		}
-		// Two criteria: intersection with the viewport and the limiter visible in the viewport.
-		else {
-			if ( viewportIntersectArea > maxViewportIntersectArea && limiterIntersectArea >= maxLimiterIntersectArea ) {
-				setBestPosition();
-			} else if ( viewportIntersectArea >= maxViewportIntersectArea && limiterIntersectArea > maxLimiterIntersectArea ) {
-				setBestPosition();
-			}
+
+		processedPositions.push( processedPosition );
+	}
+
+	return processedPositions;
+}
+
+// For a given array of processed position data (with calculated Rects for positions and intersection areas)
+// returns such that provides the best fit of the `elementRect` into the `limiterRect` and `viewportRect` at the same time.
+//
+// **Note**: It will return early if some position fully fits into the `limiterRect`.
+//
+// @private
+// @param {Array.<Object>} Array of positions with calculated intersection areas (in order of preference).
+// Each item is an object containing:
+//
+//	* {String} positionName Name of position.
+//	* {utils/dom/rect~Rect} positionRect Rect of position.
+//	* {Number} limiterIntersectArea Area of intersection of the position with limiter part that is in the viewport.
+//	* {Number} viewportIntersectArea Area of intersection of the position with viewport.
+//
+// @param {Number} elementRectArea Area of positioned {@link module:utils/dom/position~Options#element}.
+// @returns {Array|null} An array containing the name of the position and it's rect, or null if not found.
+function getBestOfProcessedPositions( processedPositions, elementRectArea ) {
+	let maxFitFactor = 0;
+	let bestPositionRect;
+	let bestPositionName;
+
+	for ( const { positionName, positionRect, limiterIntersectArea, viewportIntersectArea } of processedPositions ) {
+		// If a such position is found that element is fully container by the limiter then, obviously,
+		// there will be no better one, so finishing.
+		if ( limiterIntersectArea === elementRectArea ) {
+			return [ positionName, positionRect ];
 		}
 
-		function setBestPosition() {
-			maxViewportIntersectArea = viewportIntersectArea;
-			maxLimiterIntersectArea = limiterIntersectArea;
+		// To maximize both viewport and limiter intersection areas we use distance on viewportIntersectArea
+		// and limiterIntersectArea plane (without sqrt because we are looking for max value).
+		const fitFactor = viewportIntersectArea ** 2 + limiterIntersectArea ** 2;
+
+		if ( fitFactor > maxFitFactor ) {
+			maxFitFactor = fitFactor;
 			bestPositionRect = positionRect;
 			bestPositionName = positionName;
 		}
-
-		// If a such position is found that element is fully container by the limiter then, obviously,
-		// there will be no better one, so finishing.
-		return limiterIntersectArea === elementRectArea;
-	} );
+	}
 
 	return bestPositionRect ? [ bestPositionName, bestPositionRect ] : null;
 }
 
+// For a given absolute Rect coordinates object and a positioned element ancestor, it returns an object with
+// new Rect coordinates that make up for the position and the scroll of the ancestor.
+//
+// This is necessary because while Rects (and DOMRects) are relative to the browser's viewport, their coordinates
+// are used in real–life to position elements with `position: absolute`, which are scoped by any positioned
+// (and scrollable) ancestors.
+//
+// @private
+//
+// @param {Object} absoluteRectCoordinates An object with absolute rect coordinates.
+// @param {Object} absoluteRectCoordinates.top
+// @param {Object} absoluteRectCoordinates.left
+// @param {HTMLElement} positionedElementAncestor An ancestor element that should be considered.
+//
+// @returns {Object} An object corresponding to `absoluteRectCoordinates` input but with values shifted
+// to make up for the positioned element ancestor.
+function shiftRectCoordinatesDueToPositionedAncestor( { left, top }, positionedElementAncestor ) {
+	const ancestorPosition = getAbsoluteRectCoordinates( new Rect( positionedElementAncestor ) );
+	const ancestorBorderWidths = getBorderWidths( positionedElementAncestor );
+
+	// (https://github.com/ckeditor/ckeditor5-ui-default/issues/126)
+	// If there's some positioned ancestor of the panel, then its `Rect` must be taken into
+	// consideration. `Rect` is always relative to the viewport while `position: absolute` works
+	// with respect to that positioned ancestor.
+	left -= ancestorPosition.left;
+	top -= ancestorPosition.top;
+
+	// (https://github.com/ckeditor/ckeditor5-utils/issues/139)
+	// If there's some positioned ancestor of the panel, not only its position must be taken into
+	// consideration (see above) but also its internal scrolls. Scroll have an impact here because `Rect`
+	// is relative to the viewport (it doesn't care about scrolling), while `position: absolute`
+	// must compensate that scrolling.
+	left += positionedElementAncestor.scrollLeft;
+	top += positionedElementAncestor.scrollTop;
+
+	// (https://github.com/ckeditor/ckeditor5-utils/issues/139)
+	// If there's some positioned ancestor of the panel, then its `Rect` includes its CSS `borderWidth`
+	// while `position: absolute` positioning does not consider it.
+	// E.g. `{ position: absolute, top: 0, left: 0 }` means upper left corner of the element,
+	// not upper-left corner of its border.
+	left -= ancestorBorderWidths.left;
+	top -= ancestorBorderWidths.top;
+
+	return { left, top };
+}
+
 // DOMRect (also Rect) works in a scroll–independent geometry but `position: absolute` doesn't.
 // This function converts Rect to `position: absolute` coordinates.
 //

+ 305 - 54
packages/ckeditor5-utils/tests/dom/position.js

@@ -13,20 +13,42 @@ let element, target, limiter;
 
 //	+--------+-----+
 //	|    E   |  T  |
-//	+--------+-----+
-const attachLeft = ( targetRect, elementRect ) => ( {
+//	|        |-----+
+//  +--------+
+const attachLeftBottom = ( targetRect, elementRect ) => ( {
 	top: targetRect.top,
 	left: targetRect.left - elementRect.width,
-	name: 'left'
+	name: 'left-bottom'
+} );
+
+//	+--------+
+//	|    E   |-----+
+//	|        |  T  |
+//  +--------+-----+
+const attachLeftTop = ( targetRect, elementRect ) => ( {
+	top: targetRect.bottom - elementRect.height,
+	left: targetRect.left - elementRect.width,
+	name: 'left-top'
 } );
 
 //	+-----+--------+
 //	|  T  |    E   |
-//	+-----+--------+
-const attachRight = targetRect => ( {
+//	+-----|        |
+//        +--------+
+const attachRightBottom = targetRect => ( {
 	top: targetRect.top,
-	left: targetRect.left + targetRect.width,
-	name: 'right'
+	left: targetRect.right,
+	name: 'right-bottom'
+} );
+
+//	      +--------+
+//	+-----|    E   |
+//	|  T  |        |
+//  +-----+--------+
+const attachRightTop = ( targetRect, elementRect ) => ( {
+	top: targetRect.bottom - elementRect.height,
+	left: targetRect.right,
+	name: 'right-top'
 } );
 
 //	+-----+
@@ -34,10 +56,21 @@ const attachRight = targetRect => ( {
 //	+-----+--+
 //	|    E   |
 //	+--------+
-const attachBottom = targetRect => ( {
+const attachBottomRight = targetRect => ( {
 	top: targetRect.bottom,
 	left: targetRect.left,
-	name: 'bottom'
+	name: 'bottom-right'
+} );
+
+//	   +-----+
+//	   |  T  |
+//	+--+-----+
+//	|    E   |
+//	+--------+
+const attachBottomLeft = ( targetRect, elementRect ) => ( {
+	top: targetRect.bottom,
+	left: targetRect.right - elementRect.width,
+	name: 'bottom-left'
 } );
 
 //	+--------+
@@ -45,12 +78,34 @@ const attachBottom = targetRect => ( {
 //	+--+-----+
 //	   |  T  |
 //	   +-----+
-const attachTop = ( targetRect, elementRect ) => ( {
+const attachTopLeft = ( targetRect, elementRect ) => ( {
 	top: targetRect.top - elementRect.height,
-	left: targetRect.left - ( elementRect.width - targetRect.width ),
-	name: 'bottom'
+	left: targetRect.right - elementRect.width,
+	name: 'top-left'
 } );
 
+//	+--------+
+//	|   E    |
+//	+-----+--+
+//	|  T  |
+//	+-----+
+const attachTopRight = ( targetRect, elementRect ) => ( {
+	top: targetRect.top - elementRect.height,
+	left: targetRect.left,
+	name: 'top-right'
+} );
+
+const allPositions = [
+	attachLeftBottom,
+	attachLeftTop,
+	attachRightBottom,
+	attachRightTop,
+	attachBottomRight,
+	attachBottomLeft,
+	attachTopLeft,
+	attachTopRight
+];
+
 describe( 'getOptimalPosition()', () => {
 	testUtils.createSinonSandbox();
 
@@ -71,11 +126,11 @@ describe( 'getOptimalPosition()', () => {
 		assertPosition( {
 			element,
 			target: () => target,
-			positions: [ attachLeft ]
+			positions: [ attachLeftBottom ]
 		}, {
 			top: 100,
 			left: 80,
-			name: 'left'
+			name: 'left-bottom'
 		} );
 	} );
 
@@ -85,11 +140,11 @@ describe( 'getOptimalPosition()', () => {
 		assertPosition( {
 			element,
 			target: new Rect( target ),
-			positions: [ attachLeft ]
+			positions: [ attachLeftBottom ]
 		}, {
 			top: 100,
 			left: 80,
-			name: 'left'
+			name: 'left-bottom'
 		} );
 	} );
 
@@ -97,10 +152,10 @@ describe( 'getOptimalPosition()', () => {
 		beforeEach( setElementTargetPlayground );
 
 		it( 'should return coordinates', () => {
-			assertPosition( { element, target, positions: [ attachLeft ] }, {
+			assertPosition( { element, target, positions: [ attachLeftBottom ] }, {
 				top: 100,
 				left: 80,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
@@ -112,10 +167,10 @@ describe( 'getOptimalPosition()', () => {
 				scrollY: 100
 			} );
 
-			assertPosition( { element, target, positions: [ attachLeft ] }, {
+			assertPosition( { element, target, positions: [ attachLeftBottom ] }, {
 				top: 200,
 				left: 180,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
@@ -143,10 +198,10 @@ describe( 'getOptimalPosition()', () => {
 
 				element.parentElement = parent;
 
-				assertPosition( { element, target, positions: [ attachLeft ] }, {
+				assertPosition( { element, target, positions: [ attachLeftBottom ] }, {
 					top: -900,
 					left: -920,
-					name: 'left'
+					name: 'left-bottom'
 				} );
 			} );
 
@@ -175,10 +230,10 @@ describe( 'getOptimalPosition()', () => {
 
 				element.parentElement = parent;
 
-				assertPosition( { element, target, positions: [ attachLeft ] }, {
+				assertPosition( { element, target, positions: [ attachLeftBottom ] }, {
 					top: 160,
 					left: 260,
-					name: 'left'
+					name: 'left-bottom'
 				} );
 			} );
 		} );
@@ -190,22 +245,22 @@ describe( 'getOptimalPosition()', () => {
 		it( 'should return coordinates', () => {
 			assertPosition( {
 				element, target,
-				positions: [ attachLeft, attachRight ]
+				positions: [ attachLeftBottom, attachRightBottom ]
 			}, {
 				top: 100,
 				left: 80,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (position preference order)', () => {
 			assertPosition( {
 				element, target,
-				positions: [ attachRight, attachLeft ]
+				positions: [ attachRightBottom, attachLeftBottom ]
 			}, {
 				top: 100,
 				left: 110,
-				name: 'right'
+				name: 'right-bottom'
 			} );
 		} );
 	} );
@@ -217,11 +272,11 @@ describe( 'getOptimalPosition()', () => {
 			assertPosition( {
 				element, target,
 				limiter: () => limiter,
-				positions: [ attachLeft, attachRight ]
+				positions: [ attachLeftBottom, attachRightBottom ]
 			}, {
 				top: 100,
 				left: -20,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
@@ -229,33 +284,33 @@ describe( 'getOptimalPosition()', () => {
 			assertPosition( {
 				element, target,
 				limiter: new Rect( limiter ),
-				positions: [ attachLeft, attachRight ]
+				positions: [ attachLeftBottom, attachRightBottom ]
 			}, {
 				top: 100,
 				left: -20,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (#1)', () => {
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachLeft, attachRight ]
+				positions: [ attachLeftBottom, attachRightBottom ]
 			}, {
 				top: 100,
 				left: -20,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (#2)', () => {
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachRight, attachLeft ]
+				positions: [ attachRightBottom, attachLeftBottom ]
 			}, {
 				top: 100,
 				left: -20,
-				name: 'left'
+				name: 'left-bottom'
 			} );
 		} );
 
@@ -272,11 +327,30 @@ describe( 'getOptimalPosition()', () => {
 
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachRight, attachLeft ]
+				positions: [ attachRightBottom, attachLeftBottom ]
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
+			} );
+		} );
+
+		it( 'should return the first position that completely fits in the limiter', () => {
+			element = getElement( {
+				top: 0,
+				right: 5,
+				bottom: 5,
+				left: 0,
+				width: 5,
+				height: 5
+			} );
+			assertPosition( {
+				element, target, limiter,
+				positions: [ attachRightBottom, attachLeftBottom ]
+			}, {
+				top: 100,
+				left: -5,
+				name: 'left-bottom'
 			} );
 		} );
 	} );
@@ -287,36 +361,36 @@ describe( 'getOptimalPosition()', () => {
 		it( 'should return coordinates (#1)', () => {
 			assertPosition( {
 				element, target,
-				positions: [ attachLeft, attachRight ],
+				positions: [ attachLeftBottom, attachRightBottom ],
 				fitInViewport: true
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (#2)', () => {
 			assertPosition( {
 				element, target,
-				positions: [ attachRight, attachLeft ],
+				positions: [ attachRightBottom, attachLeftBottom ],
 				fitInViewport: true
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (#3)', () => {
 			assertPosition( {
 				element, target,
-				positions: [ attachLeft, attachBottom, attachRight ],
+				positions: [ attachLeftBottom, attachBottomRight, attachRightBottom ],
 				fitInViewport: true
 			}, {
 				top: 110,
 				left: 0,
-				name: 'bottom'
+				name: 'bottom-right'
 			} );
 		} );
 	} );
@@ -327,48 +401,48 @@ describe( 'getOptimalPosition()', () => {
 		it( 'should return coordinates (#1)', () => {
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachLeft, attachRight ],
+				positions: [ attachLeftBottom, attachRightBottom ],
 				fitInViewport: true
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (#2)', () => {
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachRight, attachLeft ],
+				positions: [ attachRightBottom, attachLeftBottom ],
 				fitInViewport: true
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
 			} );
 		} );
 
 		it( 'should return coordinates (#3)', () => {
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachRight, attachLeft, attachBottom ],
+				positions: [ attachRightBottom, attachLeftBottom, attachBottomRight ],
 				fitInViewport: true
 			}, {
 				top: 110,
 				left: 0,
-				name: 'bottom'
+				name: 'bottom-right'
 			} );
 		} );
 
 		it( 'should return coordinates (#4)', () => {
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachTop, attachRight ],
+				positions: [ attachTopLeft, attachRightBottom ],
 				fitInViewport: true
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
 			} );
 		} );
 
@@ -402,13 +476,140 @@ describe( 'getOptimalPosition()', () => {
 
 			assertPosition( {
 				element, target, limiter,
-				positions: [ attachRight, attachTop ],
+				positions: [ attachRightBottom, attachTopLeft ],
 				fitInViewport: true
 			}, {
 				top: 100,
 				left: 10,
-				name: 'right'
+				name: 'right-bottom'
+			} );
+		} );
+
+		it( 'should prefer positions fitting entirely into the viewport', () => {
+			target = getElement( {
+				top: 100,
+				right: 35,
+				bottom: 120,
+				left: 15,
+				width: 20,
+				height: 20
+			} );
+			assertPosition( {
+				element, target, limiter,
+				positions: [ attachLeftBottom, attachRightBottom ],
+				fitInViewport: true
+			}, {
+				top: 100,
+				left: 35,
+				name: 'right-bottom'
+			} );
+		} );
+	} );
+
+	describe( 'optimisation in the context of both the limiter and the viewport', () => {
+		beforeEach( setElementTargetBigLimiterPlayground );
+
+		it( 'should prefer a position with a bigger intersection area (#1)', () => {
+			target = getElement( {
+				top: 90,
+				right: -10,
+				bottom: 110,
+				left: -30,
+				width: 20,
+				height: 20
+			} );
+			assertPositionName( {
+				element, target, limiter,
+				positions: allPositions,
+				fitInViewport: true
+			}, 'right-bottom' );
+		} );
+
+		it( 'should prefer a position with a bigger intersection area (#2)', () => {
+			target = getElement( {
+				top: 290,
+				right: -10,
+				bottom: 310,
+				left: -30,
+				width: 20,
+				height: 20
+			} );
+			assertPositionName( {
+				element, target, limiter,
+				positions: allPositions,
+				fitInViewport: true
+			}, 'right-top' );
+		} );
+
+		it( 'should prefer a position with a bigger intersection area (#3)', () => {
+			target = getElement( {
+				top: 90,
+				right: 130,
+				bottom: 110,
+				left: 110,
+				width: 20,
+				height: 20
+			} );
+			assertPositionName( {
+				element, target, limiter,
+				positions: allPositions,
+				fitInViewport: true
+			}, 'left-bottom' );
+		} );
+
+		it( 'should prefer a position with a bigger intersection area (#4)', () => {
+			target = getElement( {
+				top: 290,
+				right: 130,
+				bottom: 310,
+				left: 110,
+				width: 20,
+				height: 20
+			} );
+			assertPositionName( {
+				element, target, limiter,
+				positions: allPositions,
+				fitInViewport: true
+			}, 'left-top' );
+		} );
+
+		it( 'should not stick to the first biggest intersection in one area', () => {
+			// First position intersects more with limiter but little with viewport,
+			// second position intersects less with limiter but more with viewport and it should not be ignored.
+			//
+			// Target is outside viewport to force checking all positions, not only those completely fitting in viewport.
+			limiter = getElement( {
+				top: -100,
+				right: 100,
+				bottom: 100,
+				left: -100,
+				width: 200,
+				height: 200
+			} );
+			target = getElement( {
+				top: -30,
+				right: 80,
+				bottom: -10,
+				left: 60,
+				width: 20,
+				height: 20
+			} );
+			element = getElement( {
+				top: 0,
+				right: 200,
+				bottom: 200,
+				left: 0,
+				width: 200,
+				height: 200
 			} );
+			assertPositionName( {
+				element, target, limiter,
+				positions: [
+					attachLeftBottom,
+					attachRightBottom
+				],
+				fitInViewport: true
+			}, 'right-bottom' );
 		} );
 	} );
 } );
@@ -419,6 +620,12 @@ function assertPosition( options, expected ) {
 	expect( position ).to.deep.equal( expected );
 }
 
+function assertPositionName( options, expected ) {
+	const position = getOptimalPosition( options );
+
+	expect( position.name ).to.equal( expected );
+}
+
 // Returns a synthetic element.
 //
 // @private
@@ -432,6 +639,9 @@ function getElement( properties = {}, styles = {} ) {
 		ownerDocument: document
 	};
 
+	expect( properties.right - properties.left ).to.equal( properties.width, 'getElement incorrect horizontal values' );
+	expect( properties.bottom - properties.top ).to.equal( properties.height, 'getElement incorrect vertical values' );
+
 	Object.assign( element, properties );
 
 	if ( !styles.borderLeftWidth ) {
@@ -540,3 +750,44 @@ function setElementTargetLimiterPlayground() {
 		height: 10
 	} );
 }
+
+//
+//
+//     ^                +-----------[ Viewport ]----------------------
+//     |                |
+//   100px              |
+//     |   <--------- 200px ------->
+//	   |               <-- 100px -->
+//	   V                |
+//	       +------------+---------+     ^
+//	       |            |         |     |
+//	       |            |         |     |
+//	       |            |         |     |
+//	       |            |         |    200px
+//	       |            |         |     |
+//	       |            |         |     |
+//	       |            |         |     |
+//         +------[ Limiter ]-----+     V
+//                      |
+//                      |
+//
+//
+function setElementTargetBigLimiterPlayground() {
+	element = getElement( {
+		top: 0,
+		right: 50,
+		bottom: 50,
+		left: 0,
+		width: 50,
+		height: 50
+	} );
+
+	limiter = getElement( {
+		top: 100,
+		right: 100,
+		bottom: 300,
+		left: -100,
+		width: 200,
+		height: 200
+	} );
+}