Skip to content

Conversation

@tbelcheva
Copy link
Contributor

Closes #187
Closes #146

Copy link
Contributor

@MayaKirova MayaKirova left a comment

Choose a reason for hiding this comment

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

This change should also be applied to the igGridComponent and for other components that have two-way databinding (igCombo, igTree).

@MayaKirova
Copy link
Contributor

Two-way databinding tests are failing. Probably because the return condition is wrong.

if (typeof this._config.dataSource === "string" || this._config.dataSource instanceof Array) {
return; 
}

We should return if the data source is not array. If it is the two-way databinding logic should execute as before.
Example:

if( !(this._config.dataSource instanceof Array) ) {
    return;
 }
this._changes = this._differ.diff(this._config.dataSource);

MayaKirova
MayaKirova previously approved these changes Dec 4, 2017
@kdinev
Copy link
Member

kdinev commented Dec 4, 2017

@tbelcheva Can you include some tests for this scenario?

@kdinev
Copy link
Member

kdinev commented Dec 12, 2017

@tbelcheva bower_components/ should be added to the .gitignore

@kdinev kdinev dismissed MayaKirova’s stale review December 12, 2017 09:33

bower_components is committed.

@mpavlinov
Copy link
Contributor

@tbelcheva Please use https://www.npmjs.com/package/jquery-mockjax instead of bower

public opts2: any;
public opts3: any;
public optsNew:any;
public opts4: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

@tbelcheva Where do you configure opts4?

})
class TestComponent {
private opts: any;
private opts2: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

@tbelcheva Where do you configure opts2?

@mpavlinov mpavlinov merged commit 15cc62a into IgniteUI:master Dec 28, 2017
@tbelcheva tbelcheva deleted the fix-issue-187 branch January 18, 2018 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants