Initial implementation of the block quote feature #5
Conversation
| // 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 |
There was a problem hiding this comment.
We should not need to do tricks like this. There should be a way to to have both listeners working without having to think which should be registered when. We should take a look at lists listener if it can be fixed. If anything, we should consider adding list listener with higher priority.
There was a problem hiding this comment.
I can't recall now, but AFAIR I checked this and the problem there was bigger ;< Basically, we had a too low granularity of priorities – there's enter feature listener, list feature and this – enter uses low, list uses normal. I don't want enter to use lowest because then you won't be able to postfix it (unless you use the same method as I did here). I'd leave this for now and wait for more such issues. Also, there's https://github.com/ckeditor/ckeditor5-link/issues/87 which is strongly related but for which I don't have energy now.
But I definitely have to report it in ckeditor5. I'll do that later.
There was a problem hiding this comment.
I updated my previous comment with more details.
src/blockquotecommand.js
Outdated
| * Updates command's {@link #value} based on the current selection. | ||
| */ | ||
| refreshValue() { | ||
| const firstBlock = this.editor.document.selection.getSelectedBlocks().next().value; |
There was a problem hiding this comment.
You could have used first, right?
There was a problem hiding this comment.
Yep. Old code. Will fix it.
|
I played with manual test a bit and it looks fine. The only thing I am concerned with is unquoting single blocks instead the whole quote. For me, this seems a bit counterintuitive. I understand that this gives more flexibility and requires less clicks for user, but I am not sold on this. Other than that and two "hacks" in |
You may be right. I thought I copied the behaviour of CKEditor 4 and I did, but there are small differences. E.g. in CKE4 you unquote entire list, even if you have the selection in just one of its items. This is because for the BQ feature the list is a single element. In CKE5 a list is multiple elements (each item is separate) so you get a different behaviour. Still – in CKE4 unquoting blocks behave like in CKE5. I'll create a separate ticket for this. Theoretically, removing the partial unquoting mechanism would actually simplify the code, but what e.g. if you have a non-collapsed selection? It's a different case than collapsed one because it's very likely that you made this selection non-collapsed on purpose. So, what I'm thinking on now is that unquoting a collapsed selection should remove the entire quote and unquoting a non-collapsed selection should unquote only the touched blocks. We can discuss this, but since it's not critical, it shouldn't block us. Done: https://github.com/ckeditor/ckeditor5-block-quote/issues/6 |
Suggested merge commit message (convention)
Feature: Introduced the block quote feature. Closes ckeditor/ckeditor5#3311.
Additional information
Backspace handling will be fixed in https://github.com/ckeditor/ckeditor5-engine/issues/710. Right now, some weird things may happen, because blocks are incorrectly merged.
Enter should be fine already.
For the time being, the image cannot be quoted – see https://github.com/ckeditor/ckeditor5-image/issues/54.