Skip to content

Commit

Permalink
Do not propagate click when search box is not empty (#5580)
Browse files Browse the repository at this point in the history
This fixes a long-standing bug where if you tried to click in the
search box for a multiple select while there was text in it, the
dropdown would close and the text would be cleared. This caused
many unexpected issues, because it meant that you could only use
your keyboard to edit text within the search box.

This will still clear out the search field if you click within the
area of the selection which is not the search field. I'm not sure
if that is also unexpected behaviour, so for now I am going to
maintain it.

Fixes #3517
Fixes #3808
Fixes #5491
Closes #5551
  • Loading branch information
kevin-brown committed Jul 21, 2019
1 parent 8957615 commit f2d527e
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/js/select2/selection/search.js
Expand Up @@ -90,6 +90,12 @@ define([
}
});

this.$selection.on('click', '.select2-search--inline', function (evt) {
if (self.$search.val()) {
evt.stopPropagation();
}
});

// Try to detect the IE version should the `documentMode` property that
// is stored on the document. This is only implemented in IE and is
// slightly cleaner than doing a user agent check.
Expand Down
86 changes: 86 additions & 0 deletions tests/selection/search-tests.js
Expand Up @@ -188,4 +188,90 @@ test('the focus event shifts the focus', function (assert) {
$search[0],
'The search did not have focus originally'
);
});

test('search box without text should propagate click', function (assert) {
assert.expect(1);

var $container = $('#qunit-fixture .event-container');
var container = new MockContainer();

var CustomSelection = Utils.Decorate(MultipleSelection, InlineSearch);

var $element = $('#qunit-fixture .multiple');
var selection = new CustomSelection($element, options);

var $selection = selection.render();
selection.bind(container, $container);

// Update the selection so the search is rendered
selection.update([]);

// Make it visible so the browser can place focus on the search
$container.append($selection);

$selection.on('click', function () {
assert.ok(true, 'The click event should not have been trapped');
});

var $search = $selection.find('input');
$search.trigger('click');
});

test('search box with text should not propagate click', function (assert) {
assert.expect(0);

var $container = $('#qunit-fixture .event-container');
var container = new MockContainer();

var CustomSelection = Utils.Decorate(MultipleSelection, InlineSearch);

var $element = $('#qunit-fixture .multiple');
var selection = new CustomSelection($element, options);

var $selection = selection.render();
selection.bind(container, $container);

// Update the selection so the search is rendered
selection.update([]);

// Make it visible so the browser can place focus on the search
$container.append($selection);

$selection.on('click', function () {
assert.ok(false, 'The click event should have been trapped');
});

var $search = $selection.find('input');
$search.val('test');
$search.trigger('click');
});

test('search box with text should not close dropdown', function (assert) {
assert.expect(0);

var $container = $('#qunit-fixture .event-container');
var container = new MockContainer();

var CustomSelection = Utils.Decorate(MultipleSelection, InlineSearch);

var $element = $('#qunit-fixture .multiple');
var selection = new CustomSelection($element, options);

var $selection = selection.render();
selection.bind(container, $container);

// Update the selection so the search is rendered
selection.update([]);

// Make it visible so the browser can place focus on the search
$container.append($selection);

container.on('close', function () {
assert.ok(false, 'The dropdown should not have closed');
});

var $search = $selection.find('input');
$search.val('test');
$search.trigger('click');
});

0 comments on commit f2d527e

Please sign in to comment.