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

Commit

Permalink
Merge pull request #477 from ckeditor/t/ckeditor5/1676
Browse files Browse the repository at this point in the history
Fix: Fixed `View#render` collision when moving focus from a one editable to the other in multi-root editor. Closes ckeditor/ckeditor5#1676.
  • Loading branch information
Piotr Jasiun authored Apr 5, 2019
2 parents 5325ea8 + 5f7a35e commit 17e86f9
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 8 deletions.
17 changes: 16 additions & 1 deletion src/editableui/editableuiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export default class EditableUIView extends View {
const editingView = this._editingView;

if ( editingView.isRenderingInProgress ) {
editingView.once( 'change:isRenderingInProgress', () => update( this ) );
updateAfterRender( this );
} else {
update( this );
}
Expand All @@ -135,5 +135,20 @@ export default class EditableUIView extends View {
writer.removeClass( view.isFocused ? 'ck-blurred' : 'ck-focused', viewRoot );
} );
}

// In a case of a multi-root editor, a callback will be attached more than once (one callback for each root).
// While executing one callback the `isRenderingInProgress` observable is changing what causes executing another
// callback and render is called inside the already pending render.
// We need to be sure that callback is executed only when the value has changed from `true` to `false`.
// See https://github.com/ckeditor/ckeditor5/issues/1676.
function updateAfterRender( view ) {
editingView.once( 'change:isRenderingInProgress', ( evt, name, value ) => {
if ( !value ) {
update( view );
} else {
updateAfterRender( view );
}
} );
}
}
}
41 changes: 34 additions & 7 deletions tests/editableui/editableuiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,49 @@ describe( 'EditableUIView', () => {
} );

// https://github.com/ckeditor/ckeditor5/issues/1530.
// https://github.com/ckeditor/ckeditor5/issues/1676.
it( 'should work when update is handled during the rendering phase', () => {
const secondEditingViewRoot = new ViewRootEditableElement( 'div' );
const secondView = new EditableUIView( locale, editingView );
const secondEditableElement = document.createElement( 'div' );

document.body.appendChild( secondEditableElement );

secondEditingViewRoot.rootName = 'second';
secondEditingViewRoot._document = editingView.document;
editingView.document.roots.add( secondEditingViewRoot );

secondView.name = 'second';
secondView.render();

editingView.attachDomRoot( editableElement, 'main' );
editingView.attachDomRoot( secondEditableElement, 'second' );

view.isFocused = true;
editingView.isRenderingInProgress = true;
secondView.isFocused = false;

expect( editingViewRoot.hasClass( 'ck-focused' ) ).to.be.true;
expect( editingViewRoot.hasClass( 'ck-blurred' ) ).to.be.false;
expect( editingViewRoot.hasClass( 'ck-focused' ), 1 ).to.be.true;
expect( editingViewRoot.hasClass( 'ck-blurred' ), 2 ).to.be.false;
expect( secondEditingViewRoot.hasClass( 'ck-focused' ), 3 ).to.be.false;
expect( secondEditingViewRoot.hasClass( 'ck-blurred' ), 4 ).to.be.true;

editingView.isRenderingInProgress = true;
view.isFocused = false;
secondView.isFocused = true;

expect( editingViewRoot.hasClass( 'ck-focused' ) ).to.be.true;
expect( editingViewRoot.hasClass( 'ck-blurred' ) ).to.be.false;
expect( editingViewRoot.hasClass( 'ck-focused' ), 5 ).to.be.true;
expect( editingViewRoot.hasClass( 'ck-blurred' ), 6 ).to.be.false;
expect( secondEditingViewRoot.hasClass( 'ck-focused' ), 7 ).to.be.false;
expect( secondEditingViewRoot.hasClass( 'ck-blurred' ), 8 ).to.be.true;

editingView.isRenderingInProgress = false;

expect( editingViewRoot.hasClass( 'ck-focused' ) ).to.be.false;
expect( editingViewRoot.hasClass( 'ck-blurred' ) ).to.be.true;
expect( editingViewRoot.hasClass( 'ck-focused' ), 9 ).to.be.false;
expect( editingViewRoot.hasClass( 'ck-blurred' ), 10 ).to.be.true;
expect( secondEditingViewRoot.hasClass( 'ck-focused' ), 11 ).to.be.true;
expect( secondEditingViewRoot.hasClass( 'ck-blurred' ), 12 ).to.be.false;

secondEditableElement.remove();
} );
} );
} );
Expand Down

0 comments on commit 17e86f9

Please sign in to comment.