vitalets / angular-xeditable

Edit in place for AngularJS
http://vitalets.github.io/angular-xeditable
MIT License
1.91k stars 403 forks source link

regression: radiolist whose value is not a number (integer or string) fails now #694

Closed pdemilly closed 5 years ago

pdemilly commented 7 years ago

If you have a radiolist whose value is a string that is not a number then it fails: Works fine with other elements like select. See fiddle below.

fiddle link

ckosloski commented 7 years ago

How about if a new option was added, something like e-StringList=true. If this is true, then ng-value is used, if false value will be used. Default to value.

pdemilly commented 7 years ago

radiolist should accept number or string as value. Just looking at the code quickly it looks like:

parsed.locals.valueFn should be tested is it needs surrounded quotes, something like that:

var ngValue = isNaN(parsed.locals.valueFn) ? '\"'+parsed.locals.valueFn+'\"' : parsed.locals.valueFn; but then again this code is confusing. too many + why not interpolate the whole string

var html = '<label data-ng-repeat="'+parsed.ngRepeat+'">'+ '<input type="radio" data-ng-disabled="::' + this.attrs.eNgDisabled + '" data-ng-model="$parent.$parent.$data" data-ng-value="' + $interpolate.startSymbol() + '::' + parsed.locals.valueFn + $interpolate.endSymbol() +'"' + ngChangeHtml + ngNameHtml + '>'+ '<span data-ng-bind="::'+parsed.locals.displayFn+'"></span></label>';

`var local = { ngRepeat: parse.ngRepeat, eNgDisabled: atts.engDisabled, $data: $parent.$parent.$data, startSymbol: $interpolate.startSymbol(), endSymbol: $interpolate.endSymbol, ngChangeHtml: ngChangeHtml, ngNameHtml: ngNameHtml, ngValueFn: isNaN(parsed.locals.valueFn) ? '\"'+parsed.locals.valueFn+'\"' : parsed.locals.valueFn, displayFn: parsed.locals.displayFn };

var htmlString = ''

var html = $interpolate (htmlString, locals); `

But then again using ES6 multiline strings would be a lot easier to read

ckosloski commented 7 years ago

I don't think the isNaN test will work. I tried that it always returns true even when the list is numbers. Also, if the list is strings, it needs to be value= while anything non-string should be ng-value=

https://docs.angularjs.org/api/ng/input/input%5Bradio%5D

pdemilly commented 7 years ago

The problem is not about using value or ng-value the problem is that you build a string which is going to be interpolated and the valueFn is not surrounded by quotes so it can only take real numbers or strings representing numbers. Anything else will generate an interpolation error as it should.

ckosloski commented 7 years ago

If your proposed fix works for you, can you please open a pull request?

ckosloski commented 7 years ago

HI @pdemilly I tried using your code above, but I could not get it to work. Here is what I did:

var local = {
            ngRepeat: parsed.ngRepeat,
            eNgDisabled: this.attrs.engDisabled,
            //$data: "$parent.$parent.$data",
            startSymbol: $interpolate.startSymbol(),
            endSymbol: $interpolate.endSymbol,
            ngChangeHtml: ngChangeHtml,
            ngNameHtml: ngNameHtml,
            ngValueFn: isNaN(parsed.locals.valueFn) ? '"'+parsed.locals.valueFn+'"' : parsed.locals.valueFn,
            displayFn: parsed.locals.displayFn
        };

        var htmlString =
            '<label data-ng-repeat="'+parsed.ngRepeat+'">'+
            '<input type="radio" data-ng-disabled="{{eNgDisabled}}"' +
            'data-ng-model="$parent.$parent.$data" data-ng-value="{{startSymbol}}{{ngValueFn}}{{endSymbol}}"' +
            '{{ngChangeHtml}} {{ngNameHtml}}/>'+
            '<span data-ng-bind="'+parsed.locals.displayFn+'"></span></label>';

var html = $interpolate (htmlString)(local);

I had to comment out $data because it could not find $parent during interpolation.

This code produces this html:

<input data-ng-disabled="" data-ng-model="$parent.$parent.$data" data-ng-value="{{" f.value""="" class="ng-pristine ng-untouched ng-valid ng-empty" name="257" type="radio">

Which doesn't work. Any suggestions?

pdemilly commented 7 years ago

My code was mostly pseudo code. On my end I didn't modify desirable since I'm using wrappers using angular components if I find some time I'll do a PR

On Oct 9, 2017 8:29 AM, "ckosloski" notifications@github.com wrote:

HI @pdemilly https://github.com/pdemilly I tried using your code above, but I could not get it to work. Here is what I did:

var local = { ngRepeat: parsed.ngRepeat, eNgDisabled: this.attrs.engDisabled, //$data: "$parent.$parent.$data", startSymbol: $interpolate.startSymbol(), endSymbol: $interpolate.endSymbol, ngChangeHtml: ngChangeHtml, ngNameHtml: ngNameHtml, ngValueFn: isNaN(parsed.locals.valueFn) ? '"'+parsed.locals.valueFn+'"' : parsed.locals.valueFn, displayFn: parsed.locals.displayFn };

    var htmlString =
        '<label data-ng-repeat="'+parsed.ngRepeat+'">'+
        '<input type="radio" data-ng-disabled="{{eNgDisabled}}"' +
        'data-ng-model="$parent.$parent.$data" data-ng-value="{{startSymbol}}{{ngValueFn}}{{endSymbol}}"' +
        '{{ngChangeHtml}} {{ngNameHtml}}/>'+
        '<span data-ng-bind="'+parsed.locals.displayFn+'"></span></label>';

var html = $interpolate (htmlString)(local);

I had to comment out $data because it could not find $parent during interpolation.

This code produces this html:

<input data-ng-disabled="" data-ng-model="$parent.$parent.$data" data-ng-value="{{" f.value""="" class="ng-pristine ng-untouched ng-valid ng-empty" name="257" type="radio">

Which doesn't work. Any suggestions?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vitalets/angular-xeditable/issues/694#issuecomment-335193116, or mute the thread https://github.com/notifications/unsubscribe-auth/AB9aGMvUwC41aUGU_eWQcnfbWZCiDsbPks5sqjv0gaJpZM4PqAx9 .

ckosloski commented 7 years ago

Thanks @pdemilly

sinall commented 5 years ago

Do you have any workaround? Currently I have to use a mediator scope.