amitava82 / angular-multiselect

[NOT MAINTAINED]Native AngularJS multiselect directive
http://amitava82.github.io/angular-multiselect
MIT License
140 stars 124 forks source link

Strange behaviour of the uncheckAll method #91

Open VladZen opened 7 years ago

VladZen commented 7 years ago

https://github.com/amitava82/angular-multiselect/blob/master/src/multiselect.js#L231 there is a line in the code, that i can't understand. what is the point to remove checked mark only for items, that has been found according to search text, if it was typed?

amitava82 commented 7 years ago

Looks like unchecking within search context, if user wants up remove all items matched search text

VladZen commented 7 years ago

And also there is a method https://github.com/amitava82/angular-multiselect/blob/master/src/multiselect.js#L235 that accepts true as an argument.

but why should we always call it with an argument if we have local variable as isMultiple at the top of the code? https://github.com/amitava82/angular-multiselect/blob/master/src/multiselect.js#L41

does it make any sense?

VladZen commented 7 years ago

so got next situation: we have single select. when we use it and select items in dropdown this method is fired https://github.com/amitava82/angular-multiselect/blob/master/src/multiselect.js#L165 we call setModelValue twice: with different opposite arguments. i mean from uncheckedAll and then from selectSingle.

amitava82 commented 7 years ago

You could, works either way. Anyway, it's a 5 years old codebase, I never worked on this after release, haven't worked on Angular since React came out. I'm sure there are things that could be fixed and there are silly things like this considering I was just learning JS back then.. Send PR if you are interested to make improvements.