Skip to content

Commit 3d569f8

Browse files
committed
Fix v6 menu keyboard navigation for nested submenus
Include submenu triggers when collecting keyboard-navigable items so arrow keys work with the supported .menu > .submenu > .menu-item markup. When focus moves to another item at the same level, close open sibling submenus that no longer relate to focus (aligned with click/hover behavior).
1 parent c6eaf58 commit 3d569f8

2 files changed

Lines changed: 132 additions & 5 deletions

File tree

js/src/menu.js

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -784,16 +784,46 @@ class Menu extends BaseComponent {
784784
// Keyboard navigation
785785
// -------------------------------------------------------------------------
786786

787+
_getItemsInMenu(menu) {
788+
// Items may be direct children of `.menu`, or submenu triggers nested as
789+
// `.menu > .submenu > .menu-item` (see menu-submenu visual tests / docs).
790+
return SelectorEngine.find(
791+
`:scope > ${SELECTOR_VISIBLE_ITEMS}, :scope > ${SELECTOR_SUBMENU_TOGGLE}`,
792+
menu
793+
).filter(element => isVisible(element))
794+
}
795+
787796
_selectMenuItem({ key, target }) {
788797
const currentMenu = target.closest(SELECTOR_MENU) || this._menu
789-
const items = SelectorEngine.find(`:scope > ${SELECTOR_VISIBLE_ITEMS}`, currentMenu)
790-
.filter(element => isVisible(element))
798+
const items = this._getItemsInMenu(currentMenu)
791799

792800
if (!items.length) {
793801
return
794802
}
795803

796-
getNextActiveElement(items, target, key === ARROW_DOWN_KEY, !items.includes(target)).focus()
804+
const nextItem = getNextActiveElement(items, target, key === ARROW_DOWN_KEY, !items.includes(target))
805+
nextItem.focus()
806+
807+
// Arrowing between items at this level must close open sibling submenus that
808+
// no longer relate to focus (mouse/click already do this via _closeSiblingSubmenus).
809+
this._closeUnrelatedSubmenus(currentMenu, nextItem)
810+
}
811+
812+
_closeUnrelatedSubmenus(currentMenu, focusedElement) {
813+
for (const [submenu] of this._openSubmenus) {
814+
const submenuWrapper = submenu.closest(SELECTOR_SUBMENU)
815+
// Only consider submenus that are direct children of the menu being navigated
816+
if (!submenuWrapper || submenuWrapper.parentElement !== currentMenu) {
817+
continue
818+
}
819+
820+
// Keep open when focus is on that submenu's trigger or still inside it
821+
if (submenuWrapper.contains(focusedElement)) {
822+
continue
823+
}
824+
825+
this._closeSubmenu(submenu, submenuWrapper)
826+
}
797827
}
798828

799829
_handleSubmenuKeydown(event) {
@@ -867,12 +897,12 @@ class Menu extends BaseComponent {
867897
event.stopPropagation()
868898

869899
const currentMenu = target.closest(SELECTOR_MENU)
870-
const items = SelectorEngine.find(`:scope > ${SELECTOR_VISIBLE_ITEMS}`, currentMenu)
871-
.filter(element => isVisible(element))
900+
const items = this._getItemsInMenu(currentMenu)
872901

873902
if (items.length) {
874903
const targetItem = key === HOME_KEY ? items[0] : items.at(-1)
875904
targetItem.focus()
905+
this._closeUnrelatedSubmenus(currentMenu, targetItem)
876906
}
877907

878908
return true

js/tests/unit/menu.spec.js

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,6 +2765,103 @@ describe('Menu', () => {
27652765
})
27662766
})
27672767

2768+
it('should close open sibling submenu when keyboard focus moves to another item', () => {
2769+
return new Promise(resolve => {
2770+
// Match the supported markup from visual/menu-submenu.html
2771+
fixtureEl.innerHTML = [
2772+
'<div>',
2773+
' <button class="btn" data-bs-toggle="menu">Menu</button>',
2774+
' <div class="menu">',
2775+
' <div class="submenu" id="submenu1">',
2776+
' <button class="menu-item" type="button">Submenu 1</button>',
2777+
' <div class="menu">',
2778+
' <a class="menu-item" href="#">Action 1</a>',
2779+
' </div>',
2780+
' </div>',
2781+
' <div class="submenu" id="submenu2">',
2782+
' <button class="menu-item" type="button">Submenu 2</button>',
2783+
' <div class="menu">',
2784+
' <a class="menu-item" href="#">Action 2</a>',
2785+
' </div>',
2786+
' </div>',
2787+
' </div>',
2788+
'</div>'
2789+
].join('')
2790+
2791+
const btnMenu = fixtureEl.querySelector('[data-bs-toggle="menu"]')
2792+
const submenu1Wrapper = fixtureEl.querySelector('#submenu1')
2793+
const submenu2Wrapper = fixtureEl.querySelector('#submenu2')
2794+
const submenu1Trigger = submenu1Wrapper.querySelector(':scope > .menu-item')
2795+
const submenu2Trigger = submenu2Wrapper.querySelector(':scope > .menu-item')
2796+
const submenu1 = submenu1Wrapper.querySelector('.menu')
2797+
const submenu2 = submenu2Wrapper.querySelector('.menu')
2798+
2799+
btnMenu.addEventListener('shown.bs.menu', () => {
2800+
// Open first submenu via click (same as mouse path)
2801+
submenu1Trigger.click()
2802+
expect(submenu1.classList.contains('show')).toBeTrue()
2803+
2804+
// Move keyboard focus to the other sibling submenu trigger
2805+
submenu1Trigger.focus()
2806+
const keydown = createEvent('keydown', { bubbles: true })
2807+
keydown.key = 'ArrowDown'
2808+
submenu1Trigger.dispatchEvent(keydown)
2809+
2810+
expect(document.activeElement).toEqual(submenu2Trigger)
2811+
expect(submenu1.classList.contains('show')).toBeFalse()
2812+
expect(submenu2.classList.contains('show')).toBeFalse()
2813+
resolve()
2814+
})
2815+
2816+
// eslint-disable-next-line no-new
2817+
new Menu(btnMenu)
2818+
btnMenu.click()
2819+
})
2820+
})
2821+
2822+
it('should keyboard-navigate between submenu triggers at the top level', () => {
2823+
return new Promise(resolve => {
2824+
fixtureEl.innerHTML = [
2825+
'<div>',
2826+
' <button class="btn" data-bs-toggle="menu">Menu</button>',
2827+
' <div class="menu">',
2828+
' <div class="submenu" id="submenu1">',
2829+
' <button class="menu-item" type="button">Submenu 1</button>',
2830+
' <div class="menu"><a class="menu-item" href="#">Action 1</a></div>',
2831+
' </div>',
2832+
' <a id="plainItem" class="menu-item" href="#">Plain item</a>',
2833+
' <div class="submenu" id="submenu2">',
2834+
' <button class="menu-item" type="button">Submenu 2</button>',
2835+
' <div class="menu"><a class="menu-item" href="#">Action 2</a></div>',
2836+
' </div>',
2837+
' </div>',
2838+
'</div>'
2839+
].join('')
2840+
2841+
const btnMenu = fixtureEl.querySelector('[data-bs-toggle="menu"]')
2842+
const submenu1Trigger = fixtureEl.querySelector('#submenu1 > .menu-item')
2843+
const plainItem = fixtureEl.querySelector('#plainItem')
2844+
const submenu2Trigger = fixtureEl.querySelector('#submenu2 > .menu-item')
2845+
2846+
btnMenu.addEventListener('shown.bs.menu', () => {
2847+
submenu1Trigger.focus()
2848+
2849+
const keydown = createEvent('keydown', { bubbles: true })
2850+
keydown.key = 'ArrowDown'
2851+
submenu1Trigger.dispatchEvent(keydown)
2852+
expect(document.activeElement).toEqual(plainItem)
2853+
2854+
plainItem.dispatchEvent(keydown)
2855+
expect(document.activeElement).toEqual(submenu2Trigger)
2856+
resolve()
2857+
})
2858+
2859+
// eslint-disable-next-line no-new
2860+
new Menu(btnMenu)
2861+
btnMenu.click()
2862+
})
2863+
})
2864+
27682865
it('should open submenu with ArrowRight key', () => {
27692866
return new Promise(resolve => {
27702867
fixtureEl.innerHTML = [

0 commit comments

Comments
 (0)