Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block new line inside list item no longer breaks list structure #2501

Merged
merged 12 commits into from
Jan 2, 2019
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Fixed Issues:
* [#2506](https://github.com/ckeditor/ckeditor-dev/issues/2506): Fixed: [Enhanced Image](https://ckeditor.com/cke4/addon/image2) throws type error when empty figure tag with 'image' class is upcasted.
* [#2650](https://github.com/ckeditor/ckeditor-dev/issues/2650): Fixed: [Table](https://ckeditor.com/cke4/addon/table) dialog validator fails when function `getValue` is defined in global scope.
* [#2690](https://github.com/ckeditor/ckeditor-dev/issues/2690): Fixed: Decimal characters removed from inside ordered lists when pasting content using [Paste from Word](https://ckeditor.com/cke4/addon/pastefromword) plugin.
* [#2205](https://github.com/ckeditor/ckeditor-dev/issues/2205): Fixed: It is not possible to add new list items under an item containing block element.

Other Changes:

Expand Down
30 changes: 23 additions & 7 deletions plugins/enterkey/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,32 @@

newBlock;

// Exit the list when we're inside an empty list item block. (https://dev.ckeditor.com/ticket/5376)
if ( atBlockStart && atBlockEnd ) {
// Exit the list when we're inside an empty list item block. (https://dev.ckeditor.com/ticket/5376)
if ( block && ( block.is( 'li' ) || block.getParent().is( 'li' ) ) ) {
if ( block && atBlockStart && atBlockEnd ) {
var blockParent = block.getParent();

// Block placeholder breaks list structure (#2205).
if ( blockParent.is( 'li' ) && blockParent.getChildCount() > 1 ) {
var placeholder = new CKEDITOR.dom.element( 'li' ),
newRange = editor.createRange();

placeholder.insertAfter( blockParent );

block.remove();

newRange.setStart( placeholder, 0 );
editor.getSelection().selectRanges( [ newRange ] );

return;
}
// Exit the list when we're inside an empty list item block. (https://dev.ckeditor.com/ticket/5376).
else if ( block.is( 'li' ) || block.getParent().is( 'li' ) ) {
// Make sure to point to the li when dealing with empty list item.
if ( !block.is( 'li' ) )
if ( !block.is( 'li' ) ) {
block = block.getParent();
blockParent = block.getParent();
}

var blockParent = block.getParent(),
blockGrandParent = blockParent.getParent(),
var blockGrandParent = blockParent.getParent(),

firstChild = !block.hasPrevious(),
lastChild = !block.hasNext(),
Expand Down
87 changes: 87 additions & 0 deletions tests/plugins/enterkey/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@
config: {
enterMode: CKEDITOR.ENTER_BR
}
},
enterDIV: {
name: 'enterDIV',
config: {
enterMode: CKEDITOR.ENTER_P,
extraPlugins: 'div'
}
}
};

Expand Down Expand Up @@ -626,6 +633,86 @@
'<li>^&nbsp;</li>' +
'</ul>',

true, 'New item should be added to the list.', true );
},

// (#2205)
'test enterkey: block p placeholder at the end': function() {
assertEnter( 'enterP',
'<ul>' +
'<li>' +
'foo' +
'<p>^</p>' +
'</li>' +
'</ul>',

'<ul>' +
'<li>' +
'foo' +
'</li>' +
'<li>^&nbsp;</li>' +
'</ul>',

true, 'New item should be added to the list.', true );
},

// (#2205)
'test enterkey: block p placeholder at the beginning': function() {
assertEnter( 'enterP',
'<ul>' +
'<li>' +
'<p>^</p>' +
'foo' +
'</li>' +
'</ul>',

'<ul>' +
'<li>' +
'foo' +
'</li>' +
'<li>^&nbsp;</li>' +
'</ul>',

true, 'New item should be added to the list.', true );
},

// (#2205)
'test enterkey: block div placeholder at the beginning': function() {
assertEnter( 'enterP',
'<ul>' +
'<li>' +
'<div>^</div>' +
'foo' +
'</li>' +
'</ul>',

'<ul>' +
'<li>' +
'foo' +
'</li>' +
'<li>^&nbsp;</li>' +
'</ul>',

true, 'New item should be added to the list.', true );
},

// (#2205)
'test enterkey: block div placeholder at the end': function() {
assertEnter( 'enterP',
'<ul>' +
'<li>' +
'foo' +
'<div>^</div>' +
'</li>' +
'</ul>',

'<ul>' +
'<li>' +
'foo' +
'</li>' +
'<li>^&nbsp;</li>' +
'</ul>',

true, 'New item should be added to the list.', true );
}
} );
Expand Down
12 changes: 12 additions & 0 deletions tests/plugins/enterkey/manual/blockplaceholder.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<div id="editor">
<ul>
<li>
foo
<p><br></p>
</li>
</ul>
</div>

<script>
CKEDITOR.replace( 'editor' );
</script>
20 changes: 20 additions & 0 deletions tests/plugins/enterkey/manual/blockplaceholder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@bender-tags: 4.11.2, bug, 2205
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, enterkey, list

1. Place a cursor right under list item.

```
* foo
^ - put selection here
```

2. Press `Enter`.

## Expected

Empty line has been replaced by the new empty list item.

## Unexpected

List has been removed from the editor.
12 changes: 12 additions & 0 deletions tests/plugins/enterkey/manual/divplaceholder.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<div id="editor">
<ul>
<li>
foo
<div><br></div>
</li>
</ul>
</div>

<script>
CKEDITOR.replace( 'editor' );
</script>
20 changes: 20 additions & 0 deletions tests/plugins/enterkey/manual/divplaceholder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@bender-tags: 4.11.2, bug, 2205
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, enterkey, list, div

1. Place a cursor right under list item.

```
* foo
^ - put selection here
```

2. Press `Enter`.

## Expected

Empty line has been replaced by the new empty list item.

## Unexpected

List has been removed from the editor.