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

Conversation

pomek
Copy link
Member

@pomek pomek commented Jul 18, 2017

Suggested merge commit message (convention)

Feature: DataController#deleteContent will leave a paragraph if the entire content was selected and removed. Closes ckeditor/ckeditor5#4110.

On the occasion $root element has been as a limit element in Schema in order to simplify the checks.


Additional information

Branch t/ckeditor5-engine/1012 in ckeditor5-enter should be merged along with this PR.

@pomek pomek requested a review from Reinmar July 18, 2017 11:30
@@ -49,6 +49,10 @@ describe( 'Schema', () => {
expect( schema.limits ).to.be.instanceOf( Set );
} );

it( 'should have defined the limits elements', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"should mark $root as a limit element"

@@ -233,12 +233,6 @@ describe( 'DataController', () => {
'<heading1>f[]</heading1><paragraph>x</paragraph>'
);

test(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to avoid deleting tests completely. This may be the only test which checks if the middle paragraph (foo) is deleted. So, either just adjust its result or add some content outside to not make it a full content selection (and adjust the title).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS. Note that you haven't added this test in the "should leave a paragraph if the entire content was selected" block.

@@ -32,8 +32,15 @@ export default function deleteContent( selection, batch, options = {} ) {
return;
}

const selRange = selection.getFirstRange();
// 0. Replace the entire content with paragraph.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to fix the numbering.

@Reinmar
Copy link
Member

Reinmar commented Jul 18, 2017

Some minor issues.

@pomek
Copy link
Member Author

pomek commented Jul 19, 2017

Ready to review once again.

@Reinmar Reinmar merged commit 17e70c3 into master Jul 19, 2017
@Reinmar Reinmar deleted the t/1012 branch July 19, 2017 07:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataController#deleteContent should leave a paragraph if the entire content was selected
2 participants