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

Blockquote implementation #3905

Closed
szymonkups opened this issue Dec 1, 2016 · 7 comments · Fixed by ckeditor/ckeditor5-engine#907
Closed

Blockquote implementation #3905

szymonkups opened this issue Dec 1, 2016 · 7 comments · Fixed by ckeditor/ckeditor5-engine#907
Assignees
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@szymonkups
Copy link
Contributor

This is an initial issue describing Blockquote feature implementation.
There is no requirements list yet, but we already see that it will require some changes in the engine. Let's gather them here.

Add support for nested elements to DataController#modifySelection:

Extending selection in situation like this:

<blockquote><p>foo[]</p></blockquote><p>bar</p>

should return:

<blockquote><p>foo[</p></blockquote><p>]bar</p>
@szymonkups szymonkups changed the title Bloquote implementation Blockquote implementation Dec 1, 2016
@Reinmar
Copy link
Member

Reinmar commented Dec 1, 2016

I'm also curious how's deleteContent() dealing with such a cases. I remember that I didn't want to think too much on it back then so the implementation may be poor.

@Reinmar
Copy link
Member

Reinmar commented Dec 1, 2016

Besides, it's tricky to understand which rules are specific to blockquote and which are general and should land in the default algorithms.

@fredck
Copy link
Contributor

fredck commented Dec 1, 2016

Btw, be sure to check Editor Recommendations on Quotes.

@Reinmar
Copy link
Member

Reinmar commented Dec 1, 2016

Good point. Do we want to have a caption support? If so, then it's a far more complex feature than what we have in CKEditor 4. It may need to be implemented with a widget or some smart model operations. The latter would be a very interesting test for our data model, but I'm afraid that we don't have time for that now.

We'd need to start this with a UX research too, cause I'm not sure how should the caption be reachable when vs how to escape blockquote. Perhaps it's a simple rule that enter in an empty paragraph in a blockquote removes that paragraph and moves selection out of the quote and below the caption.

@Reinmar
Copy link
Member

Reinmar commented Dec 1, 2016

I talked with @fredck. We're going now with a CKEditor4 like implementation (but rendering to a <figure class="quote">). We'll rethink the captioning in the future.

@szymonkups
Copy link
Contributor Author

szymonkups commented Dec 5, 2016

When modifying selection in situation described in first comment will be implemented, it should also support objects. Widgets implemetation should also use it during delete and backspace handling:

<widget></widget><blockquote><p>[]</p></blockquote>
// backspace pressed
[<widget></widget>]
<blockquote><p>[]</p></blockquote><widget></widget>
// delete pressed
[<widget></widget>]

@Reinmar Reinmar self-assigned this Feb 19, 2017
Reinmar referenced this issue in ckeditor/ckeditor5-engine Feb 25, 2017
…positions where text can be placed. Covers most of #710 too.
@Reinmar
Copy link
Member

Reinmar commented Apr 4, 2017

I can see that modifySelection() is covered by describe( 'beyond element – skipping incorrect positions' ) and works as expected.

So the problem with https://github.com/ckeditor/ckeditor5-block-quote/issues/7 lays inside deleteContent().

Reinmar referenced this issue in ckeditor/ckeditor5-engine Apr 5, 2017
Other: Changed the behavior of `DataController.deleteContent()` in a case of nested elements to better match situations like using <kbd>Backspace</kbd> after a block quotation. Closes #710.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants