From ab2fc63d08b8c53d6f29bcfd73b7f2d5ceaacacd Mon Sep 17 00:00:00 2001 From: Pierre-Denis Vanduynslager Date: Sun, 22 May 2016 02:16:27 -0400 Subject: [PATCH 01/10] Dropdown: remove dependency to role="menu", role="listbox" a and li elements => fix keyboard navigation --- js/src/dropdown.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 92f841bc43..a9786a5347 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -44,11 +44,9 @@ const Dropdown = (($) => { BACKDROP : '.dropdown-backdrop', DATA_TOGGLE : '[data-toggle="dropdown"]', FORM_CHILD : '.dropdown form', - ROLE_MENU : '[role="menu"]', - ROLE_LISTBOX : '[role="listbox"]', + MENU : '.dropdown-menu', NAVBAR_NAV : '.navbar-nav', - VISIBLE_ITEMS : '[role="menu"] li:not(.disabled) a, ' - + '[role="listbox"] li:not(.disabled) a' + VISIBLE_ITEMS : '.dropdown-menu .dropdown-item:not(.disabled)' } @@ -268,8 +266,7 @@ const Dropdown = (($) => { $(document) .on(Event.KEYDOWN_DATA_API, Selector.DATA_TOGGLE, Dropdown._dataApiKeydownHandler) - .on(Event.KEYDOWN_DATA_API, Selector.ROLE_MENU, Dropdown._dataApiKeydownHandler) - .on(Event.KEYDOWN_DATA_API, Selector.ROLE_LISTBOX, Dropdown._dataApiKeydownHandler) + .on(Event.KEYDOWN_DATA_API, Selector.MENU, Dropdown._dataApiKeydownHandler) .on(Event.CLICK_DATA_API, Dropdown._clearMenus) .on(Event.CLICK_DATA_API, Selector.DATA_TOGGLE, Dropdown.prototype.toggle) .on(Event.CLICK_DATA_API, Selector.FORM_CHILD, (e) => { From 67958f35e80af425c08fa367e8a6f1eeefb830bc Mon Sep 17 00:00:00 2001 From: Pierre-Denis Vanduynslager Date: Wed, 4 Jan 2017 12:24:33 -0500 Subject: [PATCH 02/10] Merge conflict --- js/src/dropdown.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 46287fb905..873c106a7b 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -272,14 +272,8 @@ const Dropdown = (($) => { $(document) .on(Event.KEYDOWN_DATA_API, Selector.DATA_TOGGLE, Dropdown._dataApiKeydownHandler) -<<<<<<< HEAD .on(Event.KEYDOWN_DATA_API, Selector.MENU, Dropdown._dataApiKeydownHandler) - .on(Event.CLICK_DATA_API, Dropdown._clearMenus) -======= - .on(Event.KEYDOWN_DATA_API, Selector.ROLE_MENU, Dropdown._dataApiKeydownHandler) - .on(Event.KEYDOWN_DATA_API, Selector.ROLE_LISTBOX, Dropdown._dataApiKeydownHandler) .on(`${Event.CLICK_DATA_API} ${Event.FOCUSIN_DATA_API}`, Dropdown._clearMenus) ->>>>>>> twbs/v4-dev .on(Event.CLICK_DATA_API, Selector.DATA_TOGGLE, Dropdown.prototype.toggle) .on(Event.CLICK_DATA_API, Selector.FORM_CHILD, (e) => { e.stopPropagation() From eb84c03ddf5993d701724250507055b4e394fbdb Mon Sep 17 00:00:00 2001 From: Pierre-Denis Vanduynslager Date: Fri, 6 Jan 2017 15:56:38 -0500 Subject: [PATCH 03/10] Update dropdown unit test to new markup --- js/tests/unit/dropdown.js | 273 +++++++++++++++++++------------------- 1 file changed, 136 insertions(+), 137 deletions(-) diff --git a/js/tests/unit/dropdown.js b/js/tests/unit/dropdown.js index 9a34fc0c93..69428b6101 100644 --- a/js/tests/unit/dropdown.js +++ b/js/tests/unit/dropdown.js @@ -46,17 +46,17 @@ $(function () { QUnit.test('should not open dropdown if target is disabled via attribute', function (assert) { assert.expect(1) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') @@ -64,17 +64,17 @@ $(function () { QUnit.test('should set aria-expanded="true" on target when dropdown menu is shown', function (assert) { assert.expect(1) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML) .find('[data-toggle="dropdown"]') .bootstrapDropdown() @@ -86,17 +86,17 @@ $(function () { QUnit.test('should set aria-expanded="false" on target when dropdown menu is hidden', function (assert) { assert.expect(1) var done = assert.async() - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML) .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') @@ -115,17 +115,17 @@ $(function () { QUnit.test('should not open dropdown if target is disabled via class', function (assert) { assert.expect(1) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') @@ -133,17 +133,17 @@ $(function () { QUnit.test('should add class show to menu if clicked', function (assert) { assert.expect(1) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') @@ -151,17 +151,17 @@ $(function () { QUnit.test('should test if element has a # before assuming it\'s a selector', function (assert) { assert.expect(1) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') @@ -170,17 +170,17 @@ $(function () { QUnit.test('should remove "show" class if body is clicked', function (assert) { assert.expect(2) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML) .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') @@ -218,21 +218,20 @@ $(function () { QUnit.test('should remove "show" class if body is clicked, with multiple dropdowns', function (assert) { assert.expect(7) - var dropdownHTML = '' + + '
' + + 'Submenu 1' + + '
' + + '' + + '' + '
' + '' + '' - + '' + + '
' + + 'Action 1' + + '
' + '
' var $dropdowns = $(dropdownHTML).appendTo('#qunit-fixture').find('[data-toggle="dropdown"]') var $first = $dropdowns.first() @@ -291,17 +290,17 @@ $(function () { QUnit.test('should fire show and hide event', function (assert) { assert.expect(2) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML) .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') @@ -326,17 +325,17 @@ $(function () { QUnit.test('should fire shown and hidden event', function (assert) { assert.expect(2) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML) .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') @@ -360,17 +359,17 @@ $(function () { QUnit.test('should fire shown and hidden event with a relatedTarget', function (assert) { assert.expect(2) - var dropdownHTML = '' + + '
' + + 'Secondary link' + + 'Something else here' + + '
' + + 'Another link' + + '
' + + '
' + + '' var $dropdown = $(dropdownHTML) .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') @@ -394,19 +393,19 @@ $(function () { assert.expect(3) var done = assert.async() - var dropdownHTML = '
    ' - + '
  • ' + var dropdownHTML = '
    ' + + '
    ' + 'Dropdown' - + '
      ' - + '
    • Secondary link
      • ' - + '
    • Something else here
      • ' - + '
    • ' - + '
    • Another link
      • ' - + '
      • ' - + '
      • ' - + '
    ' + + '
    ' + + '' + + '
    ' + '
    ' var $dropdown = $(dropdownHTML) .appendTo('#qunit-fixture') From 9daf681478418a2f5d3a3a9864facd4838dfb69f Mon Sep 17 00:00:00 2001 From: Pierre-Denis Vanduynslager Date: Fri, 20 Jan 2017 20:27:14 -0500 Subject: [PATCH 04/10] Fix test to retrieve the proper item instead of it's parent --- js/tests/unit/dropdown.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/tests/unit/dropdown.js b/js/tests/unit/dropdown.js index 69428b6101..fc418f5211 100644 --- a/js/tests/unit/dropdown.js +++ b/js/tests/unit/dropdown.js @@ -451,7 +451,7 @@ $(function () { $dropdown.trigger($.Event('keydown', { which: 40 })) $dropdown.trigger($.Event('keydown', { which: 40 })) - assert.ok(!$(document.activeElement).parent().is('.disabled'), '.disabled is not focused') + assert.ok(!$(document.activeElement).is('.disabled'), '.disabled is not focused') }) QUnit.test('should not close the dropdown if the user clicks on a text field', function (assert) { From 1a783a42554e4bd0de8ba26c89aeed8cc717c91c Mon Sep 17 00:00:00 2001 From: Pierre-Denis Vanduynslager Date: Fri, 20 Jan 2017 23:02:19 -0500 Subject: [PATCH 05/10] Add unit test for keyboard navigation --- js/tests/unit/dropdown.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/js/tests/unit/dropdown.js b/js/tests/unit/dropdown.js index fc418f5211..f5142d8b75 100644 --- a/js/tests/unit/dropdown.js +++ b/js/tests/unit/dropdown.js @@ -450,6 +450,40 @@ $(function () { $dropdown.trigger($.Event('keydown', { which: 40 })) $dropdown.trigger($.Event('keydown', { which: 40 })) + }) + + QUnit.test('should focus next/previous element when using keyboard navigation', function (assert) { + assert.expect(4) + var done = assert.async() + var dropdownHTML = '
    ' + + '
    ' + + 'Dropdown' + + '
    ' + + 'A link' + + 'Another link' + + '
    ' + + '
    ' + + '
    ' + var $dropdown = $(dropdownHTML) + .appendTo('#qunit-fixture') + .find('[data-toggle="dropdown"]') + .bootstrapDropdown() + + $dropdown + .parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.ok(true, 'shown was fired') + $dropdown.trigger($.Event('keydown', { which: 40 })) + assert.ok($(document.activeElement)[0] === $('#item1')[0], 'item1 is focused') + + $(document.activeElement).trigger($.Event('keydown', { which: 40 })) + assert.ok($(document.activeElement)[0] === $('#item2')[0], 'item2 is focused') + + $(document.activeElement).trigger($.Event('keydown', { which: 38 })) + assert.ok($(document.activeElement)[0] === $('#item1')[0], 'item1 is focused') + done() + }) + $dropdown.trigger('click') assert.ok(!$(document.activeElement).is('.disabled'), '.disabled is not focused') }) From 4448ca057b4a4e76155bc77aa96aebb6f332fdd7 Mon Sep 17 00:00:00 2001 From: Pierre-Denis Vanduynslager Date: Fri, 20 Jan 2017 23:02:45 -0500 Subject: [PATCH 06/10] Refactor test to event and async --- js/tests/unit/dropdown.js | 205 ++++++++++++++++++++++++++------------ 1 file changed, 141 insertions(+), 64 deletions(-) diff --git a/js/tests/unit/dropdown.js b/js/tests/unit/dropdown.js index f5142d8b75..fc1bf9931d 100644 --- a/js/tests/unit/dropdown.js +++ b/js/tests/unit/dropdown.js @@ -45,7 +45,7 @@ $(function () { }) QUnit.test('should not open dropdown if target is disabled via attribute', function (assert) { - assert.expect(1) + assert.expect(0) var dropdownHTML = '
    ' + '
    ' + '' @@ -57,13 +57,15 @@ $(function () { + '
    ' + '
    ' + '
    ' - var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') - - assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown() + setTimeout(function () { + assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + }, 300) }) QUnit.test('should set aria-expanded="true" on target when dropdown menu is shown', function (assert) { assert.expect(1) + var done = assert.async() var dropdownHTML = '
    ' + '
    ' + '' @@ -75,12 +77,14 @@ $(function () { + '
    ' + '
    ' + '' - var $dropdown = $(dropdownHTML) - .find('[data-toggle="dropdown"]') - .bootstrapDropdown() - .trigger('click') - - assert.strictEqual($dropdown.attr('aria-expanded'), 'true', 'aria-expanded is set to string "true" on click') + var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown() + $dropdown + .parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.strictEqual($dropdown.attr('aria-expanded'), 'true', 'aria-expanded is set to string "true" on click') + done() + }) + $dropdown.trigger('click') }) QUnit.test('should set aria-expanded="false" on target when dropdown menu is hidden', function (assert) { @@ -114,7 +118,7 @@ $(function () { }) QUnit.test('should not open dropdown if target is disabled via class', function (assert) { - assert.expect(1) + assert.expect(0) var dropdownHTML = '
    ' + '
    ' + '' @@ -127,12 +131,14 @@ $(function () { + '
    ' + '
    ' var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') - - assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + setTimeout(function () { + assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + }, 300) }) QUnit.test('should add class show to menu if clicked', function (assert) { assert.expect(1) + var done = assert.async() var dropdownHTML = '
    ' + '
    ' + 'Dropdown' @@ -144,13 +150,19 @@ $(function () { + '
    ' + '
    ' + '' - var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') - - assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown() + $dropdown + .parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + done() + }) + $dropdown.trigger('click') }) QUnit.test('should test if element has a # before assuming it\'s a selector', function (assert) { assert.expect(1) + var done = assert.async() var dropdownHTML = '
    ' + '
    ' + 'Dropdown' @@ -162,14 +174,20 @@ $(function () { + '
    ' + '
    ' + '' - var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click') - - assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown() + $dropdown + .parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + done() + }) + $dropdown.trigger('click') }) QUnit.test('should remove "show" class if body is clicked', function (assert) { assert.expect(2) + var done = assert.async() var dropdownHTML = '
    ' + '
    ' + 'Dropdown' @@ -185,15 +203,22 @@ $(function () { .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') .bootstrapDropdown() - .trigger('click') - assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') - $(document.body).trigger('click') - assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class removed') + $dropdown + .parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + $(document.body).trigger('click') + }).on('hidden.bs.dropdown', function () { + assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class removed') + done() + }) + $dropdown.trigger('click') }) QUnit.test('should remove "show" class if body is focused', function (assert) { assert.expect(2) + var done = assert.async() var dropdownHTML = '
    ' + '
    ' + 'Dropdown' @@ -209,15 +234,21 @@ $(function () { .appendTo('#qunit-fixture') .find('[data-toggle="dropdown"]') .bootstrapDropdown() - .trigger('click') - - assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') - $(document.body).trigger('focusin') - assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class removed') + $dropdown + .parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class added on click') + $(document.body).trigger('focusin') + }).on('hidden.bs.dropdown', function () { + assert.ok(!$dropdown.parent('.dropdown').hasClass('show'), '"show" class removed') + done() + }) + $dropdown.trigger('click') }) QUnit.test('should remove "show" class if body is clicked, with multiple dropdowns', function (assert) { assert.expect(7) + var done = assert.async() var dropdownHTML = '
    ' + '' + '
    ' + '' - + '' + + '' + '
    ' + 'Action 1' + '
    ' @@ -239,21 +270,31 @@ $(function () { assert.strictEqual($dropdowns.length, 2, 'two dropdowns') - $first.trigger('click') - assert.strictEqual($first.parents('.show').length, 1, '"show" class added on click') - assert.strictEqual($('#qunit-fixture .show').length, 1, 'only one dropdown is shown') - $(document.body).trigger('click') - assert.strictEqual($('#qunit-fixture .show').length, 0, '"show" class removed') + $first.parent('.dropdown') + .on('shown.bs.dropdown', function () { + assert.strictEqual($first.parents('.show').length, 1, '"show" class added on click') + assert.strictEqual($('#qunit-fixture .show').length, 1, 'only one dropdown is shown') + $(document.body).trigger('click') + }).on('hidden.bs.dropdown', function () { + assert.strictEqual($('#qunit-fixture .show').length, 0, '"show" class removed') + $last.trigger('click') + }) - $last.trigger('click') - assert.strictEqual($last.parent('.show').length, 1, '"show" class added on click') - assert.strictEqual($('#qunit-fixture .show').length, 1, 'only one dropdown is shown') - $(document.body).trigger('click') - assert.strictEqual($('#qunit-fixture .show').length, 0, '"show" class removed') + $last.parent('.btn-group') + .on('shown.bs.dropdown', function () { + assert.strictEqual($last.parent('.show').length, 1, '"show" class added on click') + assert.strictEqual($('#qunit-fixture .show').length, 1, 'only one dropdown is shown') + $(document.body).trigger('click') + }).on('hidden.bs.dropdown', function () { + assert.strictEqual($('#qunit-fixture .show').length, 0, '"show" class removed') + done() + }) + $first.trigger('click') }) QUnit.test('should remove "show" class if body is focused, with multiple dropdowns', function (assert) { assert.expect(7) + var done = assert.async() var dropdownHTML = '
    ' + '