7336: ConversionAPI update #7652

jodator · 2020-07-20T12:52:37Z

Suggested merge commit message (convention)

Feature (engine): Introduced new upcast ConversionApi helper methods - conversionApi.safeInsert() and conversionApi.updateConversionResult(). New methods are intended to simplify writing event based element-to-element converters. Closes #7336.

MAJOR BREAKING CHANGE (engine): The config.view parameter for upcast element-to-element conversion helpers configurations is again mandatory. You can retain previous "catch-all" behavior for upcast converter using config.view = /[\s\S]+/.

Additional information

In addition to changes described previously under the ticket (#7336 (comment)):

I've updated list converters a bit (dd9481f) by using some of the API - however, I had to add a conditional statement to not update the modelRange if it was defined already (we might skip that change).
~~tests are still to be written (as the previous point might change implementation).~~ done
I've updated the API docs
the getSplitParts() and splitToAllowedParent() are not used anymore - might be dropped.

TODO:

the getSplitParts() and splitToAllowedParent() are not used anymore - might be dropped - let's keep it for a while.
decide if special handling of modelRange is to be preserved (I couldn't find another way for that).
check if the consuming part shouldn't be simplified - a follow-up but not in this PR.

…rts.

…of the `conversionApi.convertChildren()`.

…sion steps.

…methods.

# Conflicts: # packages/ckeditor5-engine/src/conversion/upcasthelpers.js

jodator · 2020-07-29T07:58:38Z

packages/ckeditor5-list/src/converters.js

-			// Otherwise continue conversion after the last list item.
-			data.modelCursor = data.modelRange.end;
-		}
+		conversionApi.updateConversionResult( listItem, data );


Check if modelCursor can be cached before.

I couldn't make it work without full copy-pate of the updateConversionResult() so 🤷‍♂️ dunno we might live with that if() inside it.

jodator · 2020-07-31T05:30:08Z

@niegowski Please make a review for this PR. We've looked at this with @Reinmar and the whole idea and change looks OK.

A conversation on a solution starts here: #7336 (comment).

packages/ckeditor5-paragraph/src/paragraph.js

jodator · 2020-07-31T05:52:42Z

@Reinmar I've updated the conversion deep-dive guide if you wish to check it - if not please just mark this PR as ✔️ to not block @niegowski with eventual merge.

packages/ckeditor5-engine/src/conversion/upcastdispatcher.js

packages/ckeditor5-engine/src/conversion/upcasthelpers.js

...ages/ckeditor5-engine/docs/_snippets/framework/extending-content-custom-element-converter.js

packages/ckeditor5-engine/tests/conversion/upcasthelpers.js

packages/ckeditor5-table/tests/converters/upcasttable.js

… docs.

jodator · 2020-08-04T08:06:07Z

@Reinmar as @niegowski is off today:

I've fixed found issues.
I've added UpcastConversionData typedef: 2e7c739.

So IMO it is good to merge.

Reinmar · 2020-08-04T15:28:19Z

I scanned all the code and I now work on the guide. I want to extend it a bit with some more context. But I may be stuck for a while on how to incorporate it into the existing guides. If you'll see that it may cause problems with maintaining this branch if it takes me 1-2 days, I can merge it as it is.

jodator · 2020-08-05T06:45:33Z

I scanned all the code and I now work on the guide. I want to extend it a bit with some more context

I'd go with a follow-up as it is only a guide update.

Reinmar · 2020-08-05T08:23:11Z

I pushed guide improvements. Could you scan them, @jodator?

Reinmar · 2020-08-05T08:51:21Z

OK, that's all for my review. I pushed some more changes to the API docs.

Please take a look at them and if I didn't make some bigger mistakes, you can merge this PR.

packages/ckeditor5-engine/src/conversion/upcastdispatcher.js

Co-authored-by: Kuba Niegowski <1232187+niegowski@users.noreply.github.com>

jodator · 2020-08-05T09:12:11Z

@Reinmar - nice update :) and @niegowski good catch with that typo. As from me the PR is OK now, so you can merge.

jodator added 21 commits June 25, 2020 14:47

Refactor upcast helpers to use conversionApi object instead of impo…

f3430d9

…rts.

Other(engine): Allow model element to be passed as a second argument …

3b58289

…of the `conversionApi.convertChildren()`.

Enforce config.view for converters.

da02f5a

Intermediate elementToElement refactor - try to extract common conver…

066ffb5

…sion steps.

Always required config.view for converters.

89eac12

Refactor prepareToElementConverter() and introduce new ConversionApi …

edeb269

…methods.

Use new upcast conversion API for code block.

4b71659

Use new upcast conversion API for title.

0e8bbca

Use new upcast conversion API for view figure.

ad8e93a

Simplify table cell upcast converter.

e53eefe

Use new upcast conversion API for table.

b8ba0aa

Fix empty cell tests.

1aea458

Merge branch 'master' into i/7336

5934eda

Stub the custom element guide.

523336a

Update docs.

7458aee

Use new upcast conversion API in list upcast.

aa6f0ba

Use safeInsert in list editing tests.

dd9481f

Update upcast helpers API docs.

cac2d76

Add tests for conversionApi.safeInsert().

23ee118

Add tests for conversionApi.updateConversionResult().

9af436c

Merge branch 'master' into i/7336

30d1c25

# Conflicts: # packages/ckeditor5-engine/src/conversion/upcasthelpers.js

jodator marked this pull request as ready for review July 21, 2020 11:36

jodator requested a review from Reinmar July 21, 2020 11:36

jodator commented Jul 29, 2020

View reviewed changes

jodator added 6 commits July 29, 2020 14:01

Update custom element converter guide.

618501c

Update links in the docs.

7251a89

Improve snippets readability.

cfe287f

Improve snippets readability & update the snippet code to match guide.

fa9520b

Last docs fix.

b0089ed

Merge branch 'master' into i/7336

bee22e2

jodator added 2 commits July 31, 2020 07:23

Update internal docs.

c34abab

Merge branch 'master' into i/7336

4b39735

jodator requested a review from niegowski July 31, 2020 05:29

jodator commented Jul 31, 2020

View reviewed changes

packages/ckeditor5-paragraph/src/paragraph.js Outdated Show resolved Hide resolved

niegowski reviewed Aug 3, 2020

View reviewed changes

jodator added 3 commits August 4, 2020 09:31

Introduce UpcastConversionData for element-to-element conversion. Fix…

2e7c739

… docs.

Remove redundant code.

375f4ae

Use match any character RegExp to match elements for auto-paragraphing.

8d34add

oskarwrobel mentioned this pull request Aug 4, 2020

Providing additional options to the DowncastHelpers config.view callback #7774

Closed

Merge branch 'master' into i/7336

a083c31

jodator mentioned this pull request Aug 5, 2020

Update the element-to-element conversion guide #7777

Closed

Guide improvements.

225bc44

Renamed the guide and more API docs improvements.

cfde080

Reinmar approved these changes Aug 5, 2020

View reviewed changes

Reinmar mentioned this pull request Aug 5, 2020

Better styles for the custom element conversion demo #7781

Closed

niegowski reviewed Aug 5, 2020

View reviewed changes

packages/ckeditor5-engine/src/conversion/upcastdispatcher.js Outdated Show resolved Hide resolved

Update packages/ckeditor5-engine/src/conversion/upcastdispatcher.js

1f0b6db

Co-authored-by: Kuba Niegowski <1232187+niegowski@users.noreply.github.com>

jodator closed this Aug 5, 2020

jodator reopened this Aug 5, 2020

niegowski merged commit 8d84af1 into master Aug 5, 2020

niegowski deleted the i/7336 branch August 5, 2020 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7336: ConversionAPI update #7652

7336: ConversionAPI update #7652

jodator commented Jul 20, 2020 •

edited by Reinmar

Loading

jodator Jul 29, 2020 •

edited

Loading

jodator Jul 31, 2020

jodator commented Jul 31, 2020

jodator commented Jul 31, 2020

jodator commented Aug 4, 2020

Reinmar commented Aug 4, 2020

jodator commented Aug 5, 2020

Reinmar commented Aug 5, 2020

Reinmar commented Aug 5, 2020

jodator commented Aug 5, 2020

7336: ConversionAPI update #7652

7336: ConversionAPI update #7652

Conversation

jodator commented Jul 20, 2020 • edited by Reinmar Loading

Suggested merge commit message (convention)

Additional information

jodator Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

jodator Jul 31, 2020

Choose a reason for hiding this comment

jodator commented Jul 31, 2020

jodator commented Jul 31, 2020

jodator commented Aug 4, 2020

Reinmar commented Aug 4, 2020

jodator commented Aug 5, 2020

Reinmar commented Aug 5, 2020

Reinmar commented Aug 5, 2020

jodator commented Aug 5, 2020

jodator commented Jul 20, 2020 •

edited by Reinmar

Loading

jodator Jul 29, 2020 •

edited

Loading