Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/1012: Handles with a paragraph when the entire content was removed #1022

Merged
merged 5 commits into from
Jul 19, 2017
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
78 changes: 70 additions & 8 deletions src/controller/deletecontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,24 @@ export default function deleteContent( selection, batch, options = {} ) {
return;
}

const selRange = selection.getFirstRange();
// 1. Replace the entire content with paragraph.
// See: https://github.com/ckeditor/ckeditor5-engine/issues/1012#issuecomment-315017594.
if ( shouldEntireContentBeReplacedWithParagraph( batch.document.schema, selection ) ) {
replaceEntireContentWithParagraph( batch, selection );

return;
}

const selRange = selection.getFirstRange();
const startPos = selRange.start;
const endPos = LivePosition.createFromPosition( selRange.end );

// 1. Remove the content if there is any.
// 2. Remove the content if there is any.
if ( !selRange.start.isTouching( selRange.end ) ) {
batch.remove( selRange );
}

// 2. Merge elements in the right branch to the elements in the left branch.
// 3. Merge elements in the right branch to the elements in the left branch.
// The only reasonable (in terms of data and selection correctness) case in which we need to do that is:
//
// <heading type=1>Fo[</heading><paragraph>]ar</paragraph> => <heading type=1>Fo^ar</heading>
Expand All @@ -56,13 +63,10 @@ export default function deleteContent( selection, batch, options = {} ) {

selection.collapse( startPos );

// 3. Autoparagraphing.
// 4. Autoparagraphing.
// Check if a text is allowed in the new container. If not, try to create a new paragraph (if it's allowed here).
if ( shouldAutoparagraph( batch.document, startPos ) ) {
const paragraph = new Element( 'paragraph' );
batch.insert( startPos, paragraph );

selection.collapse( paragraph );
insertParagraph( batch, startPos, selection );
}

endPos.detach();
Expand Down Expand Up @@ -163,3 +167,61 @@ function checkCanBeMerged( leftPos, rightPos ) {

return true;
}

// Returns the lowest limit element defined in `Schema.limits` for passed selection.
function getLimitElement( schema, selection ) {
let element = selection.getFirstRange().getCommonAncestor();

while ( !schema.limits.has( element.name ) ) {
if ( element.parent ) {
element = element.parent;
} else {
break;
}
}

return element;
}

function insertParagraph( batch, position, selection ) {
const paragraph = new Element( 'paragraph' );
batch.insert( position, paragraph );

selection.collapse( paragraph );
}

function replaceEntireContentWithParagraph( batch, selection ) {
const limitElement = getLimitElement( batch.document.schema, selection );

batch.remove( Range.createIn( limitElement ) );
insertParagraph( batch, Position.createAt( limitElement ), selection );
}

// We want to replace the entire content with a paragraph when:
// * the entire content is selected,
// * selection contains at least two elements,
// * whether the paragraph is allowed in schema in the common ancestor.
function shouldEntireContentBeReplacedWithParagraph( schema, selection ) {
const limitElement = getLimitElement( schema, selection );
const limitStartPosition = Position.createAt( limitElement );
const limitEndPosition = Position.createAt( limitElement, 'end' );

if (
!limitStartPosition.isTouching( selection.getFirstPosition() ) ||
!limitEndPosition.isTouching( selection.getLastPosition() )
) {
return false;
}

const range = selection.getFirstRange();

if ( range.start.parent == range.end.parent ) {
return false;
}

if ( !schema.check( { name: 'paragraph', inside: limitElement.name } ) ) {
return false;
}

return true;
}
2 changes: 2 additions & 0 deletions src/model/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ export default class Schema {
this.allow( { name: '$block', inside: '$root' } );
this.allow( { name: '$inline', inside: '$block' } );

this.limits.add( '$root' );

// TMP!
// Create an "all allowed" context in the schema for processing the pasted content.
// Read: https://github.com/ckeditor/ckeditor5-engine/issues/638#issuecomment-255086588
Expand Down
96 changes: 94 additions & 2 deletions tests/controller/deletecontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,8 @@ describe( 'DataController', () => {

test(
'leaves just one element when all selected',
'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]</paragraph>',
'<heading1>[]</heading1>'
'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]bar</paragraph>',
'<heading1>[]bar</heading1>'
);

it( 'uses remove delta instead of merge delta if merged element is empty', () => {
Expand Down Expand Up @@ -450,6 +450,8 @@ describe( 'DataController', () => {

const schema = doc.schema;

schema.limits.add( 'restrictedRoot' );

schema.registerItem( 'image', '$inline' );
schema.registerItem( 'paragraph', '$block' );
schema.registerItem( 'heading1', '$block' );
Expand All @@ -465,6 +467,8 @@ describe( 'DataController', () => {
// See also "in simple scenarios => deletes an element".

it( 'deletes two inline elements', () => {
doc.schema.limits.add( 'paragraph' );

setData(
doc,
'x[<image></image><image></image>]z',
Expand Down Expand Up @@ -659,6 +663,94 @@ describe( 'DataController', () => {
);
} );

describe( 'should leave a paragraph if the entire content was selected', () => {
beforeEach( () => {
doc = new Document();
doc.createRoot();

const schema = doc.schema;

schema.registerItem( 'div', '$block' );
schema.limits.add( 'div' );

schema.registerItem( 'article', '$block' );
schema.limits.add( 'article' );

schema.registerItem( 'image', '$inline' );
schema.objects.add( 'image' );

schema.registerItem( 'paragraph', '$block' );
schema.registerItem( 'heading1', '$block' );
schema.registerItem( 'heading2', '$block' );

schema.allow( { name: '$text', inside: '$root' } );

schema.allow( { name: 'image', inside: '$root' } );
schema.allow( { name: 'image', inside: 'heading1' } );
schema.allow( { name: 'heading1', inside: 'div' } );
schema.allow( { name: 'paragraph', inside: 'div' } );
schema.allow( { name: 'heading1', inside: 'article' } );
schema.allow( { name: 'heading2', inside: 'article' } );
} );

test(
'but not if only one block was selected',
'<heading1>[xx]</heading1>',
'<heading1>[]</heading1>'
);

test(
'when the entire heading and paragraph were selected',
'<heading1>[xx</heading1><paragraph>yy]</paragraph>',
'<paragraph>[]</paragraph>'
);

test(
'when the entire content was selected',
'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]</paragraph>',
'<paragraph>[]</paragraph>'
);

test(
'inside the limit element when the entire heading and paragraph were inside',
'<div><heading1>[xx</heading1><paragraph>yy]</paragraph></div>',
'<div><paragraph>[]</paragraph></div>'
);

test(
'but not if schema does not accept paragraph in limit element',
'<article><heading1>[xx</heading1><heading2>yy]</heading2></article>',
'<article><heading1>[]</heading1></article>'
);

test(
'but not if selection is not containing the whole content',
'<image></image><heading1>[xx</heading1><paragraph>yy]</paragraph>',
'<image></image><heading1>[]</heading1>'
);

test(
'but not if only single element is selected',
'<heading1>[<image></image>xx]</heading1>',
'<heading1>[]</heading1>'
);

it( 'when root element was not added as Schema.limits works fine as well', () => {
doc.createRoot( 'paragraph', 'paragraphRoot' );

setData(
doc,
'x[<image></image><image></image>]z',
{ rootName: 'paragraphRoot' }
);

deleteContent( doc.selection, doc.batch() );

expect( getData( doc, { rootName: 'paragraphRoot' } ) )
.to.equal( 'x[]z' );
} );
} );

function test( title, input, output, options ) {
it( title, () => {
setData( doc, input );
Expand Down
4 changes: 4 additions & 0 deletions tests/model/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ describe( 'Schema', () => {
expect( schema.limits ).to.be.instanceOf( Set );
} );

it( 'should mark $root as a limit element', () => {
expect( schema.limits.has( '$root' ) ).to.be.true;
} );

describe( '$clipboardHolder', () => {
it( 'should allow $block', () => {
expect( schema.check( { name: '$block', inside: [ '$clipboardHolder' ] } ) ).to.be.true;
Expand Down