search

alexandernst / angular-multi-select

A multi select dropdown directive for AngularJS.
http://alexandernst.github.io/angular-multi-select
MIT License
59 stars 26 forks source link

convert preselectValue to array if required #72

Closed fifonik closed 8 years ago

fifonik commented 8 years ago

71

fifonik commented 8 years ago

Why JSON.parse('stringValue') should throw a syntax error? It will be parsed to string value and then converted to array that contains the single string value. If preselectValue is specified as an array -- it will be kept as is. If an error with parsing value will be thrown, it will be defaulting to empty string and nothing will be pre-selected (I prefer not to catch the error in this case so developer will see the error)

zachlysobey commented 8 years ago

screen shot 2015-12-02 at 5 35 37 pm

@fifonik 'stringValue' is apparently not valid JSON?

I wasn't sure myself so I pasted it into the console before making that comment.

zachlysobey commented 8 years ago

It does appear that wrapping it in another set of quotes would work however https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse

In any case, would probably be good to associate such a change with a unit test if possible

alexandernst commented 8 years ago

@fifonik I'm with @zachlysobey here. Single quotes is invalid JSON, which will make preselected-value="'Chrome'" invalid, and given the fact that HTML requires double quotes for attributes, there isn't a way to make something like preselected-value=""Chrome"".

Here is an example of the bug (based on your punkr): http://plnkr.co/edit/5EHduosUKU4fi5UxbyVk?p=preview

I'd suggest leaving the catch code as-it-was, that would keep backwards compatibility and also fix your case.

fifonik commented 8 years ago

You are right, my changes in catch section were incorrect.