Prevent rendering when one is in the change block.
#1535
Conversation
src/controller/editingcontroller.js
Outdated
| @@ -70,6 +70,19 @@ export default class EditingController { | |||
| const selection = doc.selection; | |||
| const markers = this.model.markers; | |||
|
|
|||
| // Various plugins listen on model changes (on selection change, post fixers, etc.) and change the view as a | |||
There was a problem hiding this comment.
When plugins listen on model changes (on selection change, post fixers, etc) and change the view as a result of model's change, they might trigger view rendering before the conversion is completed (e.g. before the selection is converted). We disable rendering for the length of the outermost model change() block to prevent that.
See https://github.com/ckeditor/ckeditor5-engine/issues/1528
src/model/model.js
Outdated
| @@ -487,6 +491,22 @@ export default class Model { | |||
| * @param {module:engine/model/writer~Writer} writer `Writer` instance that has been used in the change block. | |||
| */ | |||
|
|
|||
| /** | |||
| * Fired when entering the first {@link module:engine/model/model~Model#enqueueChange} or | |||
| * {@link module:engine/model/model~Model#change} block of the pending changes. | |||
There was a problem hiding this comment.
That "pending changes" thing is misleading. It sounds like this is the Nth+ block. A one that was scheduled but needed to be postponed so it's "pending". Isn't it just the "outermost enqueueChange/change block"?
src/model/model.js
Outdated
|
|
||
| /** | ||
| * Fired when leaving the last {@link module:engine/model/model~Model#enqueueChange} or | ||
| * {@link module:engine/model/model~Model#change} block of the pending changes. |
src/view/view.js
Outdated
| * @protected | ||
| * @member {Boolean} module:engine/view/view~View#_disabledRendering | ||
| */ | ||
| this._disabledRendering = false; |
There was a problem hiding this comment.
_renderingDisabled would be more grammatical. Plus, you have _renderingInProgress above already.
| @@ -339,6 +339,112 @@ describe( 'EditingController', () => { | |||
| expect( getViewData( editing.view, { withoutSelection: true } ) ) | |||
| .to.equal( '<p></p><p>f<span>oo</span></p><p>bar</p>' ); | |||
| } ); | |||
|
|
|||
| describe( 'rendering preventing in the change block', () => { | |||
There was a problem hiding this comment.
preventing rendering in the change block
There was a problem hiding this comment.
or even:
preventing rendering while in the model.change() block
| editing.view.on( 'render', renderSpy ); | ||
| } ); | ||
|
|
||
| it( 'should not call render in the change block', () => { |
| expect( renderSpy.called ).to.be.true; | ||
| } ); | ||
|
|
||
| it( 'should not call render in the change block even if view change was called', () => { |
|
Except for some wording and naming, this LGTM. |
|
Tests look good so I'm merging it. |
Suggested merge commit message (convention)
Internal: Prevent rendering when one is in the
changeblock. Closes ckeditor/ckeditor5#4412.