From b39b665072a2d36914e741b2c11620323924be89 Mon Sep 17 00:00:00 2001 From: alpadev <2838324+alpadev@users.noreply.github.com> Date: Sat, 22 May 2021 09:58:52 +0200 Subject: [PATCH] Automatically select an item in the dropdown when using arrow keys (#34052) --- .bundlewatch.config.json | 4 +-- js/src/dropdown.js | 21 +++++++-------- js/src/util/index.js | 4 +-- js/tests/unit/dropdown.spec.js | 45 +++++++++++++++++++++++++------- js/tests/unit/util/index.spec.js | 13 ++++++++- 5 files changed, 61 insertions(+), 26 deletions(-) diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index 81badf254c..988accd7f9 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -34,7 +34,7 @@ }, { "path": "./dist/js/bootstrap.bundle.js", - "maxSize": "41.25 kB" + "maxSize": "41.5 kB" }, { "path": "./dist/js/bootstrap.bundle.min.js", @@ -50,7 +50,7 @@ }, { "path": "./dist/js/bootstrap.js", - "maxSize": "27.25 kB" + "maxSize": "27.5 kB" }, { "path": "./dist/js/bootstrap.min.js", diff --git a/js/src/dropdown.js b/js/src/dropdown.js index cab2d018bb..34beb65129 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -354,18 +354,16 @@ class Dropdown extends BaseComponent { } } - _selectMenuItem(event) { - if (![ARROW_UP_KEY, ARROW_DOWN_KEY].includes(event.key)) { - return - } - + _selectMenuItem({ key, target }) { const items = SelectorEngine.find(SELECTOR_VISIBLE_ITEMS, this._menu).filter(isVisible) if (!items.length) { return } - getNextActiveElement(items, event.target, event.key === ARROW_DOWN_KEY, false).focus() + // if target isn't included in items (e.g. when expanding the dropdown) + // allow cycling to get the last item in case key equals ARROW_UP_KEY + getNextActiveElement(items, target, key === ARROW_DOWN_KEY, !items.includes(target)).focus() } // Static @@ -480,17 +478,18 @@ class Dropdown extends BaseComponent { return } - if (!isActive && (event.key === ARROW_UP_KEY || event.key === ARROW_DOWN_KEY)) { - getToggleButton().click() + if (event.key === ARROW_UP_KEY || event.key === ARROW_DOWN_KEY) { + if (!isActive) { + getToggleButton().click() + } + + Dropdown.getInstance(getToggleButton())._selectMenuItem(event) return } if (!isActive || event.key === SPACE_KEY) { Dropdown.clearMenus() - return } - - Dropdown.getInstance(getToggleButton())._selectMenuItem(event) } } diff --git a/js/src/util/index.js b/js/src/util/index.js index 77bdc072fc..4d077b21f9 100644 --- a/js/src/util/index.js +++ b/js/src/util/index.js @@ -264,9 +264,9 @@ const execute = callback => { const getNextActiveElement = (list, activeElement, shouldGetNext, isCycleAllowed) => { let index = list.indexOf(activeElement) - // if the element does not exist in the list initialize it as the first element + // if the element does not exist in the list return an element depending on the direction and if cycle is allowed if (index === -1) { - return list[0] + return list[!shouldGetNext && isCycleAllowed ? list.length - 1 : 0] } const listLength = list.length diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index 5275f1a556..390cddfbfa 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -1561,7 +1561,7 @@ describe('Dropdown', () => { triggerDropdown.click() }) - it('should focus on the first element when using ArrowUp for the first time', done => { + it('should open the dropdown and focus on the last item when using ArrowUp for the first time', done => { fixtureEl.innerHTML = [ '
', ' ', @@ -1573,19 +1573,44 @@ describe('Dropdown', () => { ].join('') const triggerDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]') - const item1 = fixtureEl.querySelector('#item1') + const lastItem = fixtureEl.querySelector('#item2') triggerDropdown.addEventListener('shown.bs.dropdown', () => { - const keydown = createEvent('keydown') - keydown.key = 'ArrowUp' - - document.activeElement.dispatchEvent(keydown) - expect(document.activeElement).toEqual(item1, 'item1 is focused') - - done() + setTimeout(() => { + expect(document.activeElement).toEqual(lastItem, 'item2 is focused') + done() + }) }) - triggerDropdown.click() + const keydown = createEvent('keydown') + keydown.key = 'ArrowUp' + triggerDropdown.dispatchEvent(keydown) + }) + + it('should open the dropdown and focus on the first item when using ArrowDown for the first time', done => { + fixtureEl.innerHTML = [ + '
', + ' ', + '
', + ' A link', + ' Another link', + '
', + '
' + ].join('') + + const triggerDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]') + const firstItem = fixtureEl.querySelector('#item1') + + triggerDropdown.addEventListener('shown.bs.dropdown', () => { + setTimeout(() => { + expect(document.activeElement).toEqual(firstItem, 'item1 is focused') + done() + }) + }) + + const keydown = createEvent('keydown') + keydown.key = 'ArrowDown' + triggerDropdown.dispatchEvent(keydown) }) it('should not close the dropdown if the user clicks on a text field within dropdown-menu', done => { diff --git a/js/tests/unit/util/index.spec.js b/js/tests/unit/util/index.spec.js index 774945d1f9..737ecacfde 100644 --- a/js/tests/unit/util/index.spec.js +++ b/js/tests/unit/util/index.spec.js @@ -661,11 +661,22 @@ describe('Util', () => { }) describe('getNextActiveElement', () => { - it('should return first element if active not exists or not given', () => { + it('should return first element if active not exists or not given and shouldGetNext is either true, or false with cycling being disabled', () => { const array = ['a', 'b', 'c', 'd'] expect(Util.getNextActiveElement(array, '', true, true)).toEqual('a') expect(Util.getNextActiveElement(array, 'g', true, true)).toEqual('a') + expect(Util.getNextActiveElement(array, '', true, false)).toEqual('a') + expect(Util.getNextActiveElement(array, 'g', true, false)).toEqual('a') + expect(Util.getNextActiveElement(array, '', false, false)).toEqual('a') + expect(Util.getNextActiveElement(array, 'g', false, false)).toEqual('a') + }) + + it('should return last element if active not exists or not given and shouldGetNext is false but cycling is enabled', () => { + const array = ['a', 'b', 'c', 'd'] + + expect(Util.getNextActiveElement(array, '', false, true)).toEqual('d') + expect(Util.getNextActiveElement(array, 'g', false, true)).toEqual('d') }) it('should return next element or same if is last', () => {