From 89576153a026e5bdf254bf63f7ef2541e8d03928 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Sun, 21 Jul 2019 11:44:09 -0400 Subject: [PATCH 01/19] Mirror disabled state through aria-disabled on selection (#5579) This is needed to screen readers know that the Select2 is disabled when focus is put on the selection container. Because we were mirroring the disabled state to the search input on a multiple select in the past, this is really only needed for single select elements which would not otherwise has the disabled property. This was identified in a previous accessibility audit as being something which Select2 did not properly report because we were not setting the attributes properly. Fixes #4575 --- src/js/select2/selection/base.js | 3 +++ tests/a11y/selection-tests.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/js/select2/selection/base.js b/src/js/select2/selection/base.js index d79c960b..b83c2688 100644 --- a/src/js/select2/selection/base.js +++ b/src/js/select2/selection/base.js @@ -29,6 +29,7 @@ define([ $selection.attr('title', this.$element.attr('title')); $selection.attr('tabindex', this._tabindex); + $selection.attr('aria-disabled', 'false'); this.$selection = $selection; @@ -88,10 +89,12 @@ define([ container.on('enable', function () { self.$selection.attr('tabindex', self._tabindex); + self.$selection.attr('aria-disabled', 'false'); }); container.on('disable', function () { self.$selection.attr('tabindex', '-1'); + self.$selection.attr('aria-disabled', 'true'); }); }; diff --git a/tests/a11y/selection-tests.js b/tests/a11y/selection-tests.js index e2c14602..b2f1b2e4 100644 --- a/tests/a11y/selection-tests.js +++ b/tests/a11y/selection-tests.js @@ -131,6 +131,38 @@ test('a custom tabindex is copied', function (assert) { ); }); +test('aria-disabled should reflected disabled state', function (assert) { + var $select = $('#qunit-fixture .single'); + + var selection = new BaseSelection($select, options); + var $selection = selection.render(); + + var container = new MockContainer(); + selection.bind(container, $('')); + + assert.equal( + $selection.attr('aria-disabled'), + 'false', + 'The tab index should match the original tab index' + ); + + container.trigger('disable'); + + assert.equal( + $selection.attr('aria-disabled'), + 'true', + 'The selection should be dropped out of the tab order when disabled' + ); + + container.trigger('enable'); + + assert.equal( + $selection.attr('aria-disabled'), + 'false', + 'The tab index should be restored when re-enabled' + ); +}); + module('Accessibility - Single'); test('aria-labelledby should match the rendered container', function (assert) { From f2d527ea97fdfc05934792ac3906bb1a45e51e6f Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Sun, 21 Jul 2019 13:04:51 -0400 Subject: [PATCH 02/19] Do not propagate click when search box is not empty (#5580) 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 --- src/js/select2/selection/search.js | 6 +++ tests/selection/search-tests.js | 86 ++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/src/js/select2/selection/search.js b/src/js/select2/selection/search.js index 353c5621..78997d3e 100644 --- a/src/js/select2/selection/search.js +++ b/src/js/select2/selection/search.js @@ -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. diff --git a/tests/selection/search-tests.js b/tests/selection/search-tests.js index 43345d72..439b7ed6 100644 --- a/tests/selection/search-tests.js +++ b/tests/selection/search-tests.js @@ -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'); }); \ No newline at end of file From 2fce8ae6c4937b88c332ad3e30e135cf24504cf1 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Sun, 21 Jul 2019 15:44:37 -0400 Subject: [PATCH 03/19] Fix `maximumSelectionLength` being ignored by `closeOnSelect` (#5581) * Rewrote maximumSelectionLength tests to use container These brings the tests in line with other tests which we have, and makes it easier to understand what is actually going on in the tests. This also removes a redundant set of tests where we were testing with => 2 options being allowed. There are no current edge cases that would have required this. * Fix maximumSelectionLength being ignored by closeOnSelect There was a bug where the `maximumSelectionLength` option would not kick in if the `closeOnSelect` option was enabled. Normally, this was enabled by someone in their global configuration, but it could also be seen when somoene selected an option while holding the meta/ctrl/alt keys. This would implicitly enable the `closeOnSelect` behaviour, even when it was not globally enabled, and cause the bug. This fixes that issue by listening to the `select` event which is triggered whenever an option is selected, and triggers the "maximum selected" message based on that event. This should now force the message to be displayed, even when the results did not have to be queried another time. Fixes #3514 Fixes #3860 Closes #5333 --- src/js/select2/data/maximumSelectionLength.js | 25 ++- tests/data/maximumSelectionLength-tests.js | 194 +++++++----------- 2 files changed, 96 insertions(+), 123 deletions(-) diff --git a/src/js/select2/data/maximumSelectionLength.js b/src/js/select2/data/maximumSelectionLength.js index ae727529..663e94e7 100644 --- a/src/js/select2/data/maximumSelectionLength.js +++ b/src/js/select2/data/maximumSelectionLength.js @@ -7,10 +7,30 @@ define([ decorated.call(this, $e, options); } + MaximumSelectionLength.prototype.bind = + function (decorated, container, $container) { + var self = this; + + decorated.call(this, container, $container); + + container.on('select', function () { + self._checkIfMaximumSelected(); + }); + }; + MaximumSelectionLength.prototype.query = function (decorated, params, callback) { var self = this; + this._checkIfMaximumSelected(function () { + decorated.call(self, params, callback); + }); + }; + + MaximumSelectionLength.prototype._checkIfMaximumSelected = + function (_, successCallback) { + var self = this; + this.current(function (currentData) { var count = currentData != null ? currentData.length : 0; if (self.maximumSelectionLength > 0 && @@ -23,7 +43,10 @@ define([ }); return; } - decorated.call(self, params, callback); + + if (successCallback) { + successCallback(); + } }); }; diff --git a/tests/data/maximumSelectionLength-tests.js b/tests/data/maximumSelectionLength-tests.js index 89943b38..f11c7e9a 100644 --- a/tests/data/maximumSelectionLength-tests.js +++ b/tests/data/maximumSelectionLength-tests.js @@ -1,202 +1,152 @@ module('Data adapters - Maximum selection length'); +var SelectData = require('select2/data/select'); var MaximumSelectionLength = require('select2/data/maximumSelectionLength'); var $ = require('jquery'); var Options = require('select2/options'); var Utils = require('select2/utils'); -function MaximumSelectionStub () { - this.called = false; - this.currentData = []; -} - -MaximumSelectionStub.prototype.current = function (callback) { - callback(this.currentData); -}; - -MaximumSelectionStub.prototype.val = function (val) { - this.currentData.push(val); -}; - -MaximumSelectionStub.prototype.query = function (params, callback) { - this.called = true; -}; - -var MaximumSelectionData = Utils.Decorate( - MaximumSelectionStub, - MaximumSelectionLength -); +var MaximumSelectionData = Utils.Decorate(SelectData, MaximumSelectionLength); test('0 never displays the notice', function (assert) { + assert.expect(3); + + var $select = $('#qunit-fixture .multiple'); + var zeroOptions = new Options({ maximumSelectionLength: 0 }); - var data = new MaximumSelectionData(null, zeroOptions); + var container = new MockContainer(); + var data = new MaximumSelectionData($select, zeroOptions); - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; + data.bind(container, null); + + data.on('results:message', function () { + assert.ok(false, 'The message should not be displayed'); + }); data.query({ term: '' + }, function () { + assert.ok(true, 'The results should be queried'); }); - assert.ok(data.called); - - data = new MaximumSelectionData(null, zeroOptions); - - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; - - data.val('1'); + $select.val(['One']); data.query({ term: '' + }, function () { + assert.ok(true, 'The results should be queried'); }); - assert.ok(data.called); - - data = new MaximumSelectionData(null, zeroOptions); - - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; - - data.val('1'); - data.val('2'); + $select.val(['One', 'Two']); data.query({ term: '' + }, function () { + assert.ok(true, 'The results should be queried'); }); - - assert.ok(data.called); }); test('< 0 never displays the notice', function (assert) { + assert.expect(3); + + var $select = $('#qunit-fixture .multiple'); + var negativeOptions = new Options({ maximumSelectionLength: -1 }); - var data = new MaximumSelectionData(null, negativeOptions); + var container = new MockContainer(); + var data = new MaximumSelectionData($select, negativeOptions); - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; + data.bind(container, null); + + data.on('results:message', function () { + assert.ok(false, 'The message should not be displayed'); + }); data.query({ term: '' + }, function () { + assert.ok(true, 'The results should be queried'); }); - assert.ok(data.called); - - data = new MaximumSelectionData(null, negativeOptions); - - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; - - data.val('1'); + $select.val(['One']); data.query({ term: '' + }, function () { + assert.ok(true, 'The results should be queried'); }); - assert.ok(data.called); - - data = new MaximumSelectionData(null, negativeOptions); - - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; - - data.val('1'); - data.val('2'); + $select.val(['One', 'Two']); data.query({ term: '' + }, function () { + assert.ok(true, 'The results should be queried'); }); - - assert.ok(data.called); }); test('triggers when >= 1 selection' , function (assert) { + assert.expect(2); + + var $select = $('#qunit-fixture .multiple'); + var maxOfOneOptions = new Options({ maximumSelectionLength: 1 }); - var data = new MaximumSelectionData(null, maxOfOneOptions); - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; + var container = new MockContainer(); + var data = new MaximumSelectionData($select, maxOfOneOptions); + + data.bind(container, null); + + data.on('results:message', function () { + assert.ok(true, 'The message should be displayed'); + }); + + $select.val(['One']); data.query({ term: '' + }, function () { + assert.ok(false, 'The results should not be queried'); }); - assert.ok(data.called); - - data = new MaximumSelectionData(null, maxOfOneOptions); - - data.trigger = function () { - assert.ok(true, 'The event should be triggered.'); - }; - - data.val('1'); + $select.val(['One', 'Two']); data.query({ term: '' + }, function () { + assert.ok(false, 'The results should not be queried'); }); - - assert.ok(!data.called); - }); -test('triggers when >= 2 selections' , function (assert) { - var maxOfTwoOptions = new Options({ - maximumSelectionLength: 2 - }); - var data = new MaximumSelectionData(null, maxOfTwoOptions); +test('triggers after selection' , function (assert) { + assert.expect(1); - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; + var $select = $('#qunit-fixture .multiple'); - data.query({ - term: '' + var maxOfOneOptions = new Options({ + maximumSelectionLength: 1 }); - assert.ok(data.called); + var container = new MockContainer(); + var data = new MaximumSelectionData($select, maxOfOneOptions); - data = new MaximumSelectionData(null, maxOfTwoOptions); + data.bind(container, null); - data.trigger = function () { - assert.ok(false, 'No events should be triggered'); - }; - - data.val('1'); - - data.query({ - term: '' + data.on('results:message', function () { + assert.ok(true, 'The message should be displayed'); }); - assert.ok(data.called); + $select.val(['One']); - data = new MaximumSelectionData(null, maxOfTwoOptions); - - data.trigger = function () { - assert.ok(true, 'The event should be triggered.'); - }; - - data.val('1'); - data.val('2'); - - data.query({ - term: '' + container.trigger('select', { + data: {} }); - - assert.ok(!data.called); - }); From 1f3eceba5a00f5cc71c831978f162a1d53c83906 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Sat, 27 Jul 2019 16:37:57 -0400 Subject: [PATCH 04/19] Fix generated options not receiving result IDs (#5586) In order to enable the ability to uniquely identify a result by an ID in the DOM, we generate a new ID for the result based on a combination of things, including the container ID prefix that is generated and used elsewhere in Select2. This has worked fairly well for use cases including attaching Select2 to an existing `` adapter so they properly bind to a container during the test. This was causing test failures becuase the options which would previously be generated during initialization were no longer appearing. Fixes #4350 --- src/js/select2/data/array.js | 10 +++++-- tests/data/array-tests.js | 54 +++++++++++++++++++++++++++++++++++ tests/data/inputData-tests.js | 15 ++++++++++ 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/js/select2/data/array.js b/src/js/select2/data/array.js index b8899662..a43c5a6d 100644 --- a/src/js/select2/data/array.js +++ b/src/js/select2/data/array.js @@ -4,15 +4,19 @@ define([ 'jquery' ], function (SelectAdapter, Utils, $) { function ArrayAdapter ($element, options) { - var data = options.get('data') || []; + this._dataToConvert = options.get('data') || []; ArrayAdapter.__super__.constructor.call(this, $element, options); - - this.addOptions(this.convertToOptions(data)); } Utils.Extend(ArrayAdapter, SelectAdapter); + ArrayAdapter.prototype.bind = function (container, $container) { + ArrayAdapter.__super__.bind.call(this, container, $container); + + this.addOptions(this.convertToOptions(this._dataToConvert)); + }; + ArrayAdapter.prototype.select = function (data) { var $option = this.$element.find('option').filter(function (i, elm) { return elm.value == data.id.toString(); diff --git a/tests/data/array-tests.js b/tests/data/array-tests.js index d0dead68..e0c8a266 100644 --- a/tests/data/array-tests.js +++ b/tests/data/array-tests.js @@ -71,6 +71,9 @@ test('current gets default for single', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + data.current(function (val) { assert.equal( val.length, @@ -93,6 +96,9 @@ test('current gets default for multiple', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + data.current(function (val) { assert.equal( val.length, @@ -107,6 +113,9 @@ test('current works with existing selections', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + $select.val(['One']); data.current(function (val) { @@ -137,6 +146,9 @@ test('current works with selected data', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + data.select({ id: '2', text: '2' @@ -170,6 +182,9 @@ test('select works for single', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + assert.equal( $select.val(), 'default', @@ -193,6 +208,9 @@ test('multiple sets the value', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + assert.ok( $select.val() == null || $select.val().length == 0, 'nothing should be selected' @@ -211,6 +229,9 @@ test('multiple adds to the old value', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + $select.val(['One']); assert.deepEqual($select.val(), ['One']); @@ -228,6 +249,9 @@ test('option tags are automatically generated', function (assert) { var data = new ArrayData($select, arrayOptions); + var container = new MockContainer(); + data.bind(container, $('
')); + assert.equal( $select.find('option').length, 4, @@ -235,11 +259,32 @@ test('option tags are automatically generated', function (assert) { ); }); +test('automatically generated option tags have a result id', function (assert) { + var $select = $('#qunit-fixture .single-empty'); + + var data = new ArrayData($select, arrayOptions); + + var container = new MockContainer(); + data.bind(container, $('
')); + + data.select({ + id: 'default' + }); + + assert.ok( + Utils.GetData($select.find(':selected')[0], 'data')._resultId, + '