Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

ui-select modifying ng-model object #540

Closed
ilanbiala opened this issue Dec 19, 2014 · 23 comments
Closed

ui-select modifying ng-model object #540

ilanbiala opened this issue Dec 19, 2014 · 23 comments

Comments

@ilanbiala
Copy link

With this snippet:

<div class="form-group">
            <label for="users" class="col-sm-3 control-label">Users</label>
            <div class="col-sm-8">
                <ui-select multiple search-enabled ng-model="editingDesk.users" on-remove="editingDesk.removedUsers.has($item) ? editingDesk.removedUsers.delete($item) : editingDesk.removedUsers.add($item)" on-select="editingDesk.newUsers.has($item) ? editingDesk.newUsers.delete($item) : editingDesk.newUsers.add($item)">
                    <ui-select-match placeholder="Select users...">{{$item.userName}}</ui-select-match>
                    <ui-select-choices repeat="user in users | filter: $select.search">
                        {{user.userName}}
                    </ui-select-choices>
                </ui-select>
            </div>
        </div>

When the controller instantiates, editingDesk.users is properly populated. Once the controller finishes instantiating, editingDesk.users.length is 0. If I comment out the HTML above, then I don't have this issue.

@brianfeister
Copy link

My only thought is that it would relate to the changes to on-remove and / or on-select, added here: 83e8282. But that is not yet released and is after the latest tag. Did you copy the code directly from source or install via bower?

@ilanbiala
Copy link
Author

I went into the dist folder and downloaded select.min.js since I'm not using Bower. The commit code you linked only runs when the ui-select is updated somehow, but this issue comes up before anything has happened.

@brianfeister
Copy link

You'll need to post an example of it not working correctly via something like http://plnkr.co

@ilanbiala
Copy link
Author

http://plnkr.co/edit/sJEQrTeEgDUs8Ca0sdne?p=preview has a demo, but ui-select is either broken or plunker is broken because there are errors in the console that I don't see in my project. I double checked my code and I'm almost positive I'm not modifying the variable after the controller instantiates, is there anything else you can think of?

@divan
Copy link

divan commented Dec 21, 2014

I got the same issue here. ui-select installed via bower.

I found that problem occurs when the model contains objects different from the ui-select-choices source. I.e. when editingDesk.users == [$users[0],$users[1]], it will work, but if editingDesk.users == [user1, user2] - it will clear editingDesk.users (assuming user1 and user2 are of the same type as $users[0], of course).

Here is plunker demo: http://plnkr.co/edit/62RgP0cLCsbqWl9QBurc?p=preview . Uncomment last lines in demo.js to see the difference.

@ilanbiala
Copy link
Author

@brianfeister any thoughts on how we can fix this?

@benitogf
Copy link

hello I used this and its working now, here is an example http://plnkr.co/edit/NJ21xltT1ABCgWmhuaDo?p=preview

@brianfeister
Copy link

@b3n0n - thank you! Sorry to everyone else, I just haven't had the time to look into this. Does @b3n0n's code help?

@ilanbiala
Copy link
Author

@brianfeister Not sure if I'm putting in the wrong place or @b3n0n is using an old version, but with the latest select.js from the dist folder and his change made, it doesn't seem to fix the issue.

@benitogf
Copy link

It seems to work only for static lists, I was trying to do this but still no luck 😄

@ilanbiala
Copy link
Author

Yeah this needs a proper solution @brianfeister. Take a look at #410 or #405 and a few others.

@brianfeister
Copy link

Thanks for helping this along @ilanbiala and @b3n0n. I've implemented the change advised in the referenced PR's #410 & #405 in this plunkr. Unfortunately, it doesn't fix the issue. The code has moved alot since the PR's were opened, sorry to say. I'm watching this thread - if someone were to make the Plunkr work and post a code snippet, I could get a fix committed and merge it but I'm under an extreme deadline for a few weeks at work so I'll be unlikely to debug it myself during that window.

@ilanbiala
Copy link
Author

@brianfeister could it be because i'm using an ES6 Set?

@brianfeister
Copy link

That could definitely be the cause, I doubt angular.equals supports ES6 collections. Perhaps you could pre-compile with Traceur: http://arv.github.io/ngeurope/#/25.

@ilanbiala
Copy link
Author

@brianfeister I switched to an array, and I still have this issue. Can you take a look to see what the issue is, because now I'm just using regular javascript.

@Asimov4
Copy link
Contributor

Asimov4 commented Jan 12, 2015

There are cases where it does not make sense to check the values in the ng-model against the ui-select-choices as described in #405 .
In the case of tagging, any value is potentially valid and should not be discarded.

@benitogf
Copy link

so it could be a toggle for this verification? @brianfeister @Asimov4 ?

@ilanbiala
Copy link
Author

@brianfeister this seems like it could be prevented by what @Asimov4 is saying. Any thoughts?

Asimov4 added a commit to Asimov4/ui-select that referenced this issue Feb 17, 2015
Motivation: Fix angular-ui#540
The tagging mode is designed to allow new values to be added to the model.
It does not make sense to check those values against the list of choices when going from model --> view.
@Asimov4
Copy link
Contributor

Asimov4 commented Feb 17, 2015

I suggested a pull request to not check values if the tagging mode is enabled.
According to the doc: https://github.com/angular-ui/ui-select/wiki/ui-select
"Enable tagging mode (add new items on the fly)." means that new items can be added to the list of choices. We should not check the model against the initial list of choices when trying to go from model to view.

Here is a plunker where this change is in: http://plnkr.co/edit/XeUwyQV0nTHaQmgg4Sc4?p=preview
trying adding or removing the tagging option on the ui-select element.

Asimov4 added a commit to Asimov4/ui-select that referenced this issue Feb 18, 2015
Motivation: Fix angular-ui#540
The tagging mode is designed to allow new values to be added to the model.
It does not make sense to check those values against the list of choices when going from model --> view.
Asimov4 added a commit to Asimov4/ui-select that referenced this issue Feb 18, 2015
Asimov4 added a commit to Asimov4/ui-select that referenced this issue Feb 18, 2015
Motivation: Fix angular-ui#540
The tagging mode is designed to allow new values to be added to the model.
It does not make sense to check those values against the list of choices when going from model --> view.
@Asimov4
Copy link
Contributor

Asimov4 commented Feb 18, 2015

I submitted this new pull request with a test: #683
Thanks for the feedback @dimirc and sorry for the messy attempts.

Asimov4 added a commit to Asimov4/ui-select that referenced this issue Mar 8, 2015
Motivation: Fix angular-ui#540
The tagging mode is designed to allow new values to be added to the model.
It does not make sense to check those values against the list of choices when going from model --> view.
Asimov4 added a commit to Asimov4/ui-select that referenced this issue Mar 8, 2015
Motivation: Fix angular-ui#540
The tagging mode is designed to allow new values to be added to the model.
It does not make sense to check those values against the list of choices when going from model --> view.
@dimirc
Copy link
Contributor

dimirc commented Mar 12, 2015

@ilanbiala are you still having issues with this? I'm putting together some changes at #748 that might be related

@ilanbiala
Copy link
Author

I reworked my implementation to hack around it after having some issues.

Asimov4 added a commit to Asimov4/ui-select that referenced this issue Mar 13, 2015
Motivation: Fix angular-ui#540
The tagging mode is designed to allow new values to be added to the model.
It does not make sense to check those values against the list of choices when going from model --> view.
Asimov4 added a commit to Asimov4/ui-select that referenced this issue Mar 13, 2015
Motivation: Fix angular-ui#540

The tagging mode is designed to allow new values to be added to the model.

It does not make sense to check those values against the list of choices when going from model --> view.
Asimov4 added a commit to Asimov4/ui-select that referenced this issue Mar 17, 2015
Motivation: Fix angular-ui#540

The tagging mode is designed to allow new values to be added to the model.

It does not make sense to check those values against the list of choices when going from model --> view.

Conflicts:
	src/uiSelectMultipleDirective.js
@wesleycho
Copy link
Contributor

Is this still currently an issue? Can someone post a reproduction in Plunker based off of the latest release?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.