Blockquote should be split on backspace key press. Empty block element at the beginning of the limit element should be converted to a paragraph on backspace key press. #8569
Conversation
…nt at the beginning of the limit element should be converted to a paragraph on backspace key press.
|
@ckeditor/qa-team could you please test it? |
|
Checked in various browsers, I didn't find any bugs. Seems to work fine 👍 The only thing that caught my attention, is that changing heading -> paragraph is not recognized by Track Changes ( block quote works ok ), but I guess this is a case for a follow-up. |
I changed a bit the way it's handled, now it should properly integrate with TC. |
|
Yes, now it shows a proper suggestion 👍 |
| @@ -168,7 +169,7 @@ export default class ListEditing extends Plugin { | |||
|
|
|||
| data.preventDefault(); | |||
| evt.stop(); | |||
| }, { priority: 'high' } ); | |||
There was a problem hiding this comment.
Why? It's useful if such quirks are documented in the code.
There was a problem hiding this comment.
I added a code comment.
| // 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 ) => { |
There was a problem hiding this comment.
Do you think this applies to blockQuotes only? Maybe it'd be simpler if this was generalized a bit and put directly in the delete handler?
There was a problem hiding this comment.
Unfortunately, it's a different case for list items (that are not wrapped) and blockQuotes (that are wrapped).
| 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 ) => { |
There was a problem hiding this comment.
Why did you need to change anything in the Enter key support in this PR? This PR from its history and description was all about the Backspace key, so this is confusing.
There was a problem hiding this comment.
It seems more reasonable to use -10 instead of using afterInit just to order listeners.
|
I left a couple of questions and suggestions. I'm mostly worried about:
|
|
As we discussed F2F I created a followup: #8640 |
|
|
||
| editor.editing.view.document.fire( 'delete', domEvtDataStub ); | ||
|
|
||
| sinon.assert.calledWithExactly( editor.execute, 'outdentList' ); |
There was a problem hiding this comment.
I know you were consistent with all the other tests here, but just to clarify – I think all of them should be checking the actual content of the model too, not only whether the command was executed. We don't know if some other handler didn't react or the command didn't do something unexpected (quite probable when changing its behavior).
Suggested merge commit message (convention)
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
deleteevent handler listening on a higher priority to avoid intercepting by the block-quote and widget handler. Closes #8706.Additional information