diff --git a/packages/ckeditor5-ui/src/dialog/dialogview.ts b/packages/ckeditor5-ui/src/dialog/dialogview.ts index 56b35b80028..25c855220f6 100644 --- a/packages/ckeditor5-ui/src/dialog/dialogview.ts +++ b/packages/ckeditor5-ui/src/dialog/dialogview.ts @@ -290,8 +290,12 @@ export default class DialogView extends /* #__PURE__ */ DraggableViewMixin( View super.render(); this.keystrokes.set( 'Esc', ( data, cancel ) => { - this.fire( '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( 'close', { source: 'escKeyPress' } ); + cancel(); + } } ); // Support for dragging the modal. diff --git a/packages/ckeditor5-ui/tests/dialog/dialogview.js b/packages/ckeditor5-ui/tests/dialog/dialogview.js index b2e95e13958..3fb4fb50a0d 100644 --- a/packages/ckeditor5-ui/tests/dialog/dialogview.js +++ b/packages/ckeditor5-ui/tests/dialog/dialogview.js @@ -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', () => { diff --git a/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.html b/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.html new file mode 100644 index 00000000000..bdb46fc5fd5 --- /dev/null +++ b/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.html @@ -0,0 +1,24 @@ +
+

Heading 1

+

Paragraph

+

Bold Italic Link

+
    +
  • UL List item 1
  • +
  • UL List item 2
  • +
+
    +
  1. OL List item 1
  2. +
  3. OL List item 2
  4. +
+
+ bar +
+
+

Quote

+
    +
  • Quoted UL List item 1
  • +
  • Quoted UL List item 2
  • +
+

Quote

+
+
diff --git a/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.md b/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.md new file mode 100644 index 00000000000..0ae7c0d1bd5 --- /dev/null +++ b/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.md @@ -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. diff --git a/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.ts b/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.ts new file mode 100644 index 00000000000..21c581e6061 --- /dev/null +++ b/packages/ckeditor5-ui/tests/manual/dialog/dialog-esc-key-handling.ts @@ -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 ); + } );