fix(select): Added trackBy property to allow selecting the object by …

[zakton5]

Short description of what this resolves:

ion-select currently cannot select the correct value if:

1. The values are objects
2. The ngModel binding is tied to a completely separate object, even it it has the exact same values.

This is due the ion-select only wanting to select objects based on object equality.

Changes proposed in this pull request:

• Add 'trackBy' property that represents the property within the object that will be considered, as opposed to the object itself.

I'm open to other approaches. I've also considered just looking at each and every property within the object, but that seems expensive, especially if the object has many/nested properties

Usage:

<ion-select [(ngModel)]="someObject" trackBy="idProperty"></ion-select>

Ionic Version: 2.x

Fixes: #6625

[adamdbradley]

Would you also be able to provide some unit tests to go along with this change? Thanks

[zakton5]

I'm having a lot of trouble creating unit tests for the select. I keep running into errors saying that there is no Provider for Form (Form is an interface, as well as a dependency that is supposed to be injected into the select constructor). Could you provide me some examples of unit tests for the ion-select?

[xr0master]

I think that you code is wrong. I haven't this issue.

First: The values are objects works fine, but you can't use it in future, because an object pointer changes all time after initialization. So It hasn't the exact same values! Therefore you must added identification for your object.

Second: Angular 2 trackBy requires a method and not a field.
My example:

<ion-select [(ngModel)]="chosenCurrency">
    <ion-option *ngFor="let currency of currencies; trackBy: trackByCurrencyCode" [value]="currency.code">{{currency.name}}</ion-option>
</ion-select>

public trackByCurrencyCode(index: number, item: any): string {
  return item.code; // must be unique
}

[zakton5]

The difference between your code and mine is this:

<ion-select [(ngModel)]="chosenCurrency">
    <ion-option *ngFor="let currency of currencies" [value]="currency">{{currency.name}}</ion-option>
</ion-select>

The [value] of each ion-option is the actual object (currency), not a property in the object (currency.code).

[xr0master]

Of course! My code works, your doesn't :) You think, that it's Ionic issue, and I try to explain your, that your issue is not understanding how the javascript works. How Angular must compare your model with your value? A deep compare is very expansive. Angular uses hash pointer compare. But your object hash pointer changes all time. So where a bug? Therefore you could use key/id of object.

Do you understand?

[jgw96]

Hello @xr0master, would you mind keeping our Code of Conduct in mind when interacting with others? Thanks!

[zakton5]

I understand how the Javascript works and why my code doesn't work. But for my specific needs, I cannot use a key/id as the value, and I require the value to be the object itself. Currently, that is not supported, unless I am missing something. My PR simply provides that functionality.

[xr0master]

@jgw96 you are right, I don't see more meaning to try to teach something others, it's not worth it. I only hope that the library does not turn into the trash of useless code.
Sorry and have a nice day.

[zakton5]

@xr0master I would like to understand. If I am truly wrong, please show me. This is the codepen that shows the problem. If you can make it work the way I am talking about, please show me.

http://codepen.io/anon/pen/LNweqo

[zakton5]

I feel like this needs some attention. I still have the same problem as before. If anyone feels that this is not a good fix, then how do I accomplish what I would like to do?

[cleever]

@xr0master your use case seems to not be the same use case that @zakton5 have.

He doesn't want to store the currency.code in chosenCurrency variable, but the entire currency object.

This why your code works and his code don't. Different meaning and different purpose.

Angular2 supports this feature in <select>element since beta14.

https://github.com/angular/angular/blob/master/CHANGELOG.md#200-beta14-2016-04-07

Discussions about this here:
angular/angular#4843

And here:
angular/angular#7842

This type of feature is crucial when your are working on cascading select boxes and when you have model oriented forms.

<ion-select> must support this to replace the original <select> directive.

[xr0master]

@cleever I didn't say that his code is wrong, I only think that the place of code is wrong. I think, code that we can very easy implement in our controller, must be out of common library. I, you and he need the feature, but another 100 people no need it. However each added feature increases a weight of library and also decreases a performance. IMHO

Example:

<ion-select [(ngModel)]="chosenCurrency" (change)="changedCurrency()">
    <ion-option *ngFor="let currency of currencies" [value]="currency.id">{{currency.name}}</ion-option>
</ion-select>

class MyController {

  private currencyObject: any;

  public chosenCurrency: string;
  public currencies: Array<any>;

  controller() {}

  public changedCurrency(): void {
    this.currencyObject = this.currencies[this.chosenCurrency];
  }
}

It's easy, isn't it?

PS I'm sorry about my terrible English.

[zakton5]

@cleever, angular does support this but not for my specific use case. I am pulling my pre-selected objects from a database. Therefore, the list of objects in my component, and the actual object used for the model value is a completely different object, even though it has the exact same properties.

I ended up doing this, which is similar to @xr0master's solution.

<ion-select [ngModel]="someProperty.id" (ngModelChange)="selectObjectById(list, $event, 'someProperty')">
    <ion-option *ngFor="let item of list" [value]="item.id">{{item.name}}</ion-option>
</ion-select>

public selectObjectById(list: any[], id: string, property: string) {
    var item = list.find(item => item._id === id);
    var prop = eval('this.' + property);
    prop = property;
}

This is essentially what the pull request did in the first place. It's unfortunate that this is necessary, but it works as desired.