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.

Sign up for GitHub

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

Jump to bottom

Docs nested blockquotes fix #9472

Merged
merged 16 commits into from
Apr 20, 2021
Merged

Docs nested blockquotes fix #9472

merged 16 commits into from
Apr 20, 2021

Conversation

godai78
Copy link
Contributor

@godai78 godai78 commented Apr 14, 2021 •
edited
Loading

Suggested merge commit message (convention)

Docs (block-quote): nested block quotes functionality description.

@godai78 godai78 requested a review from AnnaTomanek April 14, 2021 12:39
@godai78 godai78 self-assigned this Apr 14, 2021
@jacekbogdanski
Copy link
Member

jacekbogdanski commented Apr 15, 2021 •
edited
Loading

This guide is using real examples for blockquote - nesting is now a feature, so maybe it makes sense to remove or update this sentence?

https://ckeditor.com/docs/ckeditor5/latest/framework/guides/deep-dive/schema.html#generic-items

The side effect of such a definition inheritance is that now <blockQuote> is allowed in <blockQuote> which needs to be resolved by a callback which will disallow this specific structure.

@AnnaTomanek
Copy link
Contributor

This guide is using real examples for blockquote - nesting is now a feature, so maybe it makes sense to remove or update this sentence?

Thanks @jacekbogdanski - can we just remove this sentence? I suppose the callback that disallows this structure is already removed, right?

@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/features/block-quote.md
d546d55 
Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@jacekbogdanski
Copy link
Member

From what I see the check for nested blockquote was removed in https://github.com/ckeditor/ckeditor5/pull/9382/files so I suppose we can just get rid of it 👍🏻

@godai78
Copy link
Contributor Author

godai78 commented Apr 15, 2021

I suppose the callback that disallows this structure is already removed, right?

I believe this is exactly what was removed by @psmyrek from the code, if I read it right. We now need to specifically add code to prevent this behaviour, as described in the Block Quote docs.

@AnnaTomanek
Copy link
Contributor

AnnaTomanek commented Apr 15, 2021 •
edited
Loading

Thanks Jacek! @godai78 can you please remove it as part of this PR. Also added milestone, estimate and labels since it makes no sense to create an issue for this all.

@AnnaTomanek AnnaTomanek added this to the iteration 42 milestone Apr 15, 2021
@AnnaTomanek AnnaTomanek added squad:ccx Issue to be handled by the CCX team. type:docs This issue reports a task related to documentation (e.g. an idea for a guide). labels Apr 15, 2021
AnnaTomanek
AnnaTomanek reviewed Apr 20, 2021
View reviewed changes
packages/ckeditor5-block-quote/docs/features/block-quote.md Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/features/block-quote.md Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/_snippets/features/nested-block-quote.html Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/_snippets/features/nested-block-quote.html Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/_snippets/features/nested-block-quote.html Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/_snippets/features/nested-block-quote.html Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/_snippets/features/nested-block-quote.html Outdated Show resolved Hide resolved
packages/ckeditor5-block-quote/docs/_snippets/features/nested-block-quote.html Outdated Show resolved Hide resolved
godai78 and others added 8 commits April 20, 2021 09:57
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/features/block-quote.md
f5dc2fd 
Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/features/block-quote.md
9bb898c 
Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
3f11b98 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
3e8e468 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
de3879c 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
0a0197e 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
2771fc8 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
b46d5bf 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
godai78 and others added 2 commits April 20, 2021 12:35
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
bcd1176 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@godai78 @AnnaTomanek
Update packages/ckeditor5-block-quote/docs/_snippets/features/nested-…
239296b 
…block-quote.html

Co-authored-by: Anna Tomanek <a.tomanek@cksource.com>
@AnnaTomanek AnnaTomanek merged commit 4fd3885 into release Apr 20, 2021
@AnnaTomanek AnnaTomanek deleted the docs-nested-blockquotes-fix branch April 20, 2021 10:41
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 6, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
2adc96a 
1)

Move conversion for paragraph to <p style="margin:0;"></p> to plugin

2) Drop schema listener for nested <blockQuote>.

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 6, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
9a61b94 
1.

Move conversion for paragraph to <p style="margin:0;"></p> to plugin

2.

Drop schema listener for nested <blockQuote>.

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 6, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
324255d 
1. Move conversion for paragraph to `<p style="margin:0;"></p>` to plugin

2. Drop schema listener for nested `<blockQuote>`.

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 6, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
d744e88 
Move conversion for paragraph to `<p style="margin:0;"></p>` to plugin

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 6, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
8c3a4ee 
**Add mail plugin**

Move conversion for paragraph to `<p style="margin:0;"></p>` to plugin

**Drop schema listener for `<blockQuote>`**

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb mentioned this pull request Sep 6, 2022
Merged
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 6, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
d02a06e 
**Add mail plugin**

Move conversion for paragraph to `<p style="margin:0;"></p>` to plugin

**Drop schema listener for `<blockQuote>`**

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 7, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
75afe07 
**Add mail plugin**

Move conversion for paragraph to `<p style="margin:0;"></p>` to plugin

**Drop schema listener for `<blockQuote>`**

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit to nextcloud/mail that referenced this pull request Sep 20, 2022
@kesselb
GH-7141: Add ckeditor plugin for mail
64d29b0 
**Add mail plugin**

Move conversion for paragraph to `<p style="margin:0;"></p>` to plugin

**Drop schema listener for `<blockQuote>`**

Older versions of ckeditor did not support nested blockquotes[^1].
The schema listener is a workaround to keep nested blockquotes[^2].
CKEditor 28 added support for nested blockquotes back[^3][^4].

[^1]: ckeditor/ckeditor5#419 (comment)
[^2]: ckeditor/ckeditor5#419 (comment)
[^3]: ckeditor/ckeditor5#9382
[^4]: ckeditor/ckeditor5#9472

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnnaTomanek AnnaTomanek AnnaTomanek left review comments

Assignees

@godai78 godai78

Labels
squad:ccx Issue to be handled by the CCX team. type:docs This issue reports a task related to documentation (e.g. an idea for a guide).
Projects
None yet
Milestone
iteration 42 (v27.1.0)
Development

Successfully merging this pull request may close these issues.
3 participants
@godai78 @jacekbogdanski @AnnaTomanek