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

t/ckeditor5-ui/515: The getOptimalPosition() utility should consider element ancestors with overflow to avoid cropping #293

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/dom/getancestorwithoverflow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/**
* @module utils/dom/getancestorwithoverflow
*/

import global from './global';

/**
* For a given element, returns the nearest ancestor element which CSS `overflow` is not `visible`.
*
* @param {HTMLElement} element The native DOM element to be checked.
* @returns {HTMLElement|null}
*/
export default function getAncestorWithOveflow( element ) {
while ( element && element.tagName.toLowerCase() != 'html' ) {
if ( global.window.getComputedStyle( element ).overflow != 'visible' ) {
return element;
}

element = element.parentElement;
}

return null;
}

29 changes: 28 additions & 1 deletion src/dom/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import global from './global';
import Rect from './rect';
import getPositionedAncestor from './getpositionedancestor';
import getAncestorWithOveflow from './getancestorwithoverflow';
import getBorderWidths from './getborderwidths';
import { isFunction } from 'lodash-es';

Expand Down Expand Up @@ -101,7 +102,33 @@ export function getOptimalPosition( { element, target, positions, limiter, fitIn
if ( !limiter && !fitInViewport ) {
[ name, bestPosition ] = getPosition( positions[ 0 ], targetRect, elementRect );
} else {
const limiterRect = limiter && new Rect( limiter ).getVisible();
let limiterRect;

if ( limiter ) {
limiterRect = new Rect( limiter ).getVisible();
}

const ancestorWithOveflow = getAncestorWithOveflow( element.parentElement );

// Limiter or not, there could be some ancestor down the DOM tree that has `overflow` other
// than `visible`. The algorithm must consider that to make sure the positioned element
// does not get cropped by that ancestor ✂️.
// https://github.com/ckeditor/ckeditor5-ui/issues/515
if ( ancestorWithOveflow ) {
const ancestorWithOveflowRect = new Rect( ancestorWithOveflow );

// Fitting into the limiter does not constitute a good position.
// Use that portion of the limiter Rect that is cropped by the ancestor with `overflow`.
if ( limiterRect ) {
limiterRect = limiterRect.getIntersection( ancestorWithOveflowRect );
}
// There's no configured limiter element but since there is an ancestor with `overflow`,
// well... it becomes the limiter.
else {
limiterRect = ancestorWithOveflowRect;
}
}

const viewportRect = fitInViewport && new Rect( global.window );

[ name, bestPosition ] =
Expand Down