Add sort-option, fix key for selected, and fix double submit#178
Add sort-option, fix key for selected, and fix double submit#178NicolasCARPi merged 7 commits intoNicolasCARPi:experimentalfrom
Conversation
The default option is not-sorted, which is what it used to be long ago. Not sure what is preferable.
Not sure what caused it, but somehow the form.submit was called twice. This prevents double submits in a naive but effective way.
I am leaving the console.warn in there, but we could just as easily remove it? I know logging to console could be a problem with IE9-10 (if it is not opened —WTF) but I prefer to include a console shim in my own code. And there already is another in the code.
|
Sorry, it got a bit dirty when fixing the codacy reviews. Squash merge it? |
NicolasCARPi
left a comment
There was a problem hiding this comment.
Hello, thanks for your contribution. Please see my comments.
Congrats for adding a qunit test ;)
src/jquery.jeditable.js
Outdated
| b = b[1]; | ||
| return a < b ? -1 : (a > b ? 1 : 0); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Trailing whitespace (add a plugin to your editor to see them!)
src/jquery.jeditable.js
Outdated
| $(option).prop('selected', 'selected'); | ||
| } | ||
| // 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)) { |
There was a problem hiding this comment.
use dot notation (json.selected)
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
|
|
||
| if (isSubmitting) { |
There was a problem hiding this comment.
I don't get this part. Can you explain here why you added the isSubmitting var?
There was a problem hiding this comment.
As I explained above: I experienced double submits, form.submit was called twice, and this was my work-around. You do not have to include this if I am the only person experiencing this? I have been experiencing this problem for a long time, but it didn't really bother me (my data was just saved twice) (I noticed this in the logfiles but just ignored it), until I attached a callback and then it became a "visual" problem. I tried making a test-case for it, but I need to find a way how to capture the calling of the ajax-request.
There was a problem hiding this comment.
To be clear: the user did not do anything strange, just press enter, or click 'Ok' (to start the submit action). It occurs in chrome, ie and firefox. The only thing deviating from the default is I style my buttons using bootstrap, which should not cause it imho?
|
Hello, I have merged your contribution. I renamed the option in lower case to be consistant with the rest. And also removed the console warning. Thanks :) |
|
Thanks 👏 👏 and it was my pleasure 👍 👍 😉 |
So maybe this is not ideal, but the first two commits were discussed in #167 (and #176) where I
I set the default option to not-sorted, which may not be your "default" choice. That could easily by changed? I added tests for these two cases.
Furthermore I added a naive approach to prevent double submits. I noticed in my code that every form.submit was called twice. I was not able to get to the real cause of that, but did manage a simple work-around preventing it.
I hope this is useful. If you have any feedback, let me know.