Skip to content

Commit 9cba04f

Browse files
EugeniyKiyashkoEugeniyKiyashko
authored
dxMenu: clicking an item with a link shouldn’t sent multiple requests (T1299240) (DevExpress#30387)
1 parent ee26f47 commit 9cba04f

File tree

5 files changed

+107
-16
lines changed

5 files changed

+107
-16
lines changed

‎packages/devextreme/js/__internal/ui/context_menu/m_menu_base.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -574,12 +574,19 @@ class MenuBase extends HierarchicalCollectionWidget<Properties> {
574574
const { event, itemData } = actionArgs.args[0];
575575

576576
const $itemElement = this._getItemElementByEventArgs(event);
577-
const link = $itemElement && $itemElement.find(`.${ITEM_URL_CLASS}`).get(0);
577+
const link = $itemElement && $itemElement.find(`.${ITEM_URL_CLASS}`)[0];
578578

579-
if (itemData.url && link) {
580-
// @ts-expect-error
581-
link.click();
579+
if (!itemData.url || !link) {
580+
return;
582581
}
582+
583+
const isNativeLinkClick = $(event.target).closest(`.${ITEM_URL_CLASS}`).length;
584+
585+
if (isNativeLinkClick) {
586+
return;
587+
}
588+
589+
this._clickByLink(link);
583590
}
584591

585592
_updateSubmenuVisibilityOnClick(actionArgs): void {

‎packages/devextreme/js/__internal/ui/hierarchical_collection/hierarchical_collection_widget.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,14 @@ TKey = any,
138138
.append(textContainer);
139139
}
140140

141+
_clickByLink(link: HTMLElement): void {
142+
link.addEventListener('click', (e: MouseEvent): void => {
143+
e.stopPropagation();
144+
}, { once: true });
145+
146+
link.click();
147+
}
148+
141149
_getIconContainer(itemData: TItem): dxElementWrapper | undefined | null {
142150
// @ts-expect-error ts-error
143151
if (!itemData.icon) {

‎packages/devextreme/js/__internal/ui/tree_view/m_tree_view.base.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,13 +1604,13 @@ class TreeViewBase extends HierarchicalCollectionWidget<TreeViewBaseProperties>
16041604
};
16051605
}
16061606

1607-
_itemClick(actionArgs) {
1608-
const args = actionArgs.args[0];
1609-
const target = args.event.target[0] || args.event.target;
1607+
_itemClick(actionArgs): void {
1608+
const { event, itemData } = actionArgs.args[0];
1609+
const target = event.target[0] || event.target;
16101610
const link = target.getElementsByClassName(ITEM_URL_CLASS)[0];
16111611

1612-
if (args.itemData.url && link) {
1613-
link.click();
1612+
if (itemData.url && link) {
1613+
this._clickByLink(link);
16141614
}
16151615
}
16161616

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3707,6 +3707,27 @@ QUnit.module('adaptivity: behavior', {
37073707
assert.ok(clickSpy.calledOnce);
37083708
});
37093709

3710+
QUnit.test('onItemClick should be raised once if item.url is set', function(assert) {
3711+
const onItemClickSpy = sinon.spy();
3712+
3713+
new Menu(this.$element, {
3714+
items: this.items,
3715+
onItemClick: onItemClickSpy,
3716+
adaptivityEnabled: true
3717+
});
3718+
3719+
const parentTreeviewItem = $(`.${DX_TREEVIEW_ITEM_CLASS}`).eq(1);
3720+
3721+
parentTreeviewItem.trigger('dxclick');
3722+
onItemClickSpy.resetHistory();
3723+
3724+
const treeviewItem = $(`.${DX_TREEVIEW_ITEM_CLASS}`).eq(2);
3725+
3726+
treeviewItem.trigger('dxclick');
3727+
3728+
assert.strictEqual(onItemClickSpy.calledOnce, true, 'onItemClick was called once');
3729+
});
3730+
37103731
QUnit.test('link should be clicked programmatically with enter key if item.url is set', function(assert) {
37113732
if(shouldSkipOnMobile(assert)) {
37123733
return;

‎packages/devextreme/testing/tests/DevExpress.ui.widgets/menuBase.tests.js‎

Lines changed: 62 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -475,25 +475,55 @@ QUnit.module('Menu rendering', () => {
475475
assert.strictEqual(content.text, 'Item text');
476476
});
477477

478-
QUnit.test('Link should be programmatically clicked if item.url is set and text inside link is clicked', function(assert) {
478+
QUnit.test('onItemClick should be raised once if item.url is set and text inside link is clicked', function(assert) {
479+
const onItemClickSpy = sinon.spy();
480+
481+
const menuBase = createMenu({
482+
items: [{ text: 'Item text', url: 'http://some_url' }],
483+
onItemClick: onItemClickSpy,
484+
});
485+
486+
const menuItemLink = menuBase.element
487+
.find(`.${ITEM_URL_CLASS}`)
488+
.get(0);
489+
490+
menuItemLink.addEventListener('click', (e) => {
491+
e.preventDefault();
492+
});
493+
494+
const $itemText = menuBase.element
495+
.find(`.${DX_MENU_ITEM_TEXT_CLASS}`)
496+
.eq(0);
497+
498+
$itemText.trigger('click');
499+
500+
assert.strictEqual(onItemClickSpy.callCount, 1, 'onItemClick called once');
501+
});
502+
503+
QUnit.test('Link should be clicked if item.url is set and text inside link is clicked', function(assert) {
479504
const clickSpy = sinon.spy();
505+
480506
const menuBase = createMenu({
481507
items: [{ text: 'Item text', url: 'http://some_url' }],
482508
});
483509

484-
const $menuItemLink = menuBase.element
510+
const menuItemLink = menuBase.element
485511
.find(`.${ITEM_URL_CLASS}`)
486512
.get(0);
487513

488-
$menuItemLink.click = clickSpy;
514+
menuItemLink.addEventListener('click', (e) => {
515+
e.preventDefault();
516+
517+
clickSpy(e);
518+
});
489519

490520
const $itemText = menuBase.element
491521
.find(`.${DX_MENU_ITEM_TEXT_CLASS}`)
492522
.eq(0);
493523

494-
$itemText.trigger('dxclick');
524+
$itemText.trigger('click');
495525

496-
assert.ok(clickSpy.calledOnce);
526+
assert.strictEqual(clickSpy.callCount, 1, 'link clicked once');
497527
});
498528

499529
QUnit.test('Link should be programmatically clicked if item.url is set and item is clicked', function(assert) {
@@ -502,11 +532,11 @@ QUnit.module('Menu rendering', () => {
502532
items: [{ text: 'Item text', url: 'http://some_url' }],
503533
});
504534

505-
const $menuItemLink = menuBase.element
535+
const menuItemLink = menuBase.element
506536
.find(`.${ITEM_URL_CLASS}`)
507537
.get(0);
508538

509-
$menuItemLink.click = clickSpy;
539+
menuItemLink.click = clickSpy;
510540

511541
const $item = menuBase.element
512542
.find(`.${DX_MENU_ITEM_CLASS}`)
@@ -517,6 +547,31 @@ QUnit.module('Menu rendering', () => {
517547
assert.ok(clickSpy.calledOnce);
518548
});
519549

550+
551+
QUnit.test('onItemClick should be raised once if item.url is set and item is clicked', function(assert) {
552+
const onItemClickSpy = sinon.spy();
553+
const menuBase = createMenu({
554+
items: [{ text: 'Item text', url: 'http://some_url' }],
555+
onItemClick: onItemClickSpy,
556+
});
557+
558+
const menuItemLink = menuBase.element
559+
.find(`.${ITEM_URL_CLASS}`)
560+
.get(0);
561+
562+
menuItemLink.addEventListener('click', (e) => {
563+
e.preventDefault();
564+
});
565+
566+
const $item = menuBase.element
567+
.find(`.${DX_MENU_ITEM_CLASS}`)
568+
.eq(0);
569+
570+
$item.trigger('dxclick');
571+
572+
assert.strictEqual(onItemClickSpy.calledOnce, true, 'onItemClick called once');
573+
});
574+
520575
QUnit.test('Link should be rendered with empty text (T1181344)', function(assert) {
521576
createMenu({
522577
items: [{ text: '', url: 'http://some_url' }],

0 commit comments

Comments
 (0)