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

View refactoring #1296

Merged
merged 103 commits into from
Feb 16, 2018
Merged

View refactoring #1296

merged 103 commits into from
Feb 16, 2018

Conversation

szymonkups
Copy link
Contributor

@szymonkups szymonkups commented Feb 12, 2018

Suggested merge commit message (convention)

Other: Refactoring of the view API. Closes ckeditor/ckeditor5#4222.

BREAKING CHANGE: view.Writer is no longer an object literal with functions but a class.
BREAKING CHANGE: View controller view.View is introduced. Changes to the view document tree structure should be done by using writer provided to callback in view.change() method.
BREAKING CHANGE: View document is now separated from the DOM. view.Renderer, view.DomConverter and observers are moved to view.View.
BREAKING CHANGE: view#event:render is introduced to indicate a moment when all changes are applied and document may be rendered to the DOM.
BREAKING CHANGE: Downcast converter helpers no longer accepts view elements instances as constructors are now protected. Callbacks using view writer should be used.


Additional information

Main CKEditor 5 repo contains ckeditor5-engine/1210 branch which have modified mgit.json with other changed repositories on correct branches.

…riter is passed as parameter to conversion methods.
src/view/view.js Outdated

this._ongoingChange = false;

this.document.fire( 'change' );
Copy link

@pjasiun pjasiun Feb 14, 2018

Choose a reason for hiding this comment

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

As we talk, I am not sure about the order here. change event is fired after the rendering, what can be a surprise for someone. I think that rendering should be the result of the change event (listener with the low priority). Also both change and render should be inside _ongoingChange block, so there will be no infinite loop if one adds some more changes. Also if view.change block is open after the rendering, in the change event, an exception should be thrown.

@pjasiun pjasiun merged commit dd9ae51 into master Feb 16, 2018
@pjasiun pjasiun deleted the t/1210 branch February 16, 2018 16:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update view API after changes in the model
4 participants