Add possibility to find value according to a property of the model - #256

Nice job, works well, thanks! Fixed my pain of using unsupported "track by".

Can you explain me the use case for this? Maybe we already can do if with current implementation

The problem was that I fetched from server instances of choices and then instances of the selected items. These instances were different, so it did not match anything. By this fix instances can be now tracked by 'id'. Maybe this is already supported, but it did not work for me before this fix.

Before: http://plnkr.co/edit/abv6H0yJFw7G841taUgo?p=preview
After: http://plnkr.co/edit/fk9wTOVR6MgOI2VcU5oU?p=preview

Exactly, the problem I had was the same: I get instance from server, those instances are ng-resource and that's why it didn't match anything in my choice list. Therefore track by can be useful in that way.

ITman1: Where is the difference ? I can't find where code is changed

Only in the initialization of the selection, so look for $scope.multipleDemo.selectedPeople. In the first example there are identical instances. In the second example instances are new, but selected items are semantically the same.

@JeffGreat sorry the delay on the review, can you create a simple test for this feature?

Sorry dimirc I didn't see you comment. I'll try to make test quite soon.

dimirc, I've just change the test spec file but since it became quite old I wonder if I can send you a pull request. Let me know.

Arf, build failed

@JeffGreat - I love the fix to the object comparison issue. If you can do a few things for me I'll merge it up:

• Re-base with latest.
• Would love to see another object in the list to make the test a bit more complex and show it doesn't match the second.
• Add the condition for the single value. The fact that it works now is just a pure fluke, if anyone changes that logic it will fail too. Honestly, I'd love to change this logic to be a little more clear.

@dimirc - This is a valid problem; it happens when you have prototype objects that ARE the same but the comparison == doesn't return true. The track by expression allows you specify the 'key' to match on.

Its also worth noting if you did angular.equals it would work since it does a deep compare. Its not performant at all though so a track by solution would probably still be a better fix.

This is very interesting. If we can merge, I believe it would allow us to close #333, #414, #332.

Ok i'll try to update the fix asap since I'm a bit busy. Hope to post something in a few days (during the holidays :) )

@amcdnl - I've rebased with latest and pushed it on my fork.
What do you mean by "Add the condition for the single value"?

Hey guys - any movement on this? I'm surprised this didn't work from the start. As such, I am developing with the component and am stuck until the fix is merged or I hack up my own solution.

Just curious if it's planned to be merged soon.

@amcdnl can you and @JeffGreat sort out your request here?

I've updated the pull request, all the test passed so it just need to be merged now. @dimirc can you merge?

@JeffGreat - I can merge it but it sounded like you and @amcdnl still hadn't resolved one of his bullet points.

👍

@brianfeister you can merge since the last bullet point seems to be an improvment. On my side the feature work as is and can help others.
when @dimirc will explain me what is the last bullet I'll do another pull request.

👍 Yes, please merge this ASAP, I've just wasted a good amount of time just to figure out this is not working with ui-select. 😞

@JeffGreat Just apply the track by change for single selections. Currently you only have it for multi

Merged ... I'll fix the other issue.

I added docs too: https://github.com/angular-ui/ui-select/wiki/ui-select#multi-select-with-complex-objects

@JeffGreat Thank you so much for fixing this! I was going insane thinking I was doing something wrong. Exact same situation, I have a list of possibilities loaded from the server, and the selections are pre-populated on a different object, so they weren't matching. This solves the problem I've spent the last 3 hours on.

By the way, the merged PR is not updated in the dist/ folder... I had to npm install && gulp to get this working. So make sure you run gulp to rebuild the dist/select.js file

Can a tag be created that contains this update? 0.9.6 dosn't seem to have it

I cannot make this work in my use case using multiselect, options are fetched async with refresh="fetch($select.search) from HTTP backend

The problem is that selecting works fine and I can save to backend, but when backend returns results, UI is not updated accordingly.

My fetch() does not return all the options with empty search parameter but only limited list where previously selected value is or isn't included. The matching logic in this PR does not help since empty fetch is performed in some point and matching is performed against options where the selected value does not exist.

So my fetch() is really dynamic and the resulting options depend on given search parameter. I can easily fix this so that fetch() with empty search term would return all the possible options, but this does not make sense in performance point of view. To be more specific, my options filtering is done on server-side.

Single-select (without multiple) works just fine and as expected in this case so I don't know why multiple should be any different.

Any hints for this scenario?

@jukkasi I don't think this fix has been released yet, it is in master but it dosn't seem to be in the 0.9.6 build that is the latest on bower

@amcdnl is this correct? can we get a build done?

@ir1sh I understand it's not released and tested as local patch. I believe the problems with multiple are much bigger than this PR adresses.

I agree for one I don't understand why the object has to be in the choices object to be added to the selected

edit1: My use case is this;

-Users can add guests to a reservation. these guests can be from a list of user objects that are relatively complicated
-Users can also add custom guests in the form of an email address
This all works fine so far. The problem is when the server returns the list of guests it includes both the user objects and the email address objects. I add the track by property and the guests that are in the User list get pre populated but no matter what I do to I cant get it to prepopulate the email address only objects

edit2: this is with this pull request included

Exactly. And other major issue reported elsewhere by many is that setting the model does not update view accordingly, all of this is more or less related. This PR is referenced from many issue reports but actually I don't think it helps for any of those..

@JeffGreat if I change the track by to person.email instead of person.name in your test it still passes. should it fail?

it should fail. Actually since the email is different there is no reason to get a selected value. May be the assertion is not right.

even if I remove the track by in el it still passes

works correctly in demo-multi-select.html

@dimirc @amcdnl : to @ir1sh 's point above, would it be possible to get a 0.9.7 tag with this fix? It seems like there is sufficient desire from users to take advantage of "track by" support enabled by this.

hi guys ? how to get this patch or tag preloading working ? spent 4 hours yesterday on trying many workarounds and i'm about going to select2

@amcdnl - hoping to get a new release with some bugfixes out soon. Should I assume those are 0.9.7 and this is 0.10.0?

i have tried 0.9.7 - still can't preload values if they are not in ui-select-choices http://plnkr.co/edit/o0pAujq9u4gCq2X96WEZ?p=preview

Next release. 0.9.8

Sent from my iPhone

On Feb 14, 2015, at 12:34 PM, zukasmichael notifications@github.com wrote:

i have tried 0.9.7 - still can't preload values if they are not in ui-select-choices http://plnkr.co/edit/o0pAujq9u4gCq2X96WEZ?p=preview

�
Reply to this email directly or view it on GitHub.

👍

Does the track by work already for single selection? Can't make it work.

doesn't work without proxy object multipleDemo $scope.multipleDemo = {}; on v 0.12
seee for mo infohttp://plnkr.co/edit/o0pAujq9u4gCq2X96WEZ?p=preview