From b87c456651d6baa61d4c1547b4aa7893912899c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 21 Oct 2024 12:50:14 +0200 Subject: [PATCH 1/2] Disabled viewportOffset positioning restrictions for modal windows. --- .../ckeditor5-ui/src/dialog/dialogview.ts | 49 ++++++++++------- .../ckeditor5-ui/tests/dialog/dialogview.js | 55 ++++++++++++++++++- 2 files changed, 81 insertions(+), 23 deletions(-) diff --git a/packages/ckeditor5-ui/src/dialog/dialogview.ts b/packages/ckeditor5-ui/src/dialog/dialogview.ts index 81090f41514..62f4b040d9b 100644 --- a/packages/ckeditor5-ui/src/dialog/dialogview.ts +++ b/packages/ckeditor5-ui/src/dialog/dialogview.ts @@ -616,10 +616,35 @@ export default class DialogView extends /* #__PURE__ */ DraggableViewMixin( View } /** - * Calculates the viewport rect. + * Returns a viewport `Rect` shrunk by the viewport offset config from all sides. + * + * TODO: This is a duplicate from position.ts module. It should either be exported there or land somewhere in utils. */ - private _getViewportRect() { - return getConstrainedViewportRect( this._getViewportOffset() ); + private _getViewportRect(): Rect { + const viewportRect = new Rect( global.window ); + + // Modals should not be restricted by the viewport offsets as they are always displayed on top of the page. + if ( this.isModal ) { + return viewportRect; + } + + const viewportOffset = { + top: 0, + bottom: 0, + left: 0, + right: 0, + ...this._getViewportOffset() + }; + + viewportRect.top += viewportOffset.top!; + viewportRect.height -= viewportOffset.top!; + viewportRect.bottom -= viewportOffset.bottom!; + viewportRect.height -= viewportOffset.bottom!; + viewportRect.left += viewportOffset.left!; + viewportRect.right -= viewportOffset.right!; + viewportRect.width -= viewportOffset.left! + viewportOffset.right!; + + return viewportRect; } /** @@ -690,21 +715,3 @@ export type DialogViewCloseEvent = { * @eventName ~DialogView#moveTo */ export type DialogViewMoveToEvent = DecoratedMethodEvent; - -// Returns a viewport `Rect` shrunk by the viewport offset config from all sides. -// TODO: This is a duplicate from position.ts module. It should either be exported there or land somewhere in utils. -function getConstrainedViewportRect( viewportOffset: EditorUI[ 'viewportOffset' ] ): Rect { - viewportOffset = Object.assign( { top: 0, bottom: 0, left: 0, right: 0 }, viewportOffset ); - - const viewportRect = new Rect( global.window ); - - viewportRect.top += viewportOffset.top!; - viewportRect.height -= viewportOffset.top!; - viewportRect.bottom -= viewportOffset.bottom!; - viewportRect.height -= viewportOffset.bottom!; - viewportRect.left += viewportOffset.left!; - viewportRect.right -= viewportOffset.right!; - viewportRect.width -= viewportOffset.left! + viewportOffset.right!; - - return viewportRect; -} diff --git a/packages/ckeditor5-ui/tests/dialog/dialogview.js b/packages/ckeditor5-ui/tests/dialog/dialogview.js index 2ff384d9370..a2f51bf4c9b 100644 --- a/packages/ckeditor5-ui/tests/dialog/dialogview.js +++ b/packages/ckeditor5-ui/tests/dialog/dialogview.js @@ -806,7 +806,7 @@ describe( 'DialogView', () => { expect( view.element.firstChild.style.top ).to.equal( '5000px' ); } ); - it( 'should consider viewport offset configuration', () => { + it( 'should consider viewport offset configuration (dialog mode)', () => { getViewportOffsetStub.returns( { top: 10, right: 20, @@ -834,6 +834,37 @@ describe( 'DialogView', () => { expect( view.element.firstChild.style.left ).to.equal( '380px' ); expect( view.element.firstChild.style.top ).to.equal( '5000px' ); } ); + + it( 'should ignore viewport offset configuration (modal mode)', () => { + getViewportOffsetStub.returns( { + top: 10, + right: 20, + bottom: 30, + left: 40 + } ); + + view.isModal = true; + + view.moveTo( 50, 5 ); + + expect( view.element.firstChild.style.left ).to.equal( '50px' ); + expect( view.element.firstChild.style.top ).to.equal( '5px' ); + + view.moveTo( 0, 20 ); + + expect( view.element.firstChild.style.left ).to.equal( '0px' ); + expect( view.element.firstChild.style.top ).to.equal( '20px' ); + + view.moveTo( 1000, 20 ); + + expect( view.element.firstChild.style.left ).to.equal( '400px' ); + expect( view.element.firstChild.style.top ).to.equal( '20px' ); + + view.moveTo( 1000, 5000 ); + + expect( view.element.firstChild.style.left ).to.equal( '400px' ); + expect( view.element.firstChild.style.top ).to.equal( '5000px' ); + } ); } ); describe( 'moveBy()', () => { @@ -1128,7 +1159,7 @@ describe( 'DialogView', () => { } ); } ); - it( 'should consider viewport offsets', () => { + it( 'should consider viewport offsets (dialog mode)', () => { getViewportOffsetStub.returns( { top: 100, right: 0, @@ -1146,6 +1177,26 @@ describe( 'DialogView', () => { expect( view.element.firstChild.style.top ).to.equal( '115px' ); } ); + it( 'should ignore viewport offsets (modal mode)', () => { + view.isModal = true; + + getViewportOffsetStub.returns( { + top: 100, + right: 0, + bottom: 0, + left: 100 + } ); + + view.position = DialogViewPosition.EDITOR_TOP_SIDE; + + testUtils.sinon.stub( locale, 'contentLanguageDirection' ).get( () => 'rtl' ); + + view.updatePosition(); + + expect( view.element.firstChild.style.left ).to.equal( '25px' ); + expect( view.element.firstChild.style.top ).to.equal( '25px' ); + } ); + it( 'should not warn or throw if the view has not been rendered yet', () => { const warnStub = testUtils.sinon.stub( console, 'warn' ); From afeaf3d7b61ce8460644d100e54cc4ba86ad9071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 21 Oct 2024 12:51:37 +0200 Subject: [PATCH 2/2] Disabled dragging support for modal windows. --- packages/ckeditor5-ui/src/dialog/dialogview.ts | 4 +++- .../ckeditor5-ui/tests/dialog/dialogview.js | 18 ++++++++++++++++++ .../theme/components/dialog/dialog.css | 5 ++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-ui/src/dialog/dialogview.ts b/packages/ckeditor5-ui/src/dialog/dialogview.ts index 62f4b040d9b..56b35b80028 100644 --- a/packages/ckeditor5-ui/src/dialog/dialogview.ts +++ b/packages/ckeditor5-ui/src/dialog/dialogview.ts @@ -266,6 +266,7 @@ export default class DialogView extends /* #__PURE__ */ DraggableViewMixin( View class: [ 'ck', 'ck-dialog', + bind.if( 'isModal', 'ck-dialog_modal' ), bind.to( 'className' ) ], role: 'dialog', @@ -340,7 +341,8 @@ export default class DialogView extends /* #__PURE__ */ DraggableViewMixin( View * Returns the element that should be used as a drag handle. */ public override get dragHandleElement(): HTMLElement | null { - if ( this.headerView ) { + // Modals should not be draggable. + if ( this.headerView && !this.isModal ) { return this.headerView.element; } else { return null; diff --git a/packages/ckeditor5-ui/tests/dialog/dialogview.js b/packages/ckeditor5-ui/tests/dialog/dialogview.js index a2f51bf4c9b..b2e95e13958 100644 --- a/packages/ckeditor5-ui/tests/dialog/dialogview.js +++ b/packages/ckeditor5-ui/tests/dialog/dialogview.js @@ -131,6 +131,14 @@ describe( 'DialogView', () => { it( 'should host the dialog', () => { expect( overlayElement.firstChild.classList.contains( 'ck-dialog' ) ).to.be.true; } ); + + it( 'should set the CSS class on the dialog element in the modal mode', () => { + view.isModal = false; + expect( overlayElement.firstChild.classList.contains( 'ck-dialog_modal' ) ).to.be.false; + + view.isModal = true; + expect( overlayElement.firstChild.classList.contains( 'ck-dialog_modal' ) ).to.be.true; + } ); } ); describe( 'dialog', () => { @@ -565,6 +573,16 @@ describe( 'DialogView', () => { expect( view.dragHandleElement ).to.be.null; } ); + it( 'should not provide #dragHandleElement when in a modal mode because modals should not be draggable', () => { + view.setupParts( { + title: 'foo' + } ); + + view.isModal = true; + + expect( view.dragHandleElement ).to.be.null; + } ); + it( 'should be possible by dragging the #headerView', () => { view.setupParts( { title: 'foo' diff --git a/packages/ckeditor5-ui/theme/components/dialog/dialog.css b/packages/ckeditor5-ui/theme/components/dialog/dialog.css index 657fea5ca8b..d2832df40e6 100644 --- a/packages/ckeditor5-ui/theme/components/dialog/dialog.css +++ b/packages/ckeditor5-ui/theme/components/dialog/dialog.css @@ -27,8 +27,11 @@ & .ck.ck-form__header { flex-shrink: 0; + } - & .ck-form__header__label { + /* Modals should not be draggable. */ + &:not(.ck-dialog_modal) { + & .ck.ck-form__header .ck-form__header__label { cursor: grab; } }