Skip to content

Commit

Permalink
Merge pull request #17291 from ckeditor/ck/17290
Browse files Browse the repository at this point in the history
Other (ui): Disabled dragging support for modal windows in the `Dialog` plugin. Closes #17290.

Other (ui): Disabled positioning restrictions for modal windows caused by `config.ui.viewportOffset`. Closes #17290.
  • Loading branch information
oleq authored Oct 22, 2024
2 parents 926400f + afeaf3d commit 6c16073
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 25 deletions.
53 changes: 31 additions & 22 deletions packages/ckeditor5-ui/src/dialog/dialogview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -616,10 +618,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;
}

/**
Expand Down Expand Up @@ -690,21 +717,3 @@ export type DialogViewCloseEvent = {
* @eventName ~DialogView#moveTo
*/
export type DialogViewMoveToEvent = DecoratedMethodEvent<DialogView, 'moveTo'>;

// 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;
}
73 changes: 71 additions & 2 deletions packages/ckeditor5-ui/tests/dialog/dialogview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -806,7 +824,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,
Expand Down Expand Up @@ -834,6 +852,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()', () => {
Expand Down Expand Up @@ -1128,7 +1177,7 @@ describe( 'DialogView', () => {
} );
} );

it( 'should consider viewport offsets', () => {
it( 'should consider viewport offsets (dialog mode)', () => {
getViewportOffsetStub.returns( {
top: 100,
right: 0,
Expand All @@ -1146,6 +1195,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' );

Expand Down
5 changes: 4 additions & 1 deletion packages/ckeditor5-ui/theme/components/dialog/dialog.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down

0 comments on commit 6c16073

Please sign in to comment.