From 23bcd2ff7e39ad939a2bcdd2238335cf277bca3e Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 30 Mar 2023 19:13:21 +0200 Subject: [PATCH 1/2] Pressing Shift+Enter in Safari should fire soft enter event. --- packages/ckeditor5-enter/package.json | 3 +- packages/ckeditor5-enter/src/enterobserver.ts | 19 +++++++++- .../ckeditor5-enter/tests/enterobserver.js | 37 +++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-enter/package.json b/packages/ckeditor5-enter/package.json index f62740423e6..0562f06cb84 100644 --- a/packages/ckeditor5-enter/package.json +++ b/packages/ckeditor5-enter/package.json @@ -13,7 +13,8 @@ "main": "src/index.ts", "dependencies": { "@ckeditor/ckeditor5-core": "^36.0.1", - "@ckeditor/ckeditor5-engine": "^36.0.1" + "@ckeditor/ckeditor5-engine": "^36.0.1", + "@ckeditor/ckeditor5-utils": "^36.0.1" }, "devDependencies": { "@ckeditor/ckeditor5-basic-styles": "^36.0.1", diff --git a/packages/ckeditor5-enter/src/enterobserver.ts b/packages/ckeditor5-enter/src/enterobserver.ts index c10355336ee..c0f5a572423 100644 --- a/packages/ckeditor5-enter/src/enterobserver.ts +++ b/packages/ckeditor5-enter/src/enterobserver.ts @@ -13,9 +13,12 @@ import { BubblingEventInfo, type View, type ViewDocumentInputEvent, - type BubblingEvent + type BubblingEvent, + type ViewDocumentKeyDownEvent } from '@ckeditor/ckeditor5-engine'; +import { env } from '@ckeditor/ckeditor5-utils'; + const ENTER_EVENT_TYPES: Record = { insertParagraph: { isSoft: false }, insertLineBreak: { isSoft: true } @@ -32,14 +35,26 @@ export default class EnterObserver extends Observer { super( view ); const doc = this.document; + let shiftPressed = false; + + doc.on( 'keydown', ( evt, data ) => { + shiftPressed = data.shiftKey; + } ); doc.on( 'beforeinput', ( evt, data ) => { if ( !this.isEnabled ) { return; } + let inputType = data.inputType; + + // See https://github.com/ckeditor/ckeditor5/issues/13321. + if ( env.isSafari && shiftPressed && inputType == 'insertParagraph' ) { + inputType = 'insertLineBreak'; + } + const domEvent = data.domEvent; - const enterEventSpec = ENTER_EVENT_TYPES[ data.inputType ]; + const enterEventSpec = ENTER_EVENT_TYPES[ inputType ]; if ( !enterEventSpec ) { return; diff --git a/packages/ckeditor5-enter/tests/enterobserver.js b/packages/ckeditor5-enter/tests/enterobserver.js index f340dd4c1a1..d558b20ed13 100644 --- a/packages/ckeditor5-enter/tests/enterobserver.js +++ b/packages/ckeditor5-enter/tests/enterobserver.js @@ -9,7 +9,9 @@ import View from '@ckeditor/ckeditor5-engine/src/view/view'; import EnterObserver from '../src/enterobserver'; import createViewRoot from '@ckeditor/ckeditor5-engine/tests/view/_utils/createroot'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import env from '@ckeditor/ckeditor5-utils/src/env'; import { fireBeforeInputDomEvent } from '@ckeditor/ckeditor5-typing/tests/_utils/utils'; +import { getCode } from '@ckeditor/ckeditor5-utils'; describe( 'EnterObserver', () => { let view, viewDocument, enterSpy; @@ -84,6 +86,32 @@ describe( 'EnterObserver', () => { sinon.assert.notCalled( enterSpy ); } ); + // See https://github.com/ckeditor/ckeditor5/issues/13321. + it( 'should handle the insertParagraph input type and fire the soft enter event if shift key is pressed in Safari', () => { + sinon.stub( env, 'isSafari' ).value( true ); + + fireKeyEvent( 'enter', { shiftKey: true } ); + + fireBeforeInputDomEvent( domRoot, { + inputType: 'insertParagraph' + } ); + + sinon.assert.calledOnce( enterSpy ); + sinon.assert.calledWithMatch( enterSpy, {}, { isSoft: true } ); + expect( enterSpy.firstCall.args[ 1 ] ).to.have.property( 'isSoft', true ); + + // Verify if the effect is not persistent. + fireKeyEvent( 'enter', { shiftKey: false } ); + + fireBeforeInputDomEvent( domRoot, { + inputType: 'insertParagraph' + } ); + + sinon.assert.calledTwice( enterSpy ); + expect( enterSpy.firstCall.args[ 1 ] ).to.have.property( 'isSoft', true ); + expect( enterSpy.secondCall.args[ 1 ] ).to.have.property( 'isSoft', false ); + } ); + it( 'should never preventDefault() the beforeinput event', () => { let interceptedEventData; @@ -136,4 +164,13 @@ describe( 'EnterObserver', () => { view.getObserver( EnterObserver ).stopObserving(); } ).to.not.throw(); } ); + + function fireKeyEvent( key, options ) { + viewDocument.fire( 'keydown', { + keyCode: getCode( key ), + preventDefault: () => {}, + domTarget: document.body, + ...options + } ); + } } ); From cdb17cd4cce3a307775b0c5ff7ad60ce261a5bdd Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 31 Mar 2023 14:19:50 +0200 Subject: [PATCH 2/2] Review fixes. --- .../ckeditor5-enter/tests/enterobserver.js | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-enter/tests/enterobserver.js b/packages/ckeditor5-enter/tests/enterobserver.js index d558b20ed13..f2a6d4ea996 100644 --- a/packages/ckeditor5-enter/tests/enterobserver.js +++ b/packages/ckeditor5-enter/tests/enterobserver.js @@ -9,9 +9,8 @@ import View from '@ckeditor/ckeditor5-engine/src/view/view'; import EnterObserver from '../src/enterobserver'; import createViewRoot from '@ckeditor/ckeditor5-engine/tests/view/_utils/createroot'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -import env from '@ckeditor/ckeditor5-utils/src/env'; import { fireBeforeInputDomEvent } from '@ckeditor/ckeditor5-typing/tests/_utils/utils'; -import { getCode } from '@ckeditor/ckeditor5-utils'; +import { getCode, env } from '@ckeditor/ckeditor5-utils'; describe( 'EnterObserver', () => { let view, viewDocument, enterSpy; @@ -112,6 +111,23 @@ describe( 'EnterObserver', () => { expect( enterSpy.secondCall.args[ 1 ] ).to.have.property( 'isSoft', false ); } ); + // See https://github.com/ckeditor/ckeditor5/issues/13321. + it( 'should handle the insertParagraph input type and fire the enter event if shift key was pressed before in Safari', () => { + sinon.stub( env, 'isSafari' ).value( true ); + + fireKeyEvent( 'shift', { shiftKey: true }, 'keydown' ); + fireKeyEvent( 'shift', { shiftKey: false }, 'keyup' ); + fireKeyEvent( 'enter', { shiftKey: false }, 'keydown' ); + + fireBeforeInputDomEvent( domRoot, { + inputType: 'insertParagraph' + } ); + + sinon.assert.calledOnce( enterSpy ); + sinon.assert.calledWithMatch( enterSpy, {}, { isSoft: false } ); + expect( enterSpy.firstCall.args[ 1 ] ).to.have.property( 'isSoft', false ); + } ); + it( 'should never preventDefault() the beforeinput event', () => { let interceptedEventData; @@ -165,8 +181,8 @@ describe( 'EnterObserver', () => { } ).to.not.throw(); } ); - function fireKeyEvent( key, options ) { - viewDocument.fire( 'keydown', { + function fireKeyEvent( key, options, type = 'keydown' ) { + viewDocument.fire( type, { keyCode: getCode( key ), preventDefault: () => {}, domTarget: document.body,