diff --git a/src/balloonpanel/balloonpanelview.js b/src/balloonpanel/balloonpanelview.js index 65804f0..c3567f2 100644 --- a/src/balloonpanel/balloonpanelview.js +++ b/src/balloonpanel/balloonpanelview.js @@ -7,7 +7,7 @@ * @module ui/balloonpanel/balloonpanelview */ -/* globals window, document, Range, HTMLElement */ +/* globals window, Range, HTMLElement */ import View from '../view.js'; import Template from '../template.js'; @@ -228,6 +228,8 @@ export default class BalloonPanelView extends View { */ _smartAttachTo( rects, visibleContainerRect, panelSurfaceArea ) { const viewportRect = new AbsoluteDomRect( getAbsoluteViewportRect() ); + const positionedAncestor = getPositionedAncestor( this.element.parentElement ); + let maxIntersectRectPos; let maxContainerIntersectArea = -1; let maxViewportIntersectArea = -1; @@ -248,17 +250,25 @@ export default class BalloonPanelView extends View { // Move the balloon panel. this.arrow = maxIntersectRectPos; - this.top = rects[ maxIntersectRectPos ].top; - this.left = rects[ maxIntersectRectPos ].left; + + let { top, left } = rects[ maxIntersectRectPos ]; + + // (#126) If there's some positioned ancestor of the panel, then its positioned rect must be taken into + // consideration because `AbsoluteDomRect` is always relative to the viewport. + if ( positionedAncestor ) { + const positionedAncestorRect = positionedAncestor.getBoundingClientRect(); + + top -= positionedAncestorRect.top; + left -= positionedAncestorRect.left; + } + + this.top = top; + this.left = left; } } /** - * An abstract class which represents a client rect of an HTMLElement or a Range in DOM. - * - * Note: The geometry used by each instance corresponds with coordinates of an object - * with `position: absolute` relative to the `` (`document.body`), and hence - * it is useful to manage such objects. + * A class which represents a client rect of an HTMLElement or a Range in DOM. * * @private */ @@ -310,33 +320,42 @@ class AbsoluteDomRect { // @param {HTMLElement|Range|Object} elementOrRangeOrRect Target object witch rect is to be determined. // @returns {Object} Client rect object. function getAbsoluteRect( elementOrRangeOrRect ) { - const bodyRect = document.body.getBoundingClientRect(); - if ( elementOrRangeOrRect instanceof HTMLElement || elementOrRangeOrRect instanceof Range ) { - const elementRect = elementOrRangeOrRect.getBoundingClientRect(); - - return { - top: elementRect.top - bodyRect.top, - right: elementRect.right - bodyRect.left, - bottom: elementRect.bottom - bodyRect.top, - left: elementRect.left - bodyRect.left, - width: elementRect.width, - height: elementRect.height - }; + let { top, right, bottom, left, width, height } = elementOrRangeOrRect.getBoundingClientRect(); + + return { top, right, bottom, left, width, height }; } + // A rect has been passed. + else { + const absoluteRect = Object.assign( {}, elementOrRangeOrRect ); + + if ( absoluteRect.width === undefined ) { + absoluteRect.width = absoluteRect.right - absoluteRect.left; + } - // The rect has been passed. - const absoluteRect = Object.assign( {}, elementOrRangeOrRect ); + if ( absoluteRect.height === undefined ) { + absoluteRect.height = absoluteRect.bottom - absoluteRect.top; + } - if ( absoluteRect.width === undefined ) { - absoluteRect.width = absoluteRect.right - absoluteRect.left; + return absoluteRect; } +} - if ( absoluteRect.height === undefined ) { - absoluteRect.height = absoluteRect.bottom - absoluteRect.top; +// For a given element, returns the nearest ancestor element which position is not "static". +// +// @private +// @param {HTMLElement} element Element which ancestors are checked. +// @returns {HTMLElement|null} +function getPositionedAncestor( element ) { + while ( element && element.tagName.toLowerCase() != 'html' ) { + if ( window.getComputedStyle( element ).position != 'static' ) { + return element; + } + + element = element.parentNode; } - return absoluteRect; + return null; } // Returns the client rect of the element limited by the visible (to the user) diff --git a/tests/balloonpanel/balloonpanelview.js b/tests/balloonpanel/balloonpanelview.js index d52afcc..ea3e0e8 100644 --- a/tests/balloonpanel/balloonpanelview.js +++ b/tests/balloonpanel/balloonpanelview.js @@ -112,7 +112,7 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'show', () => { + describe( 'show()', () => { it( 'should set view#isVisible as true', () => { view.isVisible = false; @@ -122,7 +122,7 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'hide', () => { + describe( 'hide()', () => { it( 'should set view#isVisible as false', () => { view.isVisible = true; @@ -132,7 +132,7 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'attachTo', () => { + describe( 'attachTo()', () => { let targetEl, limiterEl; beforeEach( () => { @@ -229,6 +229,54 @@ describe( 'BalloonPanelView', () => { expect( view.arrow ).to.equal( 'nw' ); } ); + + // #126 + it( 'works in a positioned ancestor (position: absolute)', () => { + const positionedAncestor = document.createElement( 'div' ); + + positionedAncestor.style.position = 'absolute'; + positionedAncestor.style.top = '100px'; + positionedAncestor.style.left = '100px'; + positionedAncestor.appendChild( view.element ); + + document.body.appendChild( positionedAncestor ); + positionedAncestor.appendChild( view.element ); + + mockBoundingBox( targetEl, { + top: 0, + left: 0, + width: 100, + height: 100 + } ); + + view.attachTo( targetEl, limiterEl ); + + expect( view.top ).to.equal( 15 ); + expect( view.left ).to.equal( -80 ); + } ); + + // #126 + it( 'works in a positioned ancestor (position: static)', () => { + const positionedAncestor = document.createElement( 'div' ); + + positionedAncestor.style.position = 'static'; + positionedAncestor.appendChild( view.element ); + + document.body.appendChild( positionedAncestor ); + positionedAncestor.appendChild( view.element ); + + mockBoundingBox( targetEl, { + top: 0, + left: 0, + width: 100, + height: 100 + } ); + + view.attachTo( targetEl, limiterEl ); + + expect( view.top ).to.equal( 115 ); + expect( view.left ).to.equal( 20 ); + } ); } ); describe( 'limited by viewport', () => { diff --git a/tests/manual/tickets/126/1.html b/tests/manual/tickets/126/1.html new file mode 100644 index 0000000..a2eebee --- /dev/null +++ b/tests/manual/tickets/126/1.html @@ -0,0 +1,52 @@ + + + + +
+
+
+
+ +
+
+
+
+ + diff --git a/tests/manual/tickets/126/1.js b/tests/manual/tickets/126/1.js new file mode 100644 index 0000000..1ee1286 --- /dev/null +++ b/tests/manual/tickets/126/1.js @@ -0,0 +1,25 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document */ + +import BalloonPanelView from 'ckeditor5/ui/balloonpanel/balloonpanelview.js'; + +createPanel( 'static' ); +createPanel( 'relative' ); +createPanel( 'absolute' ); +createPanel( 'fixed' ); + +function createPanel( selector ) { + const view = new BalloonPanelView(); + + view.element.innerHTML = `Parent of this panel has position:${ selector }.`; + view.init().then( () => { + document.querySelector( `#${ selector }-container` ).appendChild( view.element ); + + view.attachTo( document.querySelector( `#anchor-${ selector }` ), document.body ); + view.isVisible = true; + } ); +} diff --git a/tests/manual/tickets/126/1.md b/tests/manual/tickets/126/1.md new file mode 100644 index 0000000..42b251e --- /dev/null +++ b/tests/manual/tickets/126/1.md @@ -0,0 +1,6 @@ +### Invalid `BalloonPanelView` positioning [#126](https://github.com/ckeditor/ckeditor5-ui-default/issues/126) + +1. There should be 4 red squares displayed in this test. +1. There should be 4 balloon panels displayed in this test. +1. Each balloon panel should be **perfectly** attached to a square. +1. Each balloon panel's triangle should hit directly into the spot (square).