Skip to content
This repository was archived by the owner on Jul 29, 2022. It is now read-only.

Add sort-option, fix key for selected, and fix double submit #178

Merged
NicolasCARPi merged 7 commits into from
Jun 20, 2018
Merged

Add sort-option, fix key for selected, and fix double submit #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
14da847
Add option to sort select-options
nathanvda Jun 17, 2018
6fc5db8
Use the key to set the selected value
nathanvda Jun 18, 2018
55b00c8
Naive/pragmatic to prevent double form submit
nathanvda Jun 18, 2018
c8d53d0
Fix codacy remarks
nathanvda Jun 18, 2018
1c72eb0
Fix codacy remarks, try II :)
nathanvda Jun 18, 2018
e1061fd
Codacy: fix last string quotes
nathanvda Jun 18, 2018
7ea5db4
Fixing @NicolasCarpi’s remarks
nathanvda Jun 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions src/jquery.jeditable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@

// timeout function
var t;
var isSubmitting = false;

if (settings.loadurl) {
t = self.setTimeout(function() {
Expand Down Expand Up @@ -339,13 +340,21 @@

form.submit(function(e) {

/* Do no submit. */
e.preventDefault();
e.stopPropagation();

if (isSubmitting) {
Copy link
Owner

@NicolasCARPi NicolasCARPi Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this part. Can you explain here why you added the isSubmitting var?

Copy link
Contributor Author

@nathanvda nathanvda Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@nathanvda nathanvda Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

console.warn('...we are already submitting .. stop!');
return false;
} else {
isSubmitting = true;
}

if (t) {
self.clearTimeout(t);
}

/* Do no submit. */
e.preventDefault();

/* Call before submit hook. */
/* If it returns false abort submitting. */
if (false !== onsubmit.apply(form, [settings, self])) {
Expand Down Expand Up @@ -668,13 +677,14 @@ var _supportInType = function (type) {
for (key in json) {
tuples.push([key, json[key]]); // Store: [key, value]
}
// sort it
tuples.sort(function(a, b) {
a = a[1];
b = b[1];
return a < b ? -1 : (a > b ? 1 : 0);
});

if (settings.sortSelectOptions) {
// sort it
tuples.sort(function (a, b) {
a = a[1];
b = b[1];
return a < b ? -1 : (a > b ? 1 : 0);
});
}
// now add the options to our select
var option;
for (var i = 0; i < tuples.length; i++) {
Expand All @@ -687,14 +697,14 @@ var _supportInType = function (type) {

if (key !== 'selected') {
option = $('<option />').val(key).append(value);
}

// add the selected prop if it's the same as original or if the key is 'selected'
if (key === 'selected' || key === $.trim(original.revert)) {
$(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)) {
$(option).prop('selected', 'selected');
}

$(this).find('select').append(option);
$(this).find('select').append(option);
}
}

// submit on change if no submit button defined
Expand Down Expand Up @@ -783,6 +793,7 @@ var _supportInType = function (type) {
loadtype : 'GET',
loadtext : 'Loading...',
placeholder: 'Click to edit',
sortSelectOptions: false,
loaddata : {},
submitdata : {},
ajaxoptions: {}
Expand Down
71 changes: 71 additions & 0 deletions tests/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,74 @@ QUnit.test('Enable/disable/destroy', function(assert) {
elem.editable().editable('destroy');
assert.notOk(elem.data('event.editable'));
});

QUnit.module('select-boxes');
QUnit.test('Default: NOT Sorting select options', function(assert) {
elem.append( '<span id="select-tester">Letter F</span>' );

$.fn.editable.defaults.sortSelectOptions = false;

$('#select-tester').editable('http://bla', {
type: 'select',
data: {'E': 'Letter E', 'F': 'Letter F', 'D': 'Letter Disk'},
selected: 'F'
});

assert.equal( $('#select-tester').attr('title'), 'Click to edit', 'Editable enabled: it sets the title' );
$('#select-tester').click();
assert.equal($('#select-tester form').length, 1, 'Clicking Editable adds inline form');

var optionsList = [];
$('#select-tester option').each(function(name, val) { optionsList.push(val.text); });

assert.deepEqual(optionsList, ['Letter E', 'Letter F', 'Letter Disk'], 'Does not sort the given options-list');
});
QUnit.test('Default: Sorting select options', function(assert) {
elem.append( '<span id="select-sorted-tester">Letter F</span>' );

$.fn.editable.defaults.sortSelectOptions = true;

$('#select-sorted-tester').editable('http://bla', {
type: 'select',
data: {'E': 'Letter E', 'F': 'Letter F', 'D': 'Letter Disk'},
selected: 'F'
});
assert.equal( $('#select-sorted-tester').attr('title'), 'Click to edit', 'Editable enabled: it sets the title' );
$('#select-sorted-tester').click();
assert.equal($('#select-sorted-tester form').length, 1, 'Clicking Editable adds inline form');

var optionsList = [];
$('#select-sorted-tester option').each(function(name, val) { optionsList.push(val.text); });

assert.deepEqual(optionsList, ['Letter Disk', 'Letter E', 'Letter F'], 'It does sort the given options list');
});
QUnit.module('select-boxes setting selected');
QUnit.test('Explicitly setting a selected option', function(assert) {
elem.append( '<span id="selected-tester">Letter F</span>' );

$.fn.editable.defaults.sortSelectOptions = false;

$('#selected-tester').editable('http://bla', {
type: 'select',
data: {'E': 'Letter E', 'F': 'Letter F', 'D': 'Letter Disk', 'selected': 'F'},
});

assert.equal( $( '#selected-tester').attr('title'), 'Click to edit', 'Editable enabled: it sets the title' );
$('#selected-tester').click();
assert.equal($('#selected-tester form select :selected').text(), 'Letter F', 'Sets the correct value as selected');
});
QUnit.test('Not setting a selected option', function(assert) {
elem.append( '<span id="selected-tester">Letter F</span>' );

$.fn.editable.defaults.sortSelectOptions = false;

$('#selected-tester').editable('http://bla', {
type: 'select',
data: {'E': 'Letter E', 'F': 'Letter F', 'D': 'Letter Disk'},
});

assert.equal( $( '#selected-tester').attr('title'), 'Click to edit', 'Editable enabled: it sets the title' );
$('#selected-tester').click();
assert.equal($('#selected-tester form select :selected').text(), 'Letter E', 'Selects the first option as selected?');
});