Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #329 from ckeditor/i/6181
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
oleq authored Apr 1, 2020
2 parents 6dc1ba6 + 9db9ccb commit 7cd1238
Show file tree
Hide file tree
Showing 2 changed files with 480 additions and 133 deletions.
254 changes: 175 additions & 79 deletions src/dom/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 ) ];
Expand All @@ -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 ) {
Expand All @@ -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 );
Expand All @@ -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.
//
Expand Down
Loading

0 comments on commit 7cd1238

Please sign in to comment.