Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Upcast writing and View enhancements #1579

Merged
merged 15 commits into from
Nov 8, 2018
Merged

Upcast writing and View enhancements #1579

merged 15 commits into from
Nov 8, 2018

Conversation

f1ames
Copy link
Contributor

@f1ames f1ames commented Oct 16, 2018

Suggested merge commit message (convention)

Other: Introduced createDocumentFragment(), createElement() and createText() methods in UpcastWriter. The View.change() method now returns result of its callback. Closes ckeditor/ckeditor5#4423.


Additional information

See ckeditor/ckeditor5#4423 for details.

@f1ames
Copy link
Contributor Author

f1ames commented Oct 16, 2018

There is related PR (change in docs) in clipboard plugin - ckeditor/ckeditor5-clipboard#59.

@pjasiun pjasiun requested a review from Reinmar October 23, 2018 12:12
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/upcastwriter.js Outdated Show resolved Hide resolved
src/view/view.js Outdated Show resolved Hide resolved
src/view/element.js Outdated Show resolved Hide resolved
Copy link
Member

@Reinmar Reinmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in comments.

@Reinmar
Copy link
Member

Reinmar commented Oct 31, 2018

A control question – did you check for new Element/DF/Text() use in at least API docs? I'm sure there's a lot of that in tests, but that we can leave (it's also correct cause constructors are protected, not private).

f1ames and others added 9 commits November 7, 2018 11:19
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
Co-Authored-By: f1ames <dr.odpowiedz@gmail.com>
@coveralls
Copy link

coveralls commented Nov 7, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling b0aa93b on t/1549 into 18bab70 on master.

@f1ames
Copy link
Contributor Author

f1ames commented Nov 7, 2018

When it comes to b0aa93b, I have changed all API samples which used protected constructors (e.g. new Element()) to use DowncastWriter.

It could be also changed to use UpcastWriter, however as UpcastWriter doesn't provide methods to create specialized elements (like downcastWriter.createAttributeElement()) some samples would require using both downcast and upcast writer which may look confusing.

@f1ames f1ames requested a review from Reinmar November 7, 2018 15:48
@Reinmar Reinmar merged commit ec13c85 into master Nov 8, 2018
@Reinmar Reinmar deleted the t/1549 branch November 8, 2018 11:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants