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

Commit 17e86f9

Browse files
author
Piotr Jasiun
authored
Merge pull request #477 from ckeditor/t/ckeditor5/1676
Fix: Fixed `View#render` collision when moving focus from a one editable to the other in multi-root editor. Closes ckeditor/ckeditor5#1676.
2 parents 5325ea8 + 5f7a35e commit 17e86f9

File tree

2 files changed

+50
-8
lines changed

2 files changed

+50
-8
lines changed

src/editableui/editableuiview.js

+16-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ export default class EditableUIView extends View {
122122
const editingView = this._editingView;
123123

124124
if ( editingView.isRenderingInProgress ) {
125-
editingView.once( 'change:isRenderingInProgress', () => update( this ) );
125+
updateAfterRender( this );
126126
} else {
127127
update( this );
128128
}
@@ -135,5 +135,20 @@ export default class EditableUIView extends View {
135135
writer.removeClass( view.isFocused ? 'ck-blurred' : 'ck-focused', viewRoot );
136136
} );
137137
}
138+
139+
// In a case of a multi-root editor, a callback will be attached more than once (one callback for each root).
140+
// While executing one callback the `isRenderingInProgress` observable is changing what causes executing another
141+
// callback and render is called inside the already pending render.
142+
// We need to be sure that callback is executed only when the value has changed from `true` to `false`.
143+
// See https://github.com/ckeditor/ckeditor5/issues/1676.
144+
function updateAfterRender( view ) {
145+
editingView.once( 'change:isRenderingInProgress', ( evt, name, value ) => {
146+
if ( !value ) {
147+
update( view );
148+
} else {
149+
updateAfterRender( view );
150+
}
151+
} );
152+
}
138153
}
139154
}

tests/editableui/editableuiview.js

+34-7
Original file line numberDiff line numberDiff line change
@@ -80,22 +80,49 @@ describe( 'EditableUIView', () => {
8080
} );
8181

8282
// https://github.com/ckeditor/ckeditor5/issues/1530.
83+
// https://github.com/ckeditor/ckeditor5/issues/1676.
8384
it( 'should work when update is handled during the rendering phase', () => {
85+
const secondEditingViewRoot = new ViewRootEditableElement( 'div' );
86+
const secondView = new EditableUIView( locale, editingView );
87+
const secondEditableElement = document.createElement( 'div' );
88+
89+
document.body.appendChild( secondEditableElement );
90+
91+
secondEditingViewRoot.rootName = 'second';
92+
secondEditingViewRoot._document = editingView.document;
93+
editingView.document.roots.add( secondEditingViewRoot );
94+
95+
secondView.name = 'second';
96+
secondView.render();
97+
98+
editingView.attachDomRoot( editableElement, 'main' );
99+
editingView.attachDomRoot( secondEditableElement, 'second' );
100+
84101
view.isFocused = true;
85-
editingView.isRenderingInProgress = true;
102+
secondView.isFocused = false;
86103

87-
expect( editingViewRoot.hasClass( 'ck-focused' ) ).to.be.true;
88-
expect( editingViewRoot.hasClass( 'ck-blurred' ) ).to.be.false;
104+
expect( editingViewRoot.hasClass( 'ck-focused' ), 1 ).to.be.true;
105+
expect( editingViewRoot.hasClass( 'ck-blurred' ), 2 ).to.be.false;
106+
expect( secondEditingViewRoot.hasClass( 'ck-focused' ), 3 ).to.be.false;
107+
expect( secondEditingViewRoot.hasClass( 'ck-blurred' ), 4 ).to.be.true;
89108

109+
editingView.isRenderingInProgress = true;
90110
view.isFocused = false;
111+
secondView.isFocused = true;
91112

92-
expect( editingViewRoot.hasClass( 'ck-focused' ) ).to.be.true;
93-
expect( editingViewRoot.hasClass( 'ck-blurred' ) ).to.be.false;
113+
expect( editingViewRoot.hasClass( 'ck-focused' ), 5 ).to.be.true;
114+
expect( editingViewRoot.hasClass( 'ck-blurred' ), 6 ).to.be.false;
115+
expect( secondEditingViewRoot.hasClass( 'ck-focused' ), 7 ).to.be.false;
116+
expect( secondEditingViewRoot.hasClass( 'ck-blurred' ), 8 ).to.be.true;
94117

95118
editingView.isRenderingInProgress = false;
96119

97-
expect( editingViewRoot.hasClass( 'ck-focused' ) ).to.be.false;
98-
expect( editingViewRoot.hasClass( 'ck-blurred' ) ).to.be.true;
120+
expect( editingViewRoot.hasClass( 'ck-focused' ), 9 ).to.be.false;
121+
expect( editingViewRoot.hasClass( 'ck-blurred' ), 10 ).to.be.true;
122+
expect( secondEditingViewRoot.hasClass( 'ck-focused' ), 11 ).to.be.true;
123+
expect( secondEditingViewRoot.hasClass( 'ck-blurred' ), 12 ).to.be.false;
124+
125+
secondEditableElement.remove();
99126
} );
100127
} );
101128
} );

0 commit comments

Comments
 (0)