Skip to content

Commit dd13e84

Browse files
julearijuleari
authored
Toolbar: close menu when a focused item is selected via the enter/space key (T1301705) (DevExpress#30750)
1 parent f68f9c6 commit dd13e84

File tree

4 files changed

+202
-6
lines changed

4 files changed

+202
-6
lines changed

‎packages/devextreme/js/__internal/ui/toolbar/internal/m_toolbar.menu.list.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { each } from '@js/core/utils/iterator';
44
import type { Item } from '@js/ui/toolbar';
55
import { ListBase } from '@ts/ui/list/m_list.base';
66

7-
const TOOLBAR_MENU_ACTION_CLASS = 'dx-toolbar-menu-action';
7+
export const TOOLBAR_MENU_ACTION_CLASS = 'dx-toolbar-menu-action';
88
const TOOLBAR_HIDDEN_BUTTON_CLASS = 'dx-toolbar-hidden-button';
99
const TOOLBAR_HIDDEN_BUTTON_GROUP_CLASS = 'dx-toolbar-hidden-button-group';
1010
const TOOLBAR_MENU_SECTION_CLASS = 'dx-toolbar-menu-section';

‎packages/devextreme/js/__internal/ui/toolbar/internal/m_toolbar.menu.ts‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import Widget from '@ts/core/widget/widget';
1818
import type { ListBase } from '@ts/ui/list/m_list.base';
1919

2020
import { toggleItemFocusableElementTabIndex } from '../m_toolbar.utils';
21-
import ToolbarMenuList from './m_toolbar.menu.list';
21+
import ToolbarMenuList, { TOOLBAR_MENU_ACTION_CLASS } from './m_toolbar.menu.list';
2222

2323
const DROP_DOWN_MENU_CLASS = 'dx-dropdownmenu';
2424
const DROP_DOWN_MENU_POPUP_CLASS = 'dx-dropdownmenu-popup';
@@ -264,8 +264,21 @@ export default class DropDownMenu extends Widget<DropDownMenuProperties> {
264264
dragEnabled: false,
265265
showTitle: false,
266266
fullScreen: false,
267+
ignoreChildEvents: false,
267268
_fixWrapperPosition: true,
268269
});
270+
this._popup.registerKeyHandler('space', (e) => {
271+
this._popupKeyHandler(e);
272+
});
273+
this._popup.registerKeyHandler('enter', (e) => {
274+
this._popupKeyHandler(e);
275+
});
276+
this._popup.registerKeyHandler('escape', (e): void => {
277+
// @ts-expect-error
278+
if (this._popup?.$overlayContent().is($(e.target))) {
279+
this.option('opened', false);
280+
}
281+
});
269282
}
270283

271284
_getMaxHeight() {
@@ -298,10 +311,7 @@ export default class DropDownMenu extends Widget<DropDownMenuProperties> {
298311
noDataText: '',
299312
itemTemplate,
300313
onItemClick: (e) => {
301-
if (this.option('closeOnClick')) {
302-
this.option('opened', false);
303-
}
304-
this._itemClickAction(e);
314+
this._itemClickHandler(e);
305315
},
306316
tabIndex: -1,
307317
focusStateEnabled: false,
@@ -312,6 +322,23 @@ export default class DropDownMenu extends Widget<DropDownMenuProperties> {
312322
});
313323
}
314324

325+
_popupKeyHandler(e): void {
326+
if ($(e.target).closest(`.${TOOLBAR_MENU_ACTION_CLASS}`).length) {
327+
this._closePopup();
328+
}
329+
}
330+
331+
_closePopup(): void {
332+
if (this.option('closeOnClick')) {
333+
this.option('opened', false);
334+
}
335+
}
336+
337+
_itemClickHandler(e): void {
338+
this._closePopup();
339+
this._itemClickAction?.(e);
340+
}
341+
315342
_itemOptionChanged(item, property, value): void {
316343
this._list?._itemOptionChanged(item, property, value);
317344
toggleItemFocusableElementTabIndex(this._list, item);

‎packages/devextreme/testing/helpers/device.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import devices from '__internal/core/m_devices';
2+
3+
export const shouldSkipOnDevices = ({
4+
deviceTypes = [],
5+
assert = null,
6+
message = null
7+
} = {}) => {
8+
if(!Array.isArray(deviceTypes)) {
9+
throw new Error('deviceTypes should be an array');
10+
}
11+
if(deviceTypes.includes(devices.real().deviceType)) {
12+
assert && assert.ok(true, message || `Test skipped on devices: ${deviceTypes.toString()}`);
13+
return true;
14+
}
15+
return false;
16+
};
17+
18+
export const shouldSkipOnDesktop = (assert = null, message = null) => shouldSkipOnDevices({
19+
deviceTypes: ['desktop'],
20+
assert,
21+
message,
22+
});
23+
24+
export const shouldSkipOnPhone = (assert = null, message = null) => shouldSkipOnDevices({
25+
deviceTypes: ['phone'],
26+
assert,
27+
message,
28+
});
29+
30+
export const shouldSkipOnMobile = (assert = null, message = null) => shouldSkipOnDevices({
31+
deviceTypes: ['phone', 'tablet'],
32+
assert,
33+
message,
34+
});

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import $ from 'jquery';
33
import ArrayStore from 'common/data/array_store';
44
import fx from 'common/core/animation/fx';
55
import Button from 'ui/button';
6+
import 'ui/check_box';
67
import Popup from 'ui/popup';
78
import DropDownMenu from '__internal/ui/toolbar/internal/m_toolbar.menu';
89
import ToolbarMenuList from '__internal/ui/toolbar/internal/m_toolbar.menu.list';
@@ -13,6 +14,7 @@ import config from 'core/config';
1314
import { DataSource } from 'common/data/data_source/data_source';
1415
import { isRenderer } from 'core/utils/type';
1516
import themes from 'ui/themes';
17+
import { shouldSkipOnMobile } from '../../helpers/device.js';
1618

1719
import 'generic_light.css!';
1820

@@ -32,6 +34,8 @@ const DROP_DOWN_MENU_POPUP_CLASS = 'dx-dropdownmenu-popup';
3234
const DROP_DOWN_MENU_POPUP_WRAPPER_CLASS = 'dx-dropdownmenu-popup-wrapper';
3335
const LIST_ITEM_CLASS = 'dx-list-item';
3436
const SCROLLVIEW_CONTENT_CLASS = 'dx-scrollview-content';
37+
const CHECK_BOX_CLASS = 'dx-checkbox';
38+
const BUTTON_CLASS = 'dx-button';
3539

3640

3741
const moduleConfig = {
@@ -595,6 +599,137 @@ QUnit.module('widget sizing render', moduleConfig, () => {
595599

596600
});
597601

602+
QUnit.module('esc on popup', {
603+
beforeEach: function() {
604+
this.instance.option({
605+
opened: false,
606+
items: [{ template: () => $('<div>').dxCheckBox({ value: false }) }],
607+
});
608+
},
609+
afterEach: function() {
610+
this.instance.option('opened', false);
611+
}
612+
}, () => {
613+
QUnit.test('esc on popup overlay should close popup', function(assert) {
614+
if(shouldSkipOnMobile(assert)) {
615+
return;
616+
}
617+
618+
this.overflowMenu.$button().focusin();
619+
this.keyboard.keyDown('enter');
620+
621+
const popup = this.overflowMenu.popup();
622+
assert.equal(popup.option('visible'), true, 'popup is visible');
623+
624+
const $popupContent = popup.$overlayContent();
625+
626+
keyboardMock($popupContent).keyDown('esc');
627+
assert.equal(popup.option('visible'), false, 'esc on popup overlay closed popup');
628+
});
629+
630+
QUnit.test('esc on nested items should NOT close popup', function(assert) {
631+
if(shouldSkipOnMobile(assert)) {
632+
return;
633+
}
634+
635+
this.overflowMenu.$button().focusin();
636+
this.keyboard.keyDown('enter');
637+
638+
const popup = this.overflowMenu.popup();
639+
assert.equal(popup.option('visible'), true, 'popup is visible');
640+
641+
const $checkBox = this.overflowMenu.$popupContent().find(`.${CHECK_BOX_CLASS}`).first();
642+
643+
keyboardMock($checkBox).keyDown('esc');
644+
assert.equal(popup.option('visible'), true, 'esc on nested item did not close popup');
645+
});
646+
});
647+
648+
QUnit.module('space/enter on list item (T1301705)', {
649+
beforeEach: function() {
650+
this.instance.option({
651+
closeOnClick: true,
652+
opened: false,
653+
items: [
654+
{
655+
template: () => $('<div>').dxCheckBox({ value: false }),
656+
widget: 'dxCheckBox',
657+
},
658+
{
659+
template: () => $('<div>').dxButton({ text: 'Button' })
660+
},
661+
]
662+
});
663+
},
664+
afterEach: function() {
665+
this.instance.option('opened', false);
666+
}
667+
}, () => {
668+
QUnit.test('space on a not dxButton in nested list item should NOT close popup', function(assert) {
669+
if(shouldSkipOnMobile(assert)) {
670+
return;
671+
}
672+
673+
this.overflowMenu.$button().focusin();
674+
this.keyboard.keyDown('enter');
675+
676+
const popup = this.overflowMenu.popup();
677+
assert.equal(popup.option('visible'), true, 'popup is visible');
678+
679+
const $checkBox = this.overflowMenu.$popupContent().find(`.${CHECK_BOX_CLASS}`).first();
680+
681+
keyboardMock($checkBox).keyDown('space');
682+
assert.equal(popup.option('visible'), true, 'space on a dxCheckBox did not close popup');
683+
});
684+
685+
QUnit.test('enter on a not dxButton in nested list item should NOT close popup', function(assert) {
686+
if(shouldSkipOnMobile(assert)) {
687+
return;
688+
}
689+
690+
this.overflowMenu.$button().focusin();
691+
this.keyboard.keyDown('enter');
692+
693+
const popup = this.overflowMenu.popup();
694+
assert.equal(popup.option('visible'), true, 'popup is visible');
695+
696+
const $checkBox = this.overflowMenu.$popupContent().find(`.${CHECK_BOX_CLASS}`).first();
697+
698+
keyboardMock($checkBox).keyDown('enter');
699+
assert.equal(popup.option('visible'), true, 'enter on a dxCheckBox did not close popup');
700+
});
701+
702+
QUnit.test('space on a dxButton in nested list item should close popup', function(assert) {
703+
if(shouldSkipOnMobile(assert)) {
704+
return;
705+
}
706+
this.overflowMenu.$button().focusin();
707+
this.keyboard.keyDown('enter');
708+
709+
const popup = this.overflowMenu.popup();
710+
assert.equal(popup.option('visible'), true, 'popup is visible');
711+
712+
const $button = this.overflowMenu.$popupContent().find(`.${BUTTON_CLASS}`).first();
713+
keyboardMock($button).keyDown('space');
714+
assert.equal(popup.option('visible'), false, 'space on a dxButton closed popup');
715+
});
716+
717+
QUnit.test('enter on a dxButton in nested list item should close popup', function(assert) {
718+
if(shouldSkipOnMobile(assert)) {
719+
return;
720+
}
721+
this.overflowMenu.$button().focusin();
722+
this.keyboard.keyDown('enter');
723+
724+
const popup = this.overflowMenu.popup();
725+
assert.equal(popup.option('visible'), true, 'popup is visible');
726+
727+
const $button = this.overflowMenu.$popupContent().find(`.${BUTTON_CLASS}`).first();
728+
keyboardMock($button).keyDown('enter');
729+
assert.equal(popup.option('visible'), false, 'enter on a dxButton closed popup');
730+
});
731+
});
732+
598733
QUnit.test('Enter or space press should call onItemClick (T318240)', function(assert) {
599734
let itemClicked = 0;
600735

0 commit comments

Comments
 (0)