Skip to content

Commit

Permalink
Merge pull request #8569 from ckeditor/i/8137
Browse files Browse the repository at this point in the history 
Feature (typing): Empty block element at the beginning of the limit element should be converted to a paragraph on backspace key press. Closes #8137.

Feature (block-quote): Block-quote should be split on backspace key press at the beginning of the block-quote. Closes #7636.

Fix (list): The `delete` event handler listening on a higher priority to avoid intercepting by the block-quote and widget handler. Closes #8706.
  • Loading branch information
@Reinmar
Reinmar authored Dec 22, 2020
2 parents 8d9e06b + a38fa9b commit 2d9954c
Show file tree
Hide file tree
Showing 6 changed files with 232 additions and 26 deletions.
53 changes: 36 additions & 17 deletions packages/ckeditor5-block-quote/src/blockquoteediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import priorities from '@ckeditor/ckeditor5-utils/src/priorities';

import BlockQuoteCommand from './blockquotecommand';

Expand Down Expand Up @@ -102,31 +103,49 @@ export default class BlockQuoteEditing extends Plugin {

return false;
} );
}

/**
* @inheritDoc
*/
afterInit() {
const editor = this.editor;
const command = editor.commands.get( 'blockQuote' );
const viewDocument = this.editor.editing.view.document;
const selection = editor.model.document.selection;
const blockQuoteCommand = editor.commands.get( 'blockQuote' );

// Overwrite default Enter key behavior.
// If Enter key is pressed with selection collapsed in empty block inside a quote, break the quote.
// This listener is added in afterInit in order to register it after list's feature listener.
// We can't use a priority for this, because 'low' is already used by the enter feature, unless
// we'd use numeric priority in this case.
this.listenTo( this.editor.editing.view.document, 'enter', ( evt, data ) => {
const doc = this.editor.model.document;
const positionParent = doc.selection.getLastPosition().parent;
//
// Priority normal - 10 to override default handler but not list's feature listener.
this.listenTo( viewDocument, 'enter', ( evt, data ) => {
if ( !selection.isCollapsed || !blockQuoteCommand.value ) {
return;
}

if ( doc.selection.isCollapsed && positionParent.isEmpty && command.value ) {
this.editor.execute( 'blockQuote' );
this.editor.editing.view.scrollToTheSelection();
const positionParent = selection.getLastPosition().parent;

if ( positionParent.isEmpty ) {
editor.execute( 'blockQuote' );
editor.editing.view.scrollToTheSelection();

data.preventDefault();
evt.stop();
}
} );
}, { priority: priorities.normal - 10 } );

// Overwrite default Backspace key behavior.
// If Backspace key is pressed with selection collapsed in first empty block inside a quote, break the quote.
//
// Priority high + 5 to override widget's feature listener but not list's feature listener.
this.listenTo( viewDocument, 'delete', ( evt, data ) => {
if ( data.direction != 'backward' || !selection.isCollapsed || !blockQuoteCommand.value ) {
return;
}

const positionParent = selection.getLastPosition().parent;

if ( positionParent.isEmpty && !positionParent.previousSibling ) {
editor.execute( 'blockQuote' );
editor.editing.view.scrollToTheSelection();

data.preventDefault();
evt.stop();
}
}, { priority: priorities.high + 5 } );
}
}
47 changes: 43 additions & 4 deletions packages/ckeditor5-block-quote/tests/integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ describe( 'BlockQuote integration', () => {
);
} );

it( 'removes empty quote when merging into another quote', () => {
it( 'unwraps empty quote when the backspace key pressed in the first empty paragraph in a quote', () => {
const data = fakeEventData();

setModelData( model,
Expand All @@ -295,12 +295,13 @@ describe( 'BlockQuote integration', () => {

expect( getModelData( model ) ).to.equal(
'<paragraph>x</paragraph>' +
'<blockQuote><paragraph>a[]</paragraph></blockQuote>' +
'<blockQuote><paragraph>a</paragraph></blockQuote>' +
'<paragraph>[]</paragraph>' +
'<paragraph>y</paragraph>'
);
} );

it( 'removes empty quote when merging into a paragraph', () => {
it( 'unwraps empty quote when the backspace key pressed in the empty paragraph that is the only content of quote', () => {
const data = fakeEventData();

setModelData( model,
Expand All @@ -312,7 +313,45 @@ describe( 'BlockQuote integration', () => {
viewDocument.fire( 'delete', data );

expect( getModelData( model ) ).to.equal(
'<paragraph>x[]</paragraph>' +
'<paragraph>x</paragraph>' +
'<paragraph>[]</paragraph>' +
'<paragraph>y</paragraph>'
);
} );

it( 'unwraps quote from the first paragraph when the backspace key pressed', () => {
const data = fakeEventData();

setModelData( model,
'<paragraph>x</paragraph>' +
'<blockQuote><paragraph>[]</paragraph><paragraph>foo</paragraph></blockQuote>' +
'<paragraph>y</paragraph>'
);

viewDocument.fire( 'delete', data );

expect( getModelData( model ) ).to.equal(
'<paragraph>x</paragraph>' +
'<paragraph>[]</paragraph>' +
'<blockQuote><paragraph>foo</paragraph></blockQuote>' +
'<paragraph>y</paragraph>'
);
} );

it( 'merges paragraphs in a quote when the backspace key pressed not in the first paragraph', () => {
const data = fakeEventData();

setModelData( model,
'<paragraph>x</paragraph>' +
'<blockQuote><paragraph></paragraph><paragraph>[]</paragraph></blockQuote>' +
'<paragraph>y</paragraph>'
);

viewDocument.fire( 'delete', data );

expect( getModelData( model ) ).to.equal(
'<paragraph>x</paragraph>' +
'<blockQuote><paragraph>[]</paragraph></blockQuote>' +
'<paragraph>y</paragraph>'
);
} );
Expand Down
5 changes: 4 additions & 1 deletion packages/ckeditor5-list/src/listediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import IndentCommand from './indentcommand';

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import priorities from '@ckeditor/ckeditor5-utils/src/priorities';

import {
cleanList,
Expand Down Expand Up @@ -134,6 +135,8 @@ export default class ListEditing extends Plugin {

// Overwrite default Backspace key behavior.
// If Backspace key is pressed with selection collapsed on first position in first list item, outdent it. #83
//
// Priority high + 10 to override widget and blockquote feature listener.
this.listenTo( viewDocument, 'delete', ( evt, data ) => {
// Check conditions from those that require less computations like those immediately available.
if ( data.direction !== 'backward' ) {
Expand Down Expand Up @@ -168,7 +171,7 @@ export default class ListEditing extends Plugin {

data.preventDefault();
evt.stop();
}, { priority: 'high' } );
}, { priority: priorities.high + 10 } );

const getCommandExecuter = commandName => {
return ( data, cancel ) => {
Expand Down
15 changes: 15 additions & 0 deletions packages/ckeditor5-list/tests/listediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,21 @@ describe( 'ListEditing', () => {

sinon.assert.calledWithExactly( editor.execute, 'outdentList' );
} );

it( 'should outdent empty list when list is nested in block quote', () => {
const domEvtDataStub = { preventDefault() {}, direction: 'backward' };

sinon.spy( editor, 'execute' );

setModelData(
model,
'<paragraph>x</paragraph><blockQuote><listItem listType="bulleted" listIndent="0">[]</listItem></blockQuote>'
);

editor.editing.view.document.fire( 'delete', domEvtDataStub );

sinon.assert.calledWithExactly( editor.execute, 'outdentList' );
} );
} );

describe( 'tab key handling callback', () => {
Expand Down
61 changes: 60 additions & 1 deletion packages/ckeditor5-typing/src/deletecommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export default class DeleteCommand extends Command {
this._buffer.lock();

const selection = writer.createSelection( options.selection || doc.selection );
const sequence = options.sequence || 1;

// Do not replace the whole selected content if selection was collapsed.
// This prevents such situation:
Expand All @@ -91,12 +92,20 @@ export default class DeleteCommand extends Command {
}

// Check if deleting in an empty editor. See #61.
if ( this._shouldEntireContentBeReplacedWithParagraph( options.sequence || 1 ) ) {
if ( this._shouldEntireContentBeReplacedWithParagraph( sequence ) ) {
this._replaceEntireContentWithParagraph( writer );

return;
}

// Check if deleting in the first empty block.
// See https://github.com/ckeditor/ckeditor5/issues/8137.
if ( this._shouldReplaceFirstBlockWithParagraph( selection, sequence ) ) {
this.editor.execute( 'paragraph', { selection } );

return;
}

// If selection is still collapsed, then there's nothing to delete.
if ( selection.isCollapsed ) {
return;
Expand Down Expand Up @@ -180,6 +189,7 @@ export default class DeleteCommand extends Command {
* The entire content is replaced with the paragraph. Selection is moved inside the paragraph.
*
* @private
* @param {module:engine/model/writer~Writer} writer The model writer.
*/
_replaceEntireContentWithParagraph( writer ) {
const model = this.editor.model;
Expand All @@ -193,4 +203,53 @@ export default class DeleteCommand extends Command {

writer.setSelection( paragraph, 0 );
}

/**
* Checks if the selection is inside an empty element that is the first child of the limit element
* and should be replaced with a paragraph.
*
* @private
* @param {module:engine/model/selection~Selection} selection The selection.
* @param {Number} sequence A number describing which subsequent delete event it is without the key being released.
* @returns {Boolean}
*/
_shouldReplaceFirstBlockWithParagraph( selection, sequence ) {
const model = this.editor.model;

// Does nothing if user pressed and held the "Backspace" key or it was a "Delete" button.
if ( sequence > 1 || this.direction != 'backward' ) {
return false;
}

if ( !selection.isCollapsed ) {
return false;
}

const position = selection.getFirstPosition();
const limitElement = model.schema.getLimitElement( position );
const limitElementFirstChild = limitElement.getChild( 0 );

// Only elements that are direct children of the limit element can be replaced.
// Unwrapping from a block quote should be handled in a dedicated feature.
if ( position.parent != limitElementFirstChild ) {
return false;
}

// A block should be replaced only if it was empty.
if ( !selection.containsEntireContent( limitElementFirstChild ) ) {
return false;
}

// Replace with a paragraph only if it's allowed there.
if ( !model.schema.checkChild( limitElement, 'paragraph' ) ) {
return false;
}

// Does nothing if the limit element already contains only a paragraph.
if ( limitElementFirstChild.name == 'paragraph' ) {
return false;
}

return true;
}
}
77 changes: 74 additions & 3 deletions packages/ckeditor5-typing/tests/deletecommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';
import DeleteCommand from '../src/deletecommand';
import Delete from '../src/delete';
import ChangeBuffer from '../src/utils/changebuffer';
import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';

import ParagraphCommand from '@ckeditor/ckeditor5-paragraph/src/paragraphcommand';
import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';

import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';

describe( 'DeleteCommand', () => {
let editor, model, doc;

Expand All @@ -26,6 +29,8 @@ describe( 'DeleteCommand', () => {
const command = new DeleteCommand( editor, 'backward' );
editor.commands.add( 'delete', command );

editor.commands.add( 'paragraph', new ParagraphCommand( editor ) );

model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
model.schema.register( 'heading1', { inheritAllFrom: '$block' } );
} );
Expand Down Expand Up @@ -315,5 +320,71 @@ describe( 'DeleteCommand', () => {

expect( getData( model ) ).to.equal( '<heading1>[]</heading1>' );
} );

describe( 'with the empty first block', () => {
it( 'replaces the first empty block with paragraph', () => {
setData( model, '<heading1>[]</heading1><paragraph>foo</paragraph>' );

editor.execute( 'delete' );

expect( getData( model ) ).to.equal( '<paragraph>[]</paragraph><paragraph>foo</paragraph>' );
} );

it( 'does not replace an element when Backspace key is held', () => {
setData( model, '<heading1>foo[]</heading1><paragraph>bar</paragraph>' );

for ( let sequence = 1; sequence < 10; ++sequence ) {
editor.execute( 'delete', { sequence } );
}

expect( getData( model ) ).to.equal( '<heading1>[]</heading1><paragraph>bar</paragraph>' );
} );

it( 'does not replace with paragraph in another paragraph already occurs in limit element', () => {
setData( model, '<paragraph>[]</paragraph><paragraph>foo</paragraph>' );

const element = doc.getRoot().getNodeByPath( [ 0 ] );

editor.execute( 'delete' );

expect( element ).is.equal( doc.getRoot().getNodeByPath( [ 0 ] ) );
expect( getData( model ) ).to.equal( '<paragraph>[]</paragraph><paragraph>foo</paragraph>' );
} );

it( 'does not replace an element if a paragraph is not allowed in current position', () => {
model.schema.addChildCheck( ( ctx, childDef ) => {
if ( ctx.endsWith( '$root' ) && childDef.name == 'paragraph' ) {
return false;
}
} );

setData( model, '<heading1>[]</heading1><heading1>foo</heading1>' );

editor.execute( 'delete' );

expect( getData( model ) ).to.equal( '<heading1>[]</heading1><heading1>foo</heading1>' );
} );

it( 'does not replace an element if it\'s not empty', () => {
setData( model, '<heading1>[]foo</heading1><paragraph>bar</paragraph>' );

editor.execute( 'delete' );

expect( getData( model ) ).to.equal( '<heading1>[]foo</heading1><paragraph>bar</paragraph>' );
} );

it( 'does not replace an element if it\'s wrapped with some other element', () => {
model.schema.register( 'blockQuote', {
allowWhere: '$block',
allowContentOf: '$root'
} );

setData( model, '<blockQuote><heading1>[]</heading1></blockQuote><paragraph>bar</paragraph>' );

editor.execute( 'delete' );

expect( getData( model ) ).to.equal( '<blockQuote><heading1>[]</heading1></blockQuote><paragraph>bar</paragraph>' );
} );
} );
} );
} );

0 comments on commit 2d9954c

Please sign in to comment.