-
Notifications
You must be signed in to change notification settings - Fork 40
Unit test covering edge case related to 'withChildren' parameter inside Renderer added #1455
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1683,6 +1683,36 @@ describe( 'Renderer', () => { | |
); | ||
} ); | ||
|
||
// #1451 | ||
it( 'should correctly render changed children even if direct parent is not marked to sync', () => { | ||
const inputView = | ||
'<container:ul>' + | ||
'<container:li>Foo</container:li>' + | ||
'<container:li><attribute:b>Bar</attribute:b></container:li>' + | ||
'</container:ul>'; | ||
|
||
const view = parse( inputView ); | ||
|
||
viewRoot._appendChild( view ); | ||
|
||
renderer.markToSync( 'children', viewRoot ); | ||
renderer.render(); | ||
|
||
expect( domRoot.innerHTML ).to.equal( '<ul><li>Foo</li><li><b>Bar</b></li></ul>' ); | ||
|
||
const viewLi = view.getChild( 0 ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uh, |
||
const viewLiIndented = view._removeChildren( 1, 1 ); // Array with one element. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what this line does. Which |
||
viewLiIndented[ 0 ]._appendChild( parse( '<attribute:i>Baz</attribute:i>' ) ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the problem here that we use I have a bad feeling about this – should renderer handle such cases? Shouldn't it be able to assume that cc @pjasiun There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but it is |
||
const viewUl = new ViewContainerElement( 'ul', null, viewLiIndented ); | ||
viewLi._appendChild( viewUl ); | ||
|
||
renderer.markToSync( 'children', view ); | ||
renderer.markToSync( 'children', viewLi ); | ||
renderer.render(); | ||
|
||
expect( domRoot.innerHTML ).to.equal( '<ul><li>Foo<ul><li><b>Bar</b><i>Baz</i></li></ul></li></ul>' ); | ||
} ); | ||
|
||
describe( 'fake selection', () => { | ||
beforeEach( () => { | ||
const { view: viewP, selection: newSelection } = parse( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is a bit too hard to read. Better variable naming + a comment what's happening inside it would help.