Selected attribute works on value not key

[nicdnepr]

Description

When use 'selected':number it not select if 'selected' is string it works

how to reproduce

$(this).editable(edit, {
            submit: 'OK',
            cancel: 'Cancel',
            type: 'select',
            data   : '{"2":"Wiki","3":"Banana","Apple":"Apple", "Pear":"Pear", "selected":"3"}',
        });

(add a jsfiddle/codepen example)

expected result

actual result

Environment

jquery v3.2.1 jeditable last ver Browser: chrome OS: ubuntu

[NicolasCARPi]

Hello,

That's because the selected works on the value, not the key.

[nicdnepr]

Is this bug? here is example data : " {'E':'Letter E','F':'Letter F','G':'Letter G', 'selected':'F'}" Also I remember before it selected by key

[NicolasCARPi]

Yes you are right. Also it makes more sense that it would be the key. The bug is probably in these lines.

[nathanvda]

I have a fix for this. Is there a specific reason why the tuples are sorted? This is very annoying to me, so I removed that in my local code.

                        // add the selected prop if it's the same as original or if the key is 'selected'
                        if (json['selected'] === key || key === $.trim(original.revert)) {
                            $(option).prop('selected', value);
                        }

Not sure why the $.trim(original.revert) is there and if that still makes sense, but this fixes my use case. I will prepare a pull request later if appreciated. I also fixed another problem with double submits I had (if that would be interesting for other people).

[NicolasCARPi]

Hello,

I have a fix for this.

Good :)

Is there a specific reason why the tuples are sorted?

Yes, it's to sort the elements by name (not value as before). See https://github.com/NicolasCARPi/jquery_jeditable/issues/97

I will prepare a pull request later if appreciated. I also fixed another problem with double submits I had (if that would be interesting for other people).

Please do!

~Nico

[nathanvda]

But there is no option to leave the options unsorted for now? I want to be able to control the order. There is no technical reason to sort them, right?

[NicolasCARPi]

Yes an option would be best.

[nicdnepr]

Looks still not fixed?

[NicolasCARPi]

@nicdnepr PRs are welcome ;)

[nathanvda]

I have not yet gotten round to preparing a PR. Hope to find some time this weekend :+1: