From f701a5036637fb83f3b2c48da03f1115d255ede5 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Sat, 19 Nov 2022 23:27:06 +0100 Subject: [PATCH 1/5] Other: Made `enterBlock()` helper publicly accessible through `EnterCommand#enterBlock()`. --- packages/ckeditor5-enter/src/entercommand.ts | 120 ++++++++++-------- .../ckeditor5-enter/tests/entercommand.js | 45 ++++++- 2 files changed, 108 insertions(+), 57 deletions(-) diff --git a/packages/ckeditor5-enter/src/entercommand.ts b/packages/ckeditor5-enter/src/entercommand.ts index 8d179f4c5ff..977c7135916 100644 --- a/packages/ckeditor5-enter/src/entercommand.ts +++ b/packages/ckeditor5-enter/src/entercommand.ts @@ -29,75 +29,89 @@ export default class EnterCommand extends Command { * @inheritDoc */ public override execute(): void { - const model = this.editor.model; - const doc = model.document; - - model.change( writer => { - enterBlock( this.editor.model, writer, doc.selection, model.schema ); + this.editor.model.change( writer => { + this.enterBlock( writer ); this.fire( 'afterExecute', { writer } ); } ); } -} -export type EnterCommandAfterExecuteEvent = { - name: 'afterExecute'; - args: [ { writer: Writer } ]; -}; + /** + * Splits a block where the document selection is placed, in the way how the Enter key is expected to work: + * + *

Foo[]bar

->

Foo

[]bar

+ *

Foobar[]

->

Foobar

[]

+ *

Fo[ob]ar

->

Fo

[]ar

+ * + * In some cases, the split will not happen: + * + * 1. The selection parent element is a limit element (`
A[bc]d
` -> `
A[]d
`). + * 2. The selection spans over multiple elements (`x[x

y]y

` -> `x

[]y

`). + * + * @param {module:engine/model/writer~Writer} writer Writer to use when performing the enter action. + * @returns {Boolean} `true` if a block was split, `false` otherwise. + */ + public enterBlock( writer: Writer ): boolean { + const model = this.editor.model; + const selection = model.document.selection; + const schema = model.schema; + const isSelectionEmpty = selection.isCollapsed; + const range = selection.getFirstRange()!; + const startElement = range.start.parent as Element; + const endElement = range.end.parent as Element; -// Creates a new block in the way that the Enter key is expected to work. -// -// @param {module:engine/model~Model} model -// @param {module:engine/model/writer~Writer} writer -// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection -// Selection on which the action should be performed. -// @param {module:engine/model/schema~Schema} schema -function enterBlock( model: Model, writer: Writer, selection: DocumentSelection, schema: Schema ): void { - const isSelectionEmpty = selection.isCollapsed; - const range = selection.getFirstRange()!; - const startElement = range.start.parent as Element; - const endElement = range.end.parent as Element; - - // Don't touch the roots and other limit elements. - if ( schema.isLimit( startElement ) || schema.isLimit( endElement ) ) { - // Delete the selected content but only if inside a single limit element. - // Abort, when crossing limit elements boundary (e.g. x[xdonttouchmey]y). - // This is an edge case and it's hard to tell what should actually happen because such a selection - // is not entirely valid. - if ( !isSelectionEmpty && startElement == endElement ) { - model.deleteContent( selection ); + // Don't touch the roots and other limit elements. + if ( schema.isLimit( startElement ) || schema.isLimit( endElement ) ) { + // Delete the selected content but only if inside a single limit element. + // Abort, when crossing limit elements boundary (e.g. x[xdonttouchmey]y). + // This is an edge case and it's hard to tell what should actually happen because such a selection + // is not entirely valid. + if ( !isSelectionEmpty && startElement == endElement ) { + model.deleteContent( selection ); + } + + return false; } - return; - } + if ( isSelectionEmpty ) { + const attributesToCopy = getCopyOnEnterAttributes( writer.model.schema, selection.getAttributes() ); - if ( isSelectionEmpty ) { - const attributesToCopy = getCopyOnEnterAttributes( writer.model.schema, selection.getAttributes() ); + splitBlock( writer, range.start ); + writer.setSelectionAttribute( attributesToCopy ); - splitBlock( writer, range.start ); - writer.setSelectionAttribute( attributesToCopy ); - } else { - const leaveUnmerged = !( range.start.isAtStart && range.end.isAtEnd ); - const isContainedWithinOneElement = ( startElement == endElement ); + return true; + } else { + const leaveUnmerged = !( range.start.isAtStart && range.end.isAtEnd ); + const isContainedWithinOneElement = ( startElement == endElement ); - model.deleteContent( selection, { leaveUnmerged } ); + model.deleteContent( selection, { leaveUnmerged } ); - if ( leaveUnmerged ) { - // Partially selected elements. - // - // x[xx]x -> x^x -> x^x - if ( isContainedWithinOneElement ) { - splitBlock( writer, selection.focus! ); - } - // Selection over multiple elements. - // - // x[x

y]y

-> x^

y

-> x

^y

- else { - writer.setSelection( endElement, 0 ); + if ( leaveUnmerged ) { + // Partially selected elements. + // + // x[xx]x -> x^x -> x^x + if ( isContainedWithinOneElement ) { + splitBlock( writer, selection.focus! ); + + return true; + } + // Selection over multiple elements. + // + // x[x

y]y

-> x^

y

-> x

^y

+ else { + writer.setSelection( endElement, 0 ); + } } } + + return false; } } +export type EnterCommandAfterExecuteEvent = { + name: 'afterExecute'; + args: [ { writer: Writer } ]; +}; + function splitBlock( writer: Writer, splitPos: Position ): void { writer.split( splitPos ); writer.setSelection( splitPos.parent.nextSibling, 0 ); diff --git a/packages/ckeditor5-enter/tests/entercommand.js b/packages/ckeditor5-enter/tests/entercommand.js index a2fd6d72d9f..8b1c3e44d81 100644 --- a/packages/ckeditor5-enter/tests/entercommand.js +++ b/packages/ckeditor5-enter/tests/entercommand.js @@ -57,6 +57,35 @@ describe( 'EnterCommand', () => { } ); describe( 'execute()', () => { + it( 'uses enterBlock()', () => { + setData( model, '

foo[]bar

' ); + + sinon.spy( command, 'enterBlock' ); + + editor.execute( 'enter' ); + + expect( command.enterBlock.called ).to.be.true; + } ); + + it( 'fires afterExecute() event with the current writer as a parameter', done => { + setData( model, '

foo[]bar

' ); + + let currentWriter; + + command.on( 'afterExecute', ( evt, { writer } ) => { + expect( writer ).to.equal( currentWriter ); + + done(); + } ); + + model.change( writer => { + currentWriter = writer; + editor.execute( 'enter' ); + } ); + } ); + } ); + + describe( 'enterBlock()', () => { describe( 'collapsed selection', () => { test( 'does nothing in the root', @@ -162,7 +191,9 @@ describe( 'EnterCommand', () => { model.change( () => { setData( model, '

ba[r

f]oo

' ); - command.execute(); + model.change( writer => { + command.enterBlock( writer ); + } ); expect( getData( model ) ).to.equal( '

ba[r

f]oo

' ); } ); @@ -179,7 +210,9 @@ describe( 'EnterCommand', () => { // @TODO: Add option for setting selection direction to model utils. doc.selection._lastRangeBackward = true; - command.execute(); + model.change( writer => { + command.enterBlock( writer ); + } ); expect( getData( model ) ).to.equal( '

[]

' ); } ); @@ -191,7 +224,9 @@ describe( 'EnterCommand', () => { setData( model, '

[x]

' ); - command.execute(); + model.change( writer => { + command.enterBlock( writer ); + } ); expect( spy.calledOnce ).to.be.true; } ); @@ -201,7 +236,9 @@ describe( 'EnterCommand', () => { it( title, () => { setData( model, input ); - command.execute(); + model.change( writer => { + command.enterBlock( writer ); + } ); expect( getData( model ) ).to.equal( output ); } ); From 2de15fa6f97c1f19ae5d7c375f52445bef14e7a5 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 21 Nov 2022 11:36:48 +0100 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Arkadiusz Filipczak --- packages/ckeditor5-enter/src/entercommand.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-enter/src/entercommand.ts b/packages/ckeditor5-enter/src/entercommand.ts index 977c7135916..2caec5f614d 100644 --- a/packages/ckeditor5-enter/src/entercommand.ts +++ b/packages/ckeditor5-enter/src/entercommand.ts @@ -47,8 +47,8 @@ export default class EnterCommand extends Command { * 1. The selection parent element is a limit element (`
A[bc]d
` -> `
A[]d
`). * 2. The selection spans over multiple elements (`x[x

y]y

` -> `x

[]y

`). * - * @param {module:engine/model/writer~Writer} writer Writer to use when performing the enter action. - * @returns {Boolean} `true` if a block was split, `false` otherwise. + * @param writer Writer to use when performing the enter action. + * @returns `true` if a block was split, `false` otherwise. */ public enterBlock( writer: Writer ): boolean { const model = this.editor.model; From be227ed7205848c16a7731981447c08024286198 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 21 Nov 2022 12:24:51 +0100 Subject: [PATCH 3/5] Docs: Added JSDoc for `EnterCommand#enterBlock()`. --- packages/ckeditor5-enter/_src/entercommand.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/ckeditor5-enter/_src/entercommand.js b/packages/ckeditor5-enter/_src/entercommand.js index a8b6a87a7de..f191855c04a 100644 --- a/packages/ckeditor5-enter/_src/entercommand.js +++ b/packages/ckeditor5-enter/_src/entercommand.js @@ -28,6 +28,23 @@ export default class EnterCommand extends Command { this.fire( 'afterExecute', { writer } ); } ); } + + /** + * Splits a block where the document selection is placed, in the way how the Enter key is expected to work: + * + *

Foo[]bar

->

Foo

[]bar

+ *

Foobar[]

->

Foobar

[]

+ *

Fo[ob]ar

->

Fo

[]ar

+ * + * In some cases, the split will not happen: + * + * 1. The selection parent element is a limit element (`
A[bc]d
` -> `
A[]d
`). + * 2. The selection spans over multiple elements (`x[x

y]y

` -> `x

[]y

`). + * + * @method #enterBlock + * @param {module:engine/model/writer~Writer} writer Writer to use when performing the enter action. + * @returns {Boolean} `true` if a block was split, `false` otherwise. + */ } // Creates a new block in the way that the Enter key is expected to work. From 0c2fe5709ccada5855ca6614242b2341b2c1ebe1 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 21 Nov 2022 12:26:51 +0100 Subject: [PATCH 4/5] Docs: Improved `EnterCommand#enterBlock()` description. --- packages/ckeditor5-enter/_src/entercommand.js | 7 +++++-- packages/ckeditor5-enter/src/entercommand.ts | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-enter/_src/entercommand.js b/packages/ckeditor5-enter/_src/entercommand.js index f191855c04a..e3c2c4ddee4 100644 --- a/packages/ckeditor5-enter/_src/entercommand.js +++ b/packages/ckeditor5-enter/_src/entercommand.js @@ -38,8 +38,11 @@ export default class EnterCommand extends Command { * * In some cases, the split will not happen: * - * 1. The selection parent element is a limit element (`
A[bc]d
` -> `
A[]d
`). - * 2. The selection spans over multiple elements (`x[x

y]y

` -> `x

[]y

`). + * // The selection parent is a limit element: + *
A[bc]d
->
A[]d
+ * + * // The selection spans over multiple elements: + * x[x

y]y

-> x

[]y

* * @method #enterBlock * @param {module:engine/model/writer~Writer} writer Writer to use when performing the enter action. diff --git a/packages/ckeditor5-enter/src/entercommand.ts b/packages/ckeditor5-enter/src/entercommand.ts index 2caec5f614d..ccd68d807fb 100644 --- a/packages/ckeditor5-enter/src/entercommand.ts +++ b/packages/ckeditor5-enter/src/entercommand.ts @@ -44,8 +44,11 @@ export default class EnterCommand extends Command { * * In some cases, the split will not happen: * - * 1. The selection parent element is a limit element (`
A[bc]d
` -> `
A[]d
`). - * 2. The selection spans over multiple elements (`x[x

y]y

` -> `x

[]y

`). + * // The selection parent is a limit element: + *
A[bc]d
->
A[]d
+ * + * // The selection spans over multiple elements: + * x[x

y]y

-> x

[]y

* * @param writer Writer to use when performing the enter action. * @returns `true` if a block was split, `false` otherwise. From 4933c80f6590f4d0aa2166666f432fd2f0c57915 Mon Sep 17 00:00:00 2001 From: Arkadiusz Filipczak Date: Mon, 21 Nov 2022 13:30:59 +0100 Subject: [PATCH 5/5] Fixed failing test --- packages/ckeditor5-paragraph/src/paragraphbuttonui.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-paragraph/src/paragraphbuttonui.ts b/packages/ckeditor5-paragraph/src/paragraphbuttonui.ts index f58d0491a52..761626501c0 100644 --- a/packages/ckeditor5-paragraph/src/paragraphbuttonui.ts +++ b/packages/ckeditor5-paragraph/src/paragraphbuttonui.ts @@ -10,6 +10,8 @@ import { Plugin, icons } from '@ckeditor/ckeditor5-core'; import { ButtonView } from '@ckeditor/ckeditor5-ui'; +import type ParagraphCommand from './paragraphcommand'; + const icon = icons.paragraph; /** @@ -36,7 +38,7 @@ export default class ParagraphButtonUI extends Plugin { editor.ui.componentFactory.add( 'paragraph', locale => { const view = new ButtonView( locale ); - const command = editor.commands.get( 'paragraph' )!; + const command: ParagraphCommand = editor.commands.get( 'paragraph' )!; view.label = t( 'Paragraph' ); view.icon = icon;