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

Stop invoking view.render() by EditingController when the model document isn't changed #1657

Merged
merged 13 commits into from
Feb 4, 2019

Conversation

ma2ciek
Copy link
Contributor

@ma2ciek ma2ciek commented Jan 31, 2019

Suggested merge commit message (convention)

Fix: Stopped invoking view.render() by EditingController when the model document isn't changed. Closes ckeditor/ckeditor5#4473.


Additional information

  • Model#_change event marked as deprecated.
  • Introduced protected method: _handleChangeBlock().
  • Introduced protected method: _hasDocumentChangedFromTheLastChangeBlock().

@coveralls
Copy link

coveralls commented Jan 31, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 0cb7c9f on t/1653 into 2e04af7 on master.

* @protected
* @returns {Boolean} Returns `true` if document has changed from the differ's reset.
*/
_hasDocumentChanged() {
Copy link

Choose a reason for hiding this comment

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

Suggested change
_hasDocumentChanged() {
_hasDocumentChangedFromTheLastChangeBlock() {

* @fires change:data
* @param {module:engine/model/writer~Writer writer} writer The writer on which post-fixers will be called.
*/
_runPostFixersAndResetDiffer( writer ) {
Copy link

@pjasiun pjasiun Feb 1, 2019

Choose a reason for hiding this comment

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

Suggested change
_runPostFixersAndResetDiffer( writer ) {
_handleChangeBlock( writer ) {

Copy link

Choose a reason for hiding this comment

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

I do not like the current name. The most important thing this method does is firing document#change. However, _runPostFixersAndFireChangeAndResetDiffer sounds ridiculous, so I am for shorter _handleChangeBlock

// Collect an information whether the model document has changed during from the last pending change.
hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChanged();

// Fire '_change' event before resetting differ.
this.fire( '_change', this._currentWriter );
Copy link

Choose a reason for hiding this comment

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

It should be marked as deprecated.

Copy link
Contributor Author

@ma2ciek ma2ciek Feb 4, 2019

Choose a reason for hiding this comment

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

This event is marked as deprecated on the bottom of that file.

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