Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to getOptimalPosition to handle cases with scrollable ancestors #14755

Merged
merged 36 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
ee806e5
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
oleq Aug 3, 2023
34404b6
Revert "Reverted a fix for balloons sticking out of a limiter."
oleq Aug 3, 2023
66d6a6d
Preserve Rect source while getIntersection() to allow further use of …
oleq Aug 4, 2023
199ea15
Do not engage the BPV viewportStickyNorth position unless part of the…
oleq Aug 4, 2023
a5ccb11
Streamlined getOptimalPosition() logic. Now it is built on top of Rec…
oleq Aug 4, 2023
a7d3d01
Took advantage better getOptimalPosition() to handle "Powered by" log…
oleq Aug 4, 2023
8f5557d
Tests: Improved Powered by manual test with multiple scrollable ances…
oleq Aug 4, 2023
515b824
Tests: Added viewport offsets to the #5328 manual test in the UI for …
oleq Aug 4, 2023
6595fb4
Tests: Improved RectDrawer's styles. Exported a reusable background s…
oleq Aug 4, 2023
1442c63
Tests: Added additional cases to the Rect#getVisible() manual test.
oleq Aug 4, 2023
cf5e63a
Moved the "absolute rect" logic from getOptimalPosition to Rect. Addr…
oleq Aug 4, 2023
c14d3f1
Tests: Used lower z-index for RectDrawer because it go too obtrusive.
oleq Aug 4, 2023
2d19f70
Fine-tuned the viewportStickyNorth position.
oleq Aug 4, 2023
13d1a7d
Exported shared styles for RectDrawer.
oleq Aug 7, 2023
c9940eb
Tests: Updated imports in Rect#getVisible manual test.
oleq Aug 8, 2023
cb1f6e0
Tests.
oleq Aug 8, 2023
ab7d646
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
pszczesniak Aug 10, 2023
1647160
Fixing tests - position.
pszczesniak Aug 11, 2023
ae76f08
PoweredBy tests fixed.
pszczesniak Aug 14, 2023
19885ba
Fixing tests from dropdownview and blocktoolbar.
pszczesniak Aug 14, 2023
c13d551
Save the link in balloon - editor crash fix.
pszczesniak Aug 16, 2023
6ff2346
Manual test fixes for ticket 5328.
pszczesniak Aug 17, 2023
6005780
Fixing unit test cases and change main condition to cover all cases.
pszczesniak Aug 22, 2023
4a77bf9
Streamlining logic of the StickyPanelView.
pszczesniak Aug 24, 2023
2db00fe
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
pszczesniak Aug 24, 2023
b873a7b
Add missing API docs.
pszczesniak Aug 25, 2023
94eb704
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
pszczesniak Aug 25, 2023
5f6e51b
Add missing import.
pszczesniak Aug 28, 2023
759c50b
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
pszczesniak Sep 5, 2023
816c7c8
Fixes after code review.
pszczesniak Sep 5, 2023
a5b8836
Visualization of condition dependency between child and parent elements.
pszczesniak Sep 5, 2023
b93c30e
Fix function typings.
pszczesniak Sep 6, 2023
8d7786d
Remove logs.
pszczesniak Sep 6, 2023
b9cba98
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
pszczesniak Sep 6, 2023
21d1597
Merge branch 'master' into ck/5328-ballon-panel-sticks-out-v2
oleq Sep 7, 2023
d8672c3
Tests: Added a missing test for Rect instances preserving source info…
oleq Sep 7, 2023
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
8 changes: 6 additions & 2 deletions packages/ckeditor5-ui/src/dropdown/dropdownview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,16 @@ export default class DropdownView extends View<HTMLDivElement> {
// If "auto", find the best position of the panel to fit into the viewport.
// Otherwise, simply assign the static position.
if ( this.panelPosition === 'auto' ) {
this.panelView.position = DropdownView._getOptimalPosition( {
const optimalPanelPosition = DropdownView._getOptimalPosition( {
element: this.panelView.element!,
target: this.buttonView.element!,
fitInViewport: true,
positions: this._panelPositions
} ).name as PanelPosition;
} );

this.panelView.position = (
optimalPanelPosition ? optimalPanelPosition.name : this._panelPositions[ 0 ].name
) as PanelPosition;
} else {
this.panelView.position = this.panelPosition;
}
Expand Down
59 changes: 17 additions & 42 deletions packages/ckeditor5-ui/src/editorui/poweredby.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@

import type { Editor, UiConfig } from '@ckeditor/ckeditor5-core';
import {
Rect,
DomEmitterMixin,
findClosestScrollableAncestor,
Rect,
verifyLicense,
type PositionOptions,
type Locale
Expand All @@ -30,14 +29,6 @@ const ICON_HEIGHT = 10;
const NARROW_ROOT_HEIGHT_THRESHOLD = 50;
const NARROW_ROOT_WIDTH_THRESHOLD = 350;
const DEFAULT_LABEL = 'Powered by';
const OFF_THE_SCREEN_POSITION = {
top: -99999,
left: -99999,
name: 'invalid',
config: {
withArrow: false
}
};

type PoweredByConfig = Required<UiConfig>[ 'poweredBy' ];

Expand Down Expand Up @@ -312,18 +303,13 @@ function getLowerLeftCornerPosition( focusedEditableElement: HTMLElement, config
function getLowerCornerPosition(
focusedEditableElement: HTMLElement,
config: PoweredByConfig,
getBalloonLeft: ( editableElementRect: Rect, balloonRect: Rect ) => number
getBalloonLeft: ( visibleEditableElementRect: Rect, balloonRect: Rect ) => number
) {
return ( editableElementRect: Rect, balloonRect: Rect ) => {
const visibleEditableElementRect = editableElementRect.getVisible();

// Root cropped by ancestors.
if ( !visibleEditableElementRect ) {
return OFF_THE_SCREEN_POSITION;
}
return ( visibleEditableElementRect: Rect, balloonRect: Rect ) => {
const editableElementRect = new Rect( focusedEditableElement );

if ( editableElementRect.width < NARROW_ROOT_WIDTH_THRESHOLD || editableElementRect.height < NARROW_ROOT_HEIGHT_THRESHOLD ) {
return OFF_THE_SCREEN_POSITION;
return null;
}

let balloonTop;
Expand All @@ -339,31 +325,20 @@ function getLowerCornerPosition(

const balloonLeft = getBalloonLeft( editableElementRect, balloonRect );

if ( config.position === 'inside' ) {
const newBalloonRect = balloonRect.clone().moveTo( balloonLeft, balloonTop );
// Clone the editable element rect and place it where the balloon would be placed.
// This will allow getVisible() to work from editable element's perspective (rect source).
// and yield a result as if the balloon was on the same (scrollable) layer as the editable element.
const newBalloonPositionRect = visibleEditableElementRect
.clone()
.moveTo( balloonLeft, balloonTop )
.getIntersection( balloonRect.clone().moveTo( balloonLeft, balloonTop ) )!;

// The watermark cannot be positioned in this corner because the corner is not quite visible.
if ( newBalloonRect.getIntersectionArea( visibleEditableElementRect ) < newBalloonRect.getArea() ) {
return OFF_THE_SCREEN_POSITION;
}
}
else {
const firstScrollableEditableElementAncestor = findClosestScrollableAncestor( focusedEditableElement );

if ( firstScrollableEditableElementAncestor ) {
const firstScrollableEditableElementAncestorRect = new Rect( firstScrollableEditableElementAncestor );
const notVisibleVertically = visibleEditableElementRect.bottom + balloonRect.height / 2 >
firstScrollableEditableElementAncestorRect.bottom;
const notVisibleHorizontally = config.side === 'left' ?
editableElementRect.left < firstScrollableEditableElementAncestorRect.left :
editableElementRect.right > firstScrollableEditableElementAncestorRect.right;

// The watermark cannot be positioned in this corner because the corner is "not visible enough".
if ( notVisibleVertically || notVisibleHorizontally ) {
return OFF_THE_SCREEN_POSITION;
}
}
const newBalloonPositionVisibleRect = newBalloonPositionRect.getVisible();

if ( !newBalloonPositionVisibleRect || newBalloonPositionVisibleRect.getArea() < balloonRect.getArea() ) {
return null;
}

return {
top: balloonTop,
left: balloonLeft,
Expand Down
35 changes: 30 additions & 5 deletions packages/ckeditor5-ui/src/panel/balloon/balloonpanelview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,35 @@ import {
toUnit,
type Locale,
type ObservableChangeEvent,
type Position,
type PositionOptions,
type PositioningFunction,
type Rect
} from '@ckeditor/ckeditor5-utils';

import { isElement } from 'lodash-es';

import '../../../theme/components/panel/balloonpanel.css';

const toPx = toUnit( 'px' );
const defaultLimiterElement = global.document.body;

// A static balloon panel positioning function that moves the balloon far off the viewport.
// It is used as a fallback when there is no way to position the balloon using provided
// positioning functions (see: `getOptimalPosition()`), for instance, when the target the
// balloon should be attached to gets obscured by scrollable containers or the viewport.
//
// It prevents the balloon from being attached to the void and possible degradation of the UX.
// At the same time, it keeps the balloon physically visible in the DOM so the focus remains
// uninterrupted.
const POSITION_OFF_SCREEN: Position = {
top: -99999,
left: -99999,
name: 'arrowless',
config: {
withArrow: false
}
};

/**
* The balloon panel view class.
*
Expand Down Expand Up @@ -251,7 +268,7 @@ export default class BalloonPanelView extends View {
fitInViewport: true
}, options ) as PositionOptions;

const optimalPosition = BalloonPanelView._getOptimalPosition( positionOptions );
const optimalPosition = BalloonPanelView._getOptimalPosition( positionOptions ) || POSITION_OFF_SCREEN;

// Usually browsers make some problems with super accurate values like 104.345px
// so it is better to use int values.
Expand Down Expand Up @@ -1131,13 +1148,21 @@ export function generatePositions( options: {

// ------- Sticky

viewportStickyNorth: ( targetRect, balloonRect, viewportRect ) => {
if ( !targetRect.getIntersection( viewportRect! ) ) {
viewportStickyNorth: ( targetRect, balloonRect, viewportRect, limiterRect ) => {
const boundaryRect = limiterRect || viewportRect;

if ( !targetRect.getIntersection( boundaryRect ) ) {
return null;
}

// Engage when the target top and bottom edges are close or off the boundary.
// By close, it means there's not enough space for the balloon arrow (offset).
if ( boundaryRect.height - targetRect.height > stickyVerticalOffset ) {
return null;
}

return {
top: viewportRect!.top + stickyVerticalOffset,
top: boundaryRect.top + stickyVerticalOffset,
left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2,
name: 'arrowless',
config: {
Expand Down
109 changes: 56 additions & 53 deletions packages/ckeditor5-ui/src/panel/sticky/stickypanelview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ import type ViewCollection from '../../viewcollection';
import {
type Locale,
type ObservableChangeEvent,
getElementsIntersectionRect,
getScrollableAncestors,
global,
toUnit,
Rect
} from '@ckeditor/ckeditor5-utils';

// @if CK_DEBUG_STICKYPANEL // const RectDrawer = require( '@ckeditor/ckeditor5-utils/tests/_utils/rectdrawer' ).default
// @if CK_DEBUG_STICKYPANEL // const {
// @if CK_DEBUG_STICKYPANEL // default: RectDrawer,
// @if CK_DEBUG_STICKYPANEL // diagonalStylesBlack
// @if CK_DEBUG_STICKYPANEL // } = require( '@ckeditor/ckeditor5-utils/tests/_utils/rectdrawer' );

import '../../../theme/components/panel/stickypanel.css';

Expand Down Expand Up @@ -234,8 +235,8 @@ export default class StickyPanelView extends View {
this.checkIfShouldBeSticky();

// Update sticky state of the panel as the window and ancestors are being scrolled.
this.listenTo( global.document, 'scroll', ( evt, data ) => {
this.checkIfShouldBeSticky( data.target as HTMLElement | Document );
this.listenTo( global.document, 'scroll', () => {
this.checkIfShouldBeSticky();
}, { useCapture: true } );

// Synchronize with `model.isActive` because sticking an inactive panel is pointless.
Expand All @@ -247,10 +248,8 @@ export default class StickyPanelView extends View {
/**
* Analyzes the environment to decide whether the panel should be sticky or not.
* Then handles the positioning of the panel.
*
* @param [scrollTarget] The element which is being scrolled.
*/
public checkIfShouldBeSticky( scrollTarget?: HTMLElement | Document ): void {
public checkIfShouldBeSticky(): void {
// @if CK_DEBUG_STICKYPANEL // RectDrawer.clear();

if ( !this.limiterElement || !this.isActive ) {
Expand All @@ -259,17 +258,21 @@ export default class StickyPanelView extends View {
return;
}

const scrollableAncestors = getScrollableAncestors( this.limiterElement );
const limiterRect = new Rect( this.limiterElement );

if ( scrollTarget && !scrollableAncestors.includes( scrollTarget ) ) {
return;
}
let visibleLimiterRect = limiterRect.getVisible();

const visibleAncestorsRect = getElementsIntersectionRect( scrollableAncestors, this.viewportTopOffset );
const limiterRect = new Rect( this.limiterElement );
if ( visibleLimiterRect ) {
const windowRect = new Rect( global.window );

windowRect.top += this.viewportTopOffset;
windowRect.height -= this.viewportTopOffset;

// @if CK_DEBUG_STICKYPANEL // if ( visibleAncestorsRect ) {
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( visibleAncestorsRect,
visibleLimiterRect = visibleLimiterRect.getIntersection( windowRect );
}

// @if CK_DEBUG_STICKYPANEL // if ( visibleLimiterRect ) {
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( visibleLimiterRect,
// @if CK_DEBUG_STICKYPANEL // { outlineWidth: '3px', opacity: '.8', outlineColor: 'red', outlineOffset: '-3px' },
// @if CK_DEBUG_STICKYPANEL // 'Visible anc'
// @if CK_DEBUG_STICKYPANEL // );
Expand All @@ -283,48 +286,40 @@ export default class StickyPanelView extends View {
// Stick the panel only if
// * the limiter's ancestors are intersecting with each other so that some of their rects are visible,
// * and the limiter's top edge is above the visible ancestors' top edge.
if ( visibleAncestorsRect && limiterRect.top < visibleAncestorsRect.top ) {
const visibleLimiterRect = limiterRect.getIntersection( visibleAncestorsRect );

// Sticky the panel only if the limiter's visible rect is at least partially visible in the
// visible ancestors' rects intersection.
if ( visibleLimiterRect ) {
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( visibleLimiterRect,
// @if CK_DEBUG_STICKYPANEL // { outlineWidth: '3px', opacity: '.8', outlineColor: 'fuchsia', outlineOffset: '-3px',
// @if CK_DEBUG_STICKYPANEL // backgroundColor: 'rgba(255, 0, 255, .3)' },
// @if CK_DEBUG_STICKYPANEL // 'Visible limiter'
if ( visibleLimiterRect && limiterRect.top < visibleLimiterRect.top ) {
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( visibleLimiterRect,
// @if CK_DEBUG_STICKYPANEL // { outlineWidth: '3px', opacity: '.8', outlineColor: 'fuchsia', outlineOffset: '-3px',
// @if CK_DEBUG_STICKYPANEL // backgroundColor: 'rgba(255, 0, 255, .3)' },
// @if CK_DEBUG_STICKYPANEL // 'Visible limiter'
// @if CK_DEBUG_STICKYPANEL // );

const visibleLimiterTop = visibleLimiterRect.top;

// Check if there's a change the panel can be sticky to the bottom of the limiter.
if ( visibleLimiterTop + this._contentPanelRect.height + this.limiterBottomOffset > visibleLimiterRect.bottom ) {
const stickyBottomOffset = Math.max( limiterRect.bottom - visibleLimiterRect.bottom, 0 ) + this.limiterBottomOffset;
// @if CK_DEBUG_STICKYPANEL // const stickyBottomOffsetRect = new Rect( {
// @if CK_DEBUG_STICKYPANEL // top: limiterRect.bottom - stickyBottomOffset, left: 0, right: 2000,
// @if CK_DEBUG_STICKYPANEL // bottom: limiterRect.bottom - stickyBottomOffset, width: 2000, height: 1
// @if CK_DEBUG_STICKYPANEL // } );
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( stickyBottomOffsetRect,
// @if CK_DEBUG_STICKYPANEL // { outlineWidth: '1px', opacity: '.8', outlineColor: 'black' },
// @if CK_DEBUG_STICKYPANEL // 'Sticky bottom offset'
// @if CK_DEBUG_STICKYPANEL // );

const visibleAncestorsTop = visibleAncestorsRect.top;

// Check if there's a change the panel can be sticky to the bottom of the limiter.
if ( visibleAncestorsTop + this._contentPanelRect.height + this.limiterBottomOffset > visibleLimiterRect.bottom ) {
const stickyBottomOffset = Math.max( limiterRect.bottom - visibleAncestorsRect.bottom, 0 ) + this.limiterBottomOffset;
// @if CK_DEBUG_STICKYPANEL // const stickyBottomOffsetRect = new Rect( {
// @if CK_DEBUG_STICKYPANEL // top: limiterRect.bottom - stickyBottomOffset, left: 0, right: 2000,
// @if CK_DEBUG_STICKYPANEL // bottom: limiterRect.bottom - stickyBottomOffset, width: 2000, height: 1
// @if CK_DEBUG_STICKYPANEL // } );
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( stickyBottomOffsetRect,
// @if CK_DEBUG_STICKYPANEL // { outlineWidth: '1px', opacity: '.8', outlineColor: 'black' },
// @if CK_DEBUG_STICKYPANEL // 'Sticky bottom offset'
// @if CK_DEBUG_STICKYPANEL // );

// Check if sticking the panel to the bottom of the limiter does not cause it to suddenly
// move upwards if there's not enough space for it.
if ( limiterRect.bottom - stickyBottomOffset > limiterRect.top + this._contentPanelRect.height ) {
this._stickToBottomOfLimiter( stickyBottomOffset );
} else {
this._unstick();
}
// Check if sticking the panel to the bottom of the limiter does not cause it to suddenly
// move upwards if there's not enough space for it.
if ( limiterRect.bottom - stickyBottomOffset > limiterRect.top + this._contentPanelRect.height ) {
this._stickToBottomOfLimiter( stickyBottomOffset );
} else {
if ( this._contentPanelRect.height + this.limiterBottomOffset < limiterRect.height ) {
this._stickToTopOfAncestors( visibleAncestorsTop );
} else {
this._unstick();
}
this._unstick();
}
} else {
this._unstick();
if ( this._contentPanelRect.height + this.limiterBottomOffset < limiterRect.height ) {
this._stickToTopOfAncestors( visibleLimiterTop );
} else {
this._unstick();
}
}
} else {
this._unstick();
Expand All @@ -335,6 +330,14 @@ export default class StickyPanelView extends View {
// @if CK_DEBUG_STICKYPANEL // console.log( '_isStickyToTheBottomOfLimiter', this._isStickyToTheBottomOfLimiter );
// @if CK_DEBUG_STICKYPANEL // console.log( '_stickyTopOffset', this._stickyTopOffset );
// @if CK_DEBUG_STICKYPANEL // console.log( '_stickyBottomOffset', this._stickyBottomOffset );
// @if CK_DEBUG_STICKYPANEL // if ( visibleLimiterRect ) {
// @if CK_DEBUG_STICKYPANEL // RectDrawer.draw( visibleLimiterRect,
// @if CK_DEBUG_STICKYPANEL // { ...diagonalStylesBlack,
// @if CK_DEBUG_STICKYPANEL // outlineWidth: '3px', opacity: '.8', outlineColor: 'orange', outlineOffset: '-3px',
// @if CK_DEBUG_STICKYPANEL // backgroundColor: 'rgba(0, 0, 255, .2)' },
// @if CK_DEBUG_STICKYPANEL // 'visibleLimiterRect'
// @if CK_DEBUG_STICKYPANEL // );
// @if CK_DEBUG_STICKYPANEL // }
}

/**
Expand Down
37 changes: 14 additions & 23 deletions packages/ckeditor5-ui/src/toolbar/block/blocktoolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
import {
Rect,
ResizeObserver,
getOptimalPosition,
toUnit,
type ObservableChangeEvent
} from '@ckeditor/ckeditor5-utils';
Expand Down Expand Up @@ -430,29 +429,21 @@ export default class BlockToolbar extends Plugin {
// MDN says that 'normal' == ~1.2 on desktop browsers.
const contentLineHeight = parseInt( contentStyles.lineHeight, 10 ) || parseInt( contentStyles.fontSize, 10 ) * 1.2;

const position = getOptimalPosition( {
element: this.buttonView.element!,
target: targetElement,
positions: [
( contentRect, buttonRect ) => {
let left;

if ( this.editor.locale.uiLanguageDirection === 'ltr' ) {
left = editableRect.left - buttonRect.width;
} else {
left = editableRect.right;
}

return {
top: contentRect.top + contentPaddingTop + ( contentLineHeight - buttonRect.height ) / 2,
left
};
}
]
} );
const buttonRect = new Rect( this.buttonView.element! ).toAbsoluteRect();
const contentRect = new Rect( targetElement ).toAbsoluteRect();

let positionLeft;

if ( this.editor.locale.uiLanguageDirection === 'ltr' ) {
positionLeft = editableRect.left - buttonRect.width;
} else {
positionLeft = editableRect.right;
}

const positionTop = contentRect.top + contentPaddingTop + ( contentLineHeight - buttonRect.height ) / 2;

this.buttonView.top = position.top;
this.buttonView.left = position.left;
this.buttonView.top = positionTop;
this.buttonView.left = positionLeft;
}

/**
Expand Down
Loading