Skip to content

Commit 1fe2eb9

Browse files
Menu: fix focus logic on submenu hiding (T1304251) (25_1) (#32014)
1 parent 44b97fe commit 1fe2eb9

File tree

2 files changed

+135
-68
lines changed

2 files changed

+135
-68
lines changed

packages/devextreme/js/__internal/ui/menu/menu.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ const ACTIONS = [
9898

9999
type MenuNode = InternalNode & Item;
100100
interface SubmenuVisibilityChangeEventParams {
101+
cancel?: boolean;
101102
itemData?: Item;
102103
rootItem: Element;
103104
submenuContainer: Element;
@@ -832,7 +833,7 @@ class Menu extends MenuBase<MenuProperties> {
832833
_submenuOnHidingHandler(
833834
$menuAnchorItem: dxElementWrapper,
834835
submenu: Submenu,
835-
eventArgs: SubmenuHidingEvent,
836+
eventArgs: SubmenuHidingEvent | SubmenuVisibilityChangeEventParams,
836837
): void {
837838
const $border = $menuAnchorItem.children(`.${DX_CONTEXT_MENU_CONTAINER_BORDER_CLASS}`);
838839
const params = this._getVisibilityChangeEventParams(
@@ -856,23 +857,29 @@ class Menu extends MenuBase<MenuProperties> {
856857

857858
this._actions.onSubmenuHiding?.(eventArgs);
858859

859-
const { focusedElement } = this.option();
860-
const { focusedElement: submenuFocusedElement } = submenu.option();
860+
if (eventArgs.cancel) {
861+
return;
862+
}
861863

862-
const isVisibleSubmenuHiding = this._visibleSubmenu === submenu;
863-
const isFocusedElementHiding = focusedElement === submenuFocusedElement;
864+
const { focusedElement } = this.option();
865+
const submenuContainerElement = $(eventArgs.submenuContainer).get(0);
866+
const focusedDomElement = $(focusedElement).get(0);
867+
const isFocusedElementInsideSubmenu = focusedDomElement && submenuContainerElement
868+
? submenuContainerElement.contains(focusedDomElement)
869+
: false;
864870

865-
if (isVisibleSubmenuHiding && isFocusedElementHiding) {
871+
if (isFocusedElementInsideSubmenu) {
866872
this.option('focusedElement', getPublicElement($menuAnchorItem));
867873
}
868874

869-
if (!eventArgs.cancel) {
870-
if (isVisibleSubmenuHiding) {
871-
this._visibleSubmenu = null;
872-
}
873-
$border.hide();
874-
$menuAnchorItem.removeClass(DX_MENU_ITEM_EXPANDED_CLASS);
875+
const isVisibleSubmenuHiding = this._visibleSubmenu === submenu;
876+
877+
if (isVisibleSubmenuHiding) {
878+
this._visibleSubmenu = null;
875879
}
880+
881+
$border.hide();
882+
$menuAnchorItem.removeClass(DX_MENU_ITEM_EXPANDED_CLASS);
876883
}
877884

878885
_submenuOnHiddenHandler(

packages/devextreme/testing/tests/DevExpress.ui.widgets/menu.tests.js

Lines changed: 116 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2393,6 +2393,122 @@ QUnit.module('Menu tests', {
23932393
submenu = getSubMenuInstance($rootMenuItem);
23942394
assert.ok(submenu.option('visible'), 'submenu shown');
23952395
});
2396+
2397+
QUnit.test('focusedElement should be set to main menu item after hiding submenu (T1291581)', function(assert) {
2398+
const menu = $('#menu').dxMenu({
2399+
orientation: 'horizontal',
2400+
focusStateEnabled: true,
2401+
items: [
2402+
{
2403+
text: 'Item 1',
2404+
items: [
2405+
{ text: 'Item 11', items: [ { text: 'Item 111' }, { text: 'Item 112' }, { text: 'Item 113' } ] },
2406+
{ text: 'Item 12' }
2407+
],
2408+
},
2409+
]
2410+
}).dxMenu('instance');
2411+
const keyboard = keyboardMock(menu.itemsContainer());
2412+
2413+
keyboard.press('enter')
2414+
.press('down')
2415+
.press('down');
2416+
2417+
assert.strictEqual($(menu.option('focusedElement')).text(), 'Item 12', 'focusedElement is submenu item');
2418+
2419+
keyboard.press('enter');
2420+
2421+
const mainMenuItemText = $(menu.itemElements()[0]).text();
2422+
2423+
assert.strictEqual($(menu.option('focusedElement')).text(), mainMenuItemText, 'focusedElement is main menu item');
2424+
});
2425+
2426+
QUnit.test('focusedElement should be set to main menu item after hiding nested submenu (T1291581)', function(assert) {
2427+
const menu = $('#menu').dxMenu({
2428+
orientation: 'horizontal',
2429+
focusStateEnabled: true,
2430+
items: [
2431+
{
2432+
text: 'Item 1',
2433+
items: [
2434+
{ text: 'Item 11', items: [ { text: 'Item 111' }, { text: 'Item 112' }, { text: 'Item 113' } ] },
2435+
{ text: 'Item 12' }
2436+
],
2437+
},
2438+
]
2439+
}).dxMenu('instance');
2440+
const keyboard = keyboardMock(menu.itemsContainer());
2441+
2442+
keyboard.press('enter')
2443+
.press('down')
2444+
.press('enter')
2445+
.press('right')
2446+
.press('down');
2447+
2448+
assert.strictEqual($(menu.option('focusedElement')).text(), 'Item 112', 'focusedElement is submenu item');
2449+
2450+
keyboard.press('enter');
2451+
2452+
const mainMenuItemText = $(menu.itemElements()[0]).text();
2453+
2454+
assert.strictEqual($(menu.option('focusedElement')).text(), mainMenuItemText, 'focusedElement is main menu item');
2455+
});
2456+
2457+
QUnit.test('focusedElement should not be moved if submenu hiding was cancelled (T1291581)', function(assert) {
2458+
const menu = $('#menu').dxMenu({
2459+
orientation: 'horizontal',
2460+
focusStateEnabled: true,
2461+
items: [
2462+
{
2463+
text: 'Item 1',
2464+
items: [
2465+
{ text: 'Item 11', items: [ { text: 'Item 111' }, { text: 'Item 112' }, { text: 'Item 113' } ] },
2466+
{ text: 'Item 12' }
2467+
],
2468+
},
2469+
],
2470+
onSubmenuHiding: function(e) {
2471+
e.cancel = true;
2472+
}
2473+
}).dxMenu('instance');
2474+
const keyboard = keyboardMock(menu.itemsContainer());
2475+
2476+
keyboard.press('enter')
2477+
.press('down')
2478+
.press('down');
2479+
2480+
const { focusedElement: initialFocusedElement } = menu.option();
2481+
2482+
keyboard.press('enter');
2483+
2484+
assert.strictEqual($(menu.option('focusedElement')).text(), 'Item 12', 'focusedElement is submenu item');
2485+
assert.strictEqual(menu.option('focusedElement'), initialFocusedElement, 'focusedElement not changed');
2486+
});
2487+
2488+
QUnit.test('focusedElement should not be set to main menu item after hiding nested submenu if no item was focused (T1304251)', function(assert) {
2489+
const menu = createMenu({
2490+
items: [{ text: 'Item 1', items: [{ text: 'Item 11' }, { text: 'Item 12' }, { text: 'Item 13' }] }],
2491+
showFirstSubmenuMode: { name: 'onHover', delay: 0 },
2492+
hideSubmenuOnMouseLeave: true
2493+
});
2494+
const $rootMenuItem = $(menu.element).find(`.${DX_MENU_ITEM_CLASS}`);
2495+
2496+
$(menu.element).trigger($.Event('dxhoverstart', { target: $rootMenuItem.get(0) }));
2497+
$($rootMenuItem).trigger('dxpointermove');
2498+
this.clock.tick(0);
2499+
2500+
const submenu = getSubMenuInstance($rootMenuItem);
2501+
const $item = $(submenu._overlay.content()).find(`.${DX_MENU_ITEM_CLASS}`);
2502+
2503+
$(menu.element).trigger($.Event('dxhoverstart', { target: $item.get(1) }));
2504+
$(menu.element).trigger($.Event('dxhoverend', { target: $item.get(1) }));
2505+
$(menu.element).trigger($.Event('dxhoverstart', { target: window }));
2506+
$($(submenu._overlay.content()).find('.dx-submenu')).trigger('dxhoverend');
2507+
this.clock.tick(0);
2508+
2509+
assert.strictEqual($rootMenuItem.eq(0).hasClass(DX_STATE_FOCUSED_CLASS), false, 'root menu item has not focused class');
2510+
assert.strictEqual(menu.instance.option('focusedElement'), null, 'menu focusedElement is null');
2511+
});
23962512
});
23972513

23982514
QUnit.module('keyboard navigation', {
@@ -2841,62 +2957,6 @@ QUnit.module('keyboard navigation', {
28412957

28422958
assert.equal($(this.instance._visibleSubmenu.option('focusedElement')).text(), 'Item 113');
28432959
});
2844-
2845-
QUnit.test('focusedElement should be set to main menu item after hiding submenu', function(assert) {
2846-
this.instance.option({
2847-
orientation: 'horizontal',
2848-
items: [
2849-
{
2850-
text: 'Item 1',
2851-
items: [
2852-
{ text: 'Item 11', items: [ { text: 'Item 111' }, { text: 'Item 112' }, { text: 'Item 113' } ] },
2853-
{ text: 'Item 12' }
2854-
],
2855-
},
2856-
]
2857-
});
2858-
2859-
this.keyboard.press('enter')
2860-
.press('down')
2861-
.press('down');
2862-
2863-
assert.strictEqual($(this.instance.option('focusedElement')).text(), 'Item 12', 'focusedElement is submenu item');
2864-
2865-
this.keyboard.press('enter');
2866-
2867-
const mainMenuItemText = $(this.instance.itemElements()[0]).text();
2868-
2869-
assert.strictEqual($(this.instance.option('focusedElement')).text(), mainMenuItemText, 'focusedElement is main menu item');
2870-
});
2871-
2872-
QUnit.test('focusedElement should be set to main menu item after hiding nested submenu', function(assert) {
2873-
this.instance.option({
2874-
orientation: 'horizontal',
2875-
items: [
2876-
{
2877-
text: 'Item 1',
2878-
items: [
2879-
{ text: 'Item 11', items: [ { text: 'Item 111' }, { text: 'Item 112' }, { text: 'Item 113' } ] },
2880-
{ text: 'Item 12' }
2881-
],
2882-
},
2883-
]
2884-
});
2885-
2886-
this.keyboard.press('enter')
2887-
.press('down')
2888-
.press('enter')
2889-
.press('right')
2890-
.press('down');
2891-
2892-
assert.strictEqual($(this.instance.option('focusedElement')).text(), 'Item 112', 'focusedElement is submenu item');
2893-
2894-
this.keyboard.press('enter');
2895-
2896-
const mainMenuItemText = $(this.instance.itemElements()[0]).text();
2897-
2898-
assert.strictEqual($(this.instance.option('focusedElement')).text(), mainMenuItemText, 'focusedElement is main menu item');
2899-
});
29002960
});
29012961

29022962
QUnit.module('Menu with templates', {

0 commit comments

Comments
 (0)