From 6cf9635d645d49038e46cab64cc05934bbb4a43d Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 4 Nov 2024 08:20:43 +0100 Subject: [PATCH] Fix wrong panel position fallback calculation in menu and dropdown views. --- .../ckeditor5-ui/src/dropdown/dropdownview.ts | 11 ++++- .../src/menubar/menubarmenupanelview.ts | 2 +- .../src/menubar/menubarmenuview.ts | 22 ++++++++- .../tests/dropdown/dropdownview.js | 33 ++++++++++++- .../tests/menubar/menubarmenuview.js | 46 ++++++++++++++++++- 5 files changed, 109 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-ui/src/dropdown/dropdownview.ts b/packages/ckeditor5-ui/src/dropdown/dropdownview.ts index a33b00e463d..13854fe9451 100644 --- a/packages/ckeditor5-ui/src/dropdown/dropdownview.ts +++ b/packages/ckeditor5-ui/src/dropdown/dropdownview.ts @@ -286,7 +286,7 @@ export default class DropdownView extends View { } ); this.panelView.position = ( - optimalPanelPosition ? optimalPanelPosition.name : this._panelPositions[ 0 ].name + optimalPanelPosition ? optimalPanelPosition.name : this._defaultPanelPositionName ) as PanelPosition; } else { this.panelView.position = this.panelPosition; @@ -358,6 +358,15 @@ export default class DropdownView extends View { } } + /** + * 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 diff --git a/packages/ckeditor5-ui/src/menubar/menubarmenupanelview.ts b/packages/ckeditor5-ui/src/menubar/menubarmenupanelview.ts index d9e79dd4160..d69189b0b6e 100644 --- a/packages/ckeditor5-ui/src/menubar/menubarmenupanelview.ts +++ b/packages/ckeditor5-ui/src/menubar/menubarmenupanelview.ts @@ -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'; diff --git a/packages/ckeditor5-ui/src/menubar/menubarmenuview.ts b/packages/ckeditor5-ui/src/menubar/menubarmenuview.ts index 6cc8331b9f1..1a28ad8c5df 100644 --- a/packages/ckeditor5-ui/src/menubar/menubarmenuview.ts +++ b/packages/ckeditor5-ui/src/menubar/menubarmenuview.ts @@ -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; } ); } @@ -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. * diff --git a/packages/ckeditor5-ui/tests/dropdown/dropdownview.js b/packages/ckeditor5-ui/tests/dropdown/dropdownview.js index cd869629bbc..2fab2b6ddc1 100644 --- a/packages/ckeditor5-ui/tests/dropdown/dropdownview.js +++ b/packages/ckeditor5-ui/tests/dropdown/dropdownview.js @@ -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(); diff --git a/packages/ckeditor5-ui/tests/menubar/menubarmenuview.js b/packages/ckeditor5-ui/tests/menubar/menubarmenuview.js index b7c6a75bfc6..9c498e0de26 100644 --- a/packages/ckeditor5-ui/tests/menubar/menubarmenuview.js +++ b/packages/ckeditor5-ui/tests/menubar/menubarmenuview.js @@ -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( () => {