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

The dialog plugin should not handle Esc key press when default-prevented by the guest view #17344

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/dialog/dialogview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,12 @@ export default class DialogView extends /* #__PURE__ */ DraggableViewMixin( View
super.render();

this.keystrokes.set( 'Esc', ( data, cancel ) => {
this.fire<DialogViewCloseEvent>( 'close', { source: 'escKeyPress' } );
cancel();
// Do not react to the Esc key if the event has already been handled and defaultPrevented
// by some logic of the dialog guest (child) view (https://github.com/ckeditor/ckeditor5/issues/17343).
if ( !data.defaultPrevented ) {
this.fire<DialogViewCloseEvent>( 'close', { source: 'escKeyPress' } );
cancel();
}
} );

// Support for dragging the modal.
Expand Down
70 changes: 63 additions & 7 deletions packages/ckeditor5-ui/tests/dialog/dialogview.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,15 +411,71 @@ describe( 'DialogView', () => {
view.render();
} );

it( 'should emit the event with data upon pressing Esc', () => {
const spy = sinon.spy();
describe( 'Esc key press handling', () => {
it( 'should emit the "close" event with data upon pressing Esc', () => {
const spy = sinon.spy();

view.on( 'close', spy );
view.on( 'close', spy );

view.element.dispatchEvent( new KeyboardEvent( 'keydown', { keyCode: keyCodes.a } ) );
sinon.assert.notCalled( spy );
view.element.dispatchEvent( new KeyboardEvent( 'keydown', { keyCode: keyCodes.esc } ) );
sinon.assert.calledWithExactly( spy, sinon.match.any, { source: 'escKeyPress' } );
const unrelatedDomEvent = new KeyboardEvent( 'keydown', {
keyCode: keyCodes.a,
bubbles: true,
cancelable: true
} );

sinon.stub( unrelatedDomEvent, 'stopPropagation' );
sinon.stub( unrelatedDomEvent, 'preventDefault' );

view.element.dispatchEvent( unrelatedDomEvent );

sinon.assert.notCalled( spy );
sinon.assert.notCalled( unrelatedDomEvent.stopPropagation );
sinon.assert.notCalled( unrelatedDomEvent.preventDefault );

const escDomEvent = new KeyboardEvent( 'keydown', {
keyCode: keyCodes.esc,
bubbles: true,
cancelable: true
} );

sinon.stub( escDomEvent, 'stopPropagation' );
sinon.stub( escDomEvent, 'preventDefault' );

view.element.dispatchEvent( escDomEvent );

sinon.assert.calledWithExactly( spy, sinon.match.any, {
source: 'escKeyPress'
} );

sinon.assert.calledOnce( escDomEvent.stopPropagation );
sinon.assert.calledOnce( escDomEvent.preventDefault );
} );

it( 'should not emit the "close" event if the original DOM event was preventDefaulted by some other logic', () => {
const spy = sinon.spy();
const childView = createContentView( 'A' );

view.setupParts( { content: childView } );
view.on( 'close', spy );

// Some child view's logic handling the key press.
childView.element.addEventListener( 'keydown', evt => {
evt.preventDefault();
} );

const escDomEvent = new KeyboardEvent( 'keydown', {
keyCode: keyCodes.esc,
bubbles: true,
cancelable: true
} );

sinon.stub( escDomEvent, 'stopPropagation' );

childView.element.dispatchEvent( escDomEvent );

sinon.assert.notCalled( spy );
sinon.assert.notCalled( escDomEvent.stopPropagation );
} );
} );

it( 'should move the dialog upon the #drag event', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<div id="editor">
<h2>Heading 1</h2>
<p>Paragraph</p>
<p><strong>Bold</strong> <i>Italic</i> <a href="https://ckeditor.com">Link</a></p>
<ul>
<li>UL List item 1</li>
<li>UL List item 2</li>
</ul>
<ol>
<li>OL List item 1</li>
<li>OL List item 2</li>
</ol>
<figure class="image image-style-side">
<img style="aspect-ratio:800/376;" src="sample.jpg" alt="bar" width="800" height="376">
</figure>
<blockquote>
<p>Quote</p>
<ul>
<li>Quoted UL List item 1</li>
<li>Quoted UL List item 2</li>
</ul>
<p>Quote</p>
</blockquote>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Esc key handling in dialogs

1. This manual test showcases how the dialog system interacts with the Esc key press.
2. Open the dialog by clicking the "Open dialog" button.
3. Keep pressing Esc key according to the instructions.
4. The dialog should not close until the last (10th) Esc key press. Only then the child view will stop `preventDefaulting` on the keydown event.
166 changes: 166 additions & 0 deletions packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/**
* @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals console, document */

declare global {
interface Window { CKEditorInspector: any }
}

import { Essentials } from '@ckeditor/ckeditor5-essentials';
import { Autoformat } from '@ckeditor/ckeditor5-autoformat';
import { BlockQuote } from '@ckeditor/ckeditor5-block-quote';
import { Bold, Italic } from '@ckeditor/ckeditor5-basic-styles';
import { Heading } from '@ckeditor/ckeditor5-heading';
import { Image, ImageCaption, ImageStyle, ImageToolbar } from '@ckeditor/ckeditor5-image';
import { Indent } from '@ckeditor/ckeditor5-indent';
import { Link } from '@ckeditor/ckeditor5-link';
import { List } from '@ckeditor/ckeditor5-list';
import { MediaEmbed } from '@ckeditor/ckeditor5-media-embed';
import { Paragraph } from '@ckeditor/ckeditor5-paragraph';
import { Table, TableToolbar } from '@ckeditor/ckeditor5-table';
import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic';
import FindAndReplace from '@ckeditor/ckeditor5-find-and-replace/src/findandreplace.js';
import SpecialCharacters from '@ckeditor/ckeditor5-special-characters/src/specialcharacters.js';
import SpecialCharactersEssentials from '@ckeditor/ckeditor5-special-characters/src/specialcharactersessentials.js';
import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting.js';
import { ButtonView, Dialog, View } from '../../../src/index.js';
import { Plugin } from '@ckeditor/ckeditor5-core';

class ViewWithEscSupport extends View {
declare public count: number;

constructor() {
super();

const bind = this.bindTemplate;

this.set( 'count', 0 );

this.setTemplate( {
tag: 'div',
attributes: {
tabindex: -1,
style: {
padding: '20px'
}
},
children: [
{
tag: 'p',
children: [
'Focus me and press Esc key 10 times. Count: ',
{ text: bind.to( 'count' ) }
]
}
],
on: {
keydown: bind.to( ( evt: Event ) => {
if ( ( evt as KeyboardEvent ).key == 'Escape' ) {
if ( this.count++ < 9 ) {
evt.preventDefault();
}
}
} )
}
} );
}

public focus() {
this.element!.focus();
}
}

class DialogWithEscapeableChildren extends Plugin {
public static get requires() {
return [ Dialog ] as const;
}

public init(): void {
const t = this.editor.locale.t;

this.editor.ui.componentFactory.add( 'dialogWithEscHandling', locale => {
const buttonView = new ButtonView( locale );

buttonView.set( {
label: t( 'Open dialog' ),
tooltip: true,
withText: true
} );

buttonView.on( 'execute', () => {
const dialog = this.editor.plugins.get( 'Dialog' );

dialog.show( {
id: 'dialogWithEscHandling',
isModal: true,
title: t( 'Dialog with esc handling' ),
content: new ViewWithEscSupport(),
actionButtons: [

{
label: t( 'Cancel' ),
withText: true,
onExecute: () => dialog.hide()
}
]
} );
} );

return buttonView;
} );
}
}

ClassicEditor.create( document.querySelector( '#editor' ) as HTMLElement, {
plugins: [
Essentials,
Autoformat,
BlockQuote,
Bold,
Heading,
Image,
ImageCaption,
ImageStyle,
ImageToolbar,
Indent,
Italic,
Link,
List,
MediaEmbed,
Paragraph,
Table,
TableToolbar,

FindAndReplace,
SpecialCharacters,
SpecialCharactersEssentials,
SourceEditing,

DialogWithEscapeableChildren
],
toolbar: {
items: [
'dialogWithEscHandling',
'|',
'accessibilityHelp', 'heading', 'bold', 'italic', 'link', 'sourceediting', 'findAndReplace'
],
shouldNotGroupWhenFull: true
},
image: {
toolbar: [ 'imageStyle:inline', 'imageStyle:block', 'imageStyle:side', '|', 'imageTextAlternative' ]
},
ui: {
viewportOffset: {
top: 50
}
}
} )
.then( editor => {
Object.assign( window, { editor } );
} )
.catch( err => {
console.error( err.stack );
} );