Skip to content

Commit

Permalink
Merge pull request #17374 from ckeditor/ck/17220
Browse files Browse the repository at this point in the history
Fix (ui): The menu or dropdown panels will no longer be placed in an incorrect position when a optimal position can't be found. Closes #17220
  • Loading branch information
Mati365 authored Nov 6, 2024
2 parents ec2f7e8 + 6cf9635 commit 00e9a7f
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 5 deletions.
11 changes: 10 additions & 1 deletion packages/ckeditor5-ui/src/dropdown/dropdownview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ export default class DropdownView extends View<HTMLDivElement> {
} );

this.panelView.position = (
optimalPanelPosition ? optimalPanelPosition.name : this._panelPositions[ 0 ].name
optimalPanelPosition ? optimalPanelPosition.name : this._defaultPanelPositionName
) as PanelPosition;
} else {
this.panelView.position = this.panelPosition;
Expand Down Expand Up @@ -358,6 +358,15 @@ export default class DropdownView extends View<HTMLDivElement> {
}
}

/**
* Returns the default position of the dropdown panel based on the direction of the UI language.
* It is used when the {@link #panelPosition} is set to `'auto'` and the panel has not found a
* suitable position to fit into the viewport.
*/
private get _defaultPanelPositionName(): PanelPosition {
return this.locale!.uiLanguageDirection === 'rtl' ? 'sw' : 'se';
}

/**
* A set of positioning functions used by the dropdown view to determine
* the optimal position (i.e. fitting into the browser viewport) of its
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-ui/src/menubar/menubarmenupanelview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,4 @@ export default class MenuBarMenuPanelView extends View implements FocusableView
*
* They are reflected as CSS class suffixes on the panel view element.
*/
export type MenuBarMenuPanelPosition = 'se' | 'sw' | 'ne' | 'nw' | 'w' | 'e';
export type MenuBarMenuPanelPosition = 'se' | 'sw' | 'ne' | 'nw' | 'w' | 'ws' | 'e' | 'es';
22 changes: 21 additions & 1 deletion packages/ckeditor5-ui/src/menubar/menubarmenuview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export default class MenuBarMenuView extends View implements FocusableView {
} );

this.panelView.position = (
optimalPanelPosition ? optimalPanelPosition.name : this._panelPositions[ 0 ].name
optimalPanelPosition ? optimalPanelPosition.name : this._defaultMenuPositionName
) as MenuBarMenuPanelPosition;
} );
}
Expand Down Expand Up @@ -249,6 +249,26 @@ export default class MenuBarMenuView extends View implements FocusableView {
}
}

/**
* The default position of the panel when the menu is opened.
* It is used when the optimal position cannot be calculated.
*/
private get _defaultMenuPositionName(): MenuBarMenuPanelPosition {
if ( this.locale!.uiLanguageDirection === 'ltr' ) {
if ( this.parentMenuView ) {
return 'es';
} else {
return 'se';
}
} else {
if ( this.parentMenuView ) {
return 'ws';
} else {
return 'sw';
}
}
}

/**
* A function used to calculate the optimal position for the dropdown panel.
*
Expand Down
33 changes: 32 additions & 1 deletion packages/ckeditor5-ui/tests/dropdown/dropdownview.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,38 @@ describe( 'DropdownView', () => {

view.isOpen = true;

expect( view.panelView.position ).is.equal( 'southEast' ); // first position from position list.
expect( view.panelView.position ).is.equal( 'se' ); // first position from position list.

view.element.remove();
parentWithOverflow.remove();
} );

it( 'fallback when _getOptimalPosition() will return null (RTL)', () => {
const locale = {
t() {}
};

const buttonView = new ButtonView( locale );
const panelView = new DropdownPanelView( locale );

const view = new DropdownView( locale, buttonView, panelView );

view.locale.uiLanguageDirection = 'rtl';
view.render();

const parentWithOverflow = global.document.createElement( 'div' );
parentWithOverflow.style.width = '1px';
parentWithOverflow.style.height = '1px';
parentWithOverflow.style.marginTop = '-1000px';
parentWithOverflow.style.overflow = 'scroll';

parentWithOverflow.appendChild( view.element );

global.document.body.appendChild( parentWithOverflow );

view.isOpen = true;

expect( view.panelView.position ).is.equal( 'sw' ); // first position from position list.

view.element.remove();
parentWithOverflow.remove();
Expand Down
46 changes: 45 additions & 1 deletion packages/ckeditor5-ui/tests/menubar/menubarmenuview.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,51 @@ describe( 'MenuBarMenuView', () => {

menuView.isOpen = true;

expect( menuView.panelView.position ).to.equal( menuView._panelPositions[ 0 ].name );
expect( menuView.panelView.position ).to.equal( 'se' );
} );

it( 'should use the default position if none were considered optimal (has parent menu)', () => {
createTopLevelMenuWithLocale( locale );

sinon.stub( MenuBarMenuView, '_getOptimalPosition' ).returns( null );

menuView.parentMenuView = new MenuBarMenuView( locale );

menuView.panelView.position = null;

menuView.isOpen = true;

expect( menuView.panelView.position ).to.equal( 'es' );
} );

it( 'should use the default position if none were considered optimal (RTL)', () => {
createTopLevelMenuWithLocale( locale );

sinon.stub( MenuBarMenuView, '_getOptimalPosition' ).returns( null );

menuView.locale.uiLanguageDirection = 'rtl';

menuView.panelView.position = null;

menuView.isOpen = true;

expect( menuView.panelView.position ).to.equal( 'sw' );
} );

it( 'should use the default position if none were considered optimal (RTL, has parent menu)', () => {
createTopLevelMenuWithLocale( locale );

sinon.stub( MenuBarMenuView, '_getOptimalPosition' ).returns( null );

menuView.locale.uiLanguageDirection = 'rtl';

menuView.parentMenuView = new MenuBarMenuView( locale );

menuView.panelView.position = null;

menuView.isOpen = true;

expect( menuView.panelView.position ).to.equal( 'ws' );
} );

afterEach( () => {
Expand Down

0 comments on commit 00e9a7f

Please sign in to comment.