Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger 'input' event before 'change' events #4649

Merged
merged 1 commit into from Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/js/select2/compat/inputData.js
Expand Up @@ -65,13 +65,13 @@ define([
});

this.$element.val(data.id);
this.$element.trigger('change');
this.$element.trigger('input').trigger('change');
} else {
var value = this.$element.val();
value += this._valueSeparator + data.id;

this.$element.val(value);
this.$element.trigger('change');
this.$element.trigger('input').trigger('change');
}
};

Expand All @@ -94,7 +94,7 @@ define([
}

self.$element.val(values.join(self._valueSeparator));
self.$element.trigger('change');
self.$element.trigger('input').trigger('change');
});
};

Expand Down
2 changes: 1 addition & 1 deletion src/js/select2/core.js
Expand Up @@ -556,7 +556,7 @@ define([
});
}

this.$element.val(newVal).trigger('change');
this.$element.val(newVal).trigger('input').trigger('change');
};

Select2.prototype.destroy = function () {
Expand Down
10 changes: 5 additions & 5 deletions src/js/select2/data/select.js
Expand Up @@ -36,7 +36,7 @@ define([
if ($(data.element).is('option')) {
data.element.selected = true;

this.$element.trigger('change');
this.$element.trigger('input').trigger('change');

return;
}
Expand All @@ -57,13 +57,13 @@ define([
}

self.$element.val(val);
self.$element.trigger('change');
self.$element.trigger('input').trigger('change');
});
} else {
var val = data.id;

this.$element.val(val);
this.$element.trigger('change');
this.$element.trigger('input').trigger('change');
}
};

Expand All @@ -79,7 +79,7 @@ define([
if ($(data.element).is('option')) {
data.element.selected = false;

this.$element.trigger('change');
this.$element.trigger('input').trigger('change');

return;
}
Expand All @@ -97,7 +97,7 @@ define([

self.$element.val(val);

self.$element.trigger('change');
self.$element.trigger('input').trigger('change');
});
};

Expand Down
4 changes: 2 additions & 2 deletions src/js/select2/selection/allowClear.js
Expand Up @@ -74,7 +74,7 @@ define([
}
}

this.$element.trigger('change');
this.$element.trigger('input').trigger('change');

this.trigger('toggle', {});
};
Expand All @@ -97,7 +97,7 @@ define([
return;
}

var removeAll = this.options.get('translations').get('removeAllItems');
var removeAll = this.options.get('translations').get('removeAllItems');

var $remove = $(
'<span class="select2-selection__clear" title="' + removeAll() +'">' +
Expand Down
52 changes: 40 additions & 12 deletions tests/data/select-tests.js
Expand Up @@ -167,12 +167,14 @@ test('duplicates - single - same id on select triggers change',
var data = new SelectData($select, data);
var second = $('#qunit-fixture .duplicates option')[2];

var changeTriggered = false;
var changeTriggered = false, inputTriggered = false;

assert.equal($select.val(), 'one');

$select.on('change', function () {
changeTriggered = true;
changeTriggered = inputTriggered;
}).on('input', function() {
inputTriggered = true;
});

data.select({
Expand All @@ -187,9 +189,14 @@ test('duplicates - single - same id on select triggers change',
'The value never changed'
);

assert.ok(
inputTriggered,
'The input event should be triggered'
);

assert.ok(
changeTriggered,
'The change event should be triggered'
'The change event should be triggered after the input event'
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how I feel about enforcing the order of the events within past tests. I can understand adding tests to check that the input event is triggered, and adding tests to ensure that one event is caught before the other, but I don't think that requires you to modify old change-only tests to cover both those cases.

);

assert.ok(
Expand All @@ -205,12 +212,14 @@ test('duplicates - single - different id on select triggers change',
var data = new SelectData($select, data);
var second = $('#qunit-fixture .duplicates option')[2];

var changeTriggered = false;
var changeTriggered = false, inputTriggered = false;

$select.val('two');

$select.on('change', function () {
changeTriggered = true;
changeTriggered = inputTriggered;
}).on('input', function() {
inputTriggered = true;
});

data.select({
Expand All @@ -225,9 +234,14 @@ test('duplicates - single - different id on select triggers change',
'The value changed to the duplicate id'
);

assert.ok(
inputTriggered,
'The input event should be triggered'
);

assert.ok(
changeTriggered,
'The change event should be triggered'
'The change event should be triggered after the input event'
);

assert.ok(
Expand All @@ -243,12 +257,14 @@ function (assert) {
var data = new SelectData($select, data);
var second = $('#qunit-fixture .duplicates-multi option')[2];

var changeTriggered = false;
var changeTriggered = false, inputTriggered = false;

$select.val(['one']);

$select.on('change', function () {
changeTriggered = true;
changeTriggered = inputTriggered;
}).on('input', function() {
inputTriggered = true;
});

data.select({
Expand All @@ -263,9 +279,14 @@ function (assert) {
'The value now has duplicates'
);

assert.ok(
inputTriggered,
'The input event should be triggered'
);

assert.ok(
changeTriggered,
'The change event should be triggered'
'The change event should be triggered after the input event'
);

assert.ok(
Expand All @@ -281,12 +302,14 @@ function (assert) {
var data = new SelectData($select, data);
var second = $('#qunit-fixture .duplicates-multi option')[2];

var changeTriggered = false;
var changeTriggered = false, inputTriggered = false;

$select.val(['two']);

$select.on('change', function () {
changeTriggered = true;
changeTriggered = inputTriggered;
}).on('input', function() {
inputTriggered = true;
});

data.select({
Expand All @@ -301,9 +324,14 @@ function (assert) {
'The value has the new id'
);

assert.ok(
inputTriggered,
'The input event should be triggered'
);

assert.ok(
changeTriggered,
'The change event should be triggered'
'The change event should be triggered after the input event'
);

assert.ok(
Expand Down