Skip to content

Commit 643144c

Browse files
EugeniyKiyashkoEugeniyKiyashko
authored
Context menu should calculate correct height for root submenu on async render (T1306389) (#31043)
1 parent 34372d6 commit 643144c

File tree

5 files changed

+135
-55
lines changed

5 files changed

+135
-55
lines changed

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

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import type {
5252
import MenuBase from '@ts/ui/context_menu/menu_base';
5353
import type { InternalNode } from '@ts/ui/hierarchical_collection/data_converter';
5454
import Overlay from '@ts/ui/overlay/overlay';
55+
import { SCROLLABLE_CLASS } from '@ts/ui/scroll_view/consts';
5556
import Scrollable from '@ts/ui/scroll_view/scrollable';
5657

5758
const DX_MENU_CLASS = 'dx-menu';
@@ -68,7 +69,6 @@ const DX_STATE_FOCUSED_CLASS = 'dx-state-focused';
6869
const DX_STATE_HOVER_CLASS = 'dx-state-hover';
6970

7071
const OVERLAY_CONTENT_CLASS = 'dx-overlay-content';
71-
const SCROLLABLE_CLASS = 'dx-scrollable';
7272

7373
const FOCUS_UP = 'up';
7474
const FOCUS_DOWN = 'down';
@@ -791,14 +791,13 @@ class ContextMenu<
791791
_setSubMenuHeight(
792792
$submenu: dxElementWrapper,
793793
$anchor: ContextMenuTarget,
794-
isNestedSubmenu: boolean,
795794
): void {
796795
const $itemsContainer = $submenu.find(`.${DX_MENU_ITEMS_CONTAINER_CLASS}`);
797796
const contentHeight = getOuterHeight($itemsContainer);
798-
const maxHeight = this._getMaxHeight($anchor, !isNestedSubmenu);
797+
const maxHeight = this._getMaxHeight($anchor, false);
799798
const menuHeight = Math.min(contentHeight, maxHeight);
800799

801-
$submenu.css('height', isNestedSubmenu ? menuHeight : '100%');
800+
$submenu.css('height', menuHeight);
802801
}
803802

804803
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -825,6 +824,24 @@ class ContextMenu<
825824
return availableHeight - SUBMENU_PADDING;
826825
}
827826

827+
_setOverlayMaxHeight($subMenu?: dxElementWrapper): void {
828+
if (!$subMenu) {
829+
return;
830+
}
831+
832+
this._overlay?.option({
833+
maxHeight: () => {
834+
const $content = $subMenu.find(`.${DX_MENU_ITEMS_CONTAINER_CLASS}`);
835+
const outerHeight: number = getOuterHeight($content);
836+
const borderWidth = this._getSubmenuBorderWidth();
837+
838+
return outerHeight + borderWidth * 2;
839+
},
840+
});
841+
842+
$subMenu.css('height', '100%');
843+
}
844+
828845
_dimensionChanged(): void {
829846
if (!this._shownSubmenus) {
830847
return;
@@ -833,7 +850,7 @@ class ContextMenu<
833850
this._shownSubmenus.forEach(($submenu) => {
834851
const $item = $submenu.closest(`.${DX_MENU_ITEM_CLASS}`);
835852

836-
this._setSubMenuHeight($submenu, $item, true);
853+
this._setSubMenuHeight($submenu, $item);
837854
this._scrollToElement($item);
838855

839856
const submenuPosition = this._getSubmenuPosition($item);
@@ -873,7 +890,7 @@ class ContextMenu<
873890

874891
const $item = $submenu?.closest(`.${DX_MENU_ITEM_CLASS}`);
875892

876-
this._setSubMenuHeight($submenu, $item, true);
893+
this._setSubMenuHeight($submenu, $item);
877894

878895
if (!this._isSubmenuVisible($submenu) && $item) {
879896
this._drawSubmenu($item);
@@ -1154,19 +1171,13 @@ class ContextMenu<
11541171
this._setOptionWithoutOptionChange('visible', true);
11551172
this._overlay?.option({
11561173
height: () => this._getMaxHeight(position.of),
1157-
maxHeight: () => {
1158-
const $content = $subMenu.find(`.${DX_MENU_ITEMS_CONTAINER_CLASS}`);
1159-
const outerHeight: number = getOuterHeight($content);
1160-
const borderWidth = this._getSubmenuBorderWidth();
1161-
1162-
return outerHeight + borderWidth * 2;
1163-
},
11641174
position,
11651175
});
11661176

1167-
if ($subMenu.length) {
1168-
this._setSubMenuHeight($subMenu, position.of, false);
1169-
}
1177+
this._setOverlayMaxHeight($subMenu);
1178+
1179+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
1180+
this._planPostRenderActions($subMenu, true);
11701181

11711182
if (this._overlay) {
11721183
promise = this._overlay.show();
@@ -1298,8 +1309,13 @@ class ContextMenu<
12981309
return this.toggle(false);
12991310
}
13001311

1301-
_postProcessRenderItems($submenu?: dxElementWrapper): void {
1302-
this._setSubmenuVisible($submenu);
1312+
_postProcessRenderItems($subMenu?: dxElementWrapper, isRootSubMenu?: boolean): void {
1313+
if (!isRootSubMenu) {
1314+
this._setSubmenuVisible($subMenu);
1315+
return;
1316+
}
1317+
1318+
this._setOverlayMaxHeight($subMenu);
13031319
}
13041320
}
13051321

‎packages/devextreme/js/__internal/ui/scroll_view/consts.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export const DIRECTION_VERTICAL = 'vertical';
44
export const DIRECTION_HORIZONTAL = 'horizontal';
55
export const DIRECTION_BOTH = 'both';
66

7+
export const SCROLLABLE_CLASS = 'dx-scrollable';
78
export const SCROLLABLE_SIMULATED_CLASS = 'dx-scrollable-simulated';
89
export const SCROLLABLE_CONTENT_CLASS = 'dx-scrollable-content';
910
export const SCROLLABLE_WRAPPER_CLASS = 'dx-scrollable-wrapper';

‎packages/devextreme/js/__internal/ui/scroll_view/scrollable.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ import supportUtils from '@ts/core/utils/m_support';
2323
import type { DOMComponentProperties } from '@ts/core/widget/dom_component';
2424
import DOMComponent from '@ts/core/widget/dom_component';
2525
import type { OptionChanged } from '@ts/core/widget/types';
26+
import {
27+
SCROLLABLE_CLASS,
28+
SCROLLABLE_CONTAINER_CLASS,
29+
SCROLLABLE_CONTENT_CLASS,
30+
SCROLLABLE_DISABLED_CLASS,
31+
SCROLLABLE_WRAPPER_CLASS,
32+
} from '@ts/ui/scroll_view/consts';
2633
import { deviceDependentOptions } from '@ts/ui/scroll_view/scrollable.device';
2734
import NativeStrategy from '@ts/ui/scroll_view/scrollable.native';
2835
import { SimulatedStrategy } from '@ts/ui/scroll_view/scrollable.simulated';
@@ -35,11 +42,6 @@ import { getElementLocationInternal } from '@ts/ui/scroll_view/utils/get_element
3542

3643
const SCROLLABLE = 'dxScrollable';
3744
const SCROLLABLE_STRATEGY = 'dxScrollableStrategy';
38-
const SCROLLABLE_CLASS = 'dx-scrollable';
39-
const SCROLLABLE_DISABLED_CLASS = 'dx-scrollable-disabled';
40-
const SCROLLABLE_CONTAINER_CLASS = 'dx-scrollable-container';
41-
const SCROLLABLE_WRAPPER_CLASS = 'dx-scrollable-wrapper';
42-
const SCROLLABLE_CONTENT_CLASS = 'dx-scrollable-content';
4345
const VERTICAL = 'vertical';
4446
const HORIZONTAL = 'horizontal';
4547
const BOTH = 'both';

‎packages/devextreme/testing/tests/DevExpress.ui.widgets/contextMenu.async.tests.js‎

Lines changed: 74 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import $ from 'jquery';
22
import ContextMenu from 'ui/context_menu';
33
import devices from '__internal/core/m_devices';
4+
import { SCROLLABLE_CONTAINER_CLASS } from '__internal/ui/scroll_view/consts';
45

56
import 'ui/button';
67
import 'generic_light.css!';
78

89
const DX_SUBMENU_CLASS = 'dx-submenu';
910
const DX_CONTEXT_MENU_ITEMS_CONTAINER_CLASS = 'dx-menu-items-container';
10-
const DX_SCROLLABLE_CONTENT_CLASS = 'dx-scrollable-container';
1111
const DX_MENU_ITEM_CLASS = 'dx-menu-item';
1212

1313
QUnit.testStart(() => {
@@ -67,7 +67,57 @@ QUnit.module('Context menu', () => {
6767
assert.strictEqual(instance.option('items').length, 2, 'items.length');
6868
});
6969

70-
QUnit.test('Context menu should have correct height on async render (T1258881)', function(assert) {
70+
QUnit.test('Context menu should calculate correct height for root submenu on async render (T1306389)', function(assert) {
71+
const done = assert.async();
72+
73+
const menuTargetSelector = '#menuTarget';
74+
const items = [
75+
{ text: 'item 1', template: 'myTemplate' },
76+
{ text: 'item 2', template: 'myTemplate' },
77+
{ text: 'item 3', template: 'myTemplate' },
78+
{ text: 'item 4', template: 'myTemplate' },
79+
{ text: 'item 5', template: 'myTemplate' }
80+
];
81+
82+
const instance = new ContextMenu($('#simpleMenu'), {
83+
target: menuTargetSelector,
84+
items,
85+
templatesRenderAsynchronously: true,
86+
itemTemplate: 'myTemplate',
87+
showSubmenuMode: { name: 'onHover', delay: 0 },
88+
integrationOptions: {
89+
templates: {
90+
myTemplate: {
91+
render({ model, container, onRendered }) {
92+
setTimeout(() => {
93+
$('<div>').text(`Async: ${model.text}`).appendTo(container);
94+
onRendered();
95+
}, 10);
96+
}
97+
}
98+
}
99+
},
100+
});
101+
102+
instance.option('onShown', () => {
103+
setTimeout(() => {
104+
const $itemsContainer = $(instance._getItemsContainers());
105+
const $scrollableContainer = $(instance._overlay.$content()).find(`.${SCROLLABLE_CONTAINER_CLASS}`);
106+
107+
const itemsHeight = $itemsContainer.outerHeight();
108+
const scrollableContainerHeight = $scrollableContainer.outerHeight();
109+
110+
assert.roughEqual(itemsHeight, scrollableContainerHeight, 1,
111+
`Items container height (${itemsHeight}) should match scrollable height (${scrollableContainerHeight})`);
112+
113+
done();
114+
}, 10);
115+
});
116+
117+
$(menuTargetSelector).trigger($.Event('dxcontextmenu'));
118+
});
119+
120+
QUnit.test('Context menu should have correct height for submenus on async render (T1258881)', function(assert) {
71121
const done = assert.async();
72122

73123
const menuTargetSelector = '#menuTarget';
@@ -89,12 +139,13 @@ QUnit.module('Context menu', () => {
89139
items,
90140
templatesRenderAsynchronously: true,
91141
itemTemplate: 'myTemplate',
142+
showSubmenuMode: { name: 'onHover', delay: 0 },
92143
integrationOptions: {
93144
templates: {
94145
myTemplate: {
95146
render({ model, container, onRendered }) {
96147
setTimeout(() => {
97-
container.append($('<div>').text(model.text));
148+
$('<div>').text(model.text).appendTo(container);
98149
onRendered();
99150
});
100151
}
@@ -103,24 +154,32 @@ QUnit.module('Context menu', () => {
103154
},
104155
});
105156

106-
$(menuTargetSelector).trigger($.Event('dxcontextmenu'));
157+
instance.option('onShown', (e) => {
158+
setTimeout(() => {
159+
const $itemsContainer = instance.itemsContainer();
160+
const $rootItem = $itemsContainer.find(`.${DX_MENU_ITEM_CLASS}`).eq(0);
161+
const eventName = devices.real().deviceType === 'desktop' ? 'dxhoverstart' : 'dxclick';
107162

108-
const $itemsContainer = instance.itemsContainer();
109-
const $rootItem = $itemsContainer.find(`.${DX_MENU_ITEM_CLASS}`).eq(0);
163+
$($itemsContainer).trigger($.Event(eventName, { target: $rootItem.get(0) }));
110164

111-
const eventName = devices.real().deviceType === 'desktop' ? 'dxhoverstart' : 'dxclick';
165+
setTimeout(() => {
166+
const $submenus = $(`.${DX_SUBMENU_CLASS}`);
112167

113-
$($itemsContainer).trigger($.Event(eventName, { target: $rootItem.get(0) }));
168+
const $nestedSubmenu = $submenus.eq(1);
169+
const $nestedSubmenuItemsContainer = $nestedSubmenu.find(`.${DX_CONTEXT_MENU_ITEMS_CONTAINER_CLASS}`);
170+
const $scrollableContainer = $nestedSubmenu.find(`.${SCROLLABLE_CONTAINER_CLASS}`);
171+
const itemsHeight = $nestedSubmenuItemsContainer.outerHeight();
172+
const scrollableContainerHeight = $scrollableContainer.outerHeight();
114173

115-
instance.option('onShown', (e) => {
116-
const $submenus = $(`.${DX_SUBMENU_CLASS}`);
117-
const $nestedSubmenu = $submenus.eq(1);
118-
const $nestedSubmenuItemsContainer = $nestedSubmenu.find(`.${DX_CONTEXT_MENU_ITEMS_CONTAINER_CLASS}`);
119-
const $scrollableContainer = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTENT_CLASS}`);
174+
assert.roughEqual(itemsHeight, scrollableContainerHeight, 1,
175+
`Items container height (${itemsHeight}) should match scrollable height (${scrollableContainerHeight})`);
120176

121-
assert.roughEqual($nestedSubmenuItemsContainer.outerHeight(), $scrollableContainer.outerHeight(), .1);
122-
done();
177+
done();
178+
}, 10);
179+
}, 10);
123180
});
181+
182+
$(menuTargetSelector).trigger($.Event('dxcontextmenu'));
124183
});
125184
});
126185

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

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import themes from 'ui/themes';
1515
import keyboardMock from '../../helpers/keyboardMock.js';
1616
import ariaAccessibilityTestHelper from '../../helpers/ariaAccessibilityTestHelper.js';
1717
import { shouldSkipOnMobile } from '../../helpers/device.js';
18+
import {
19+
SCROLLABLE_CONTENT_CLASS,
20+
SCROLLABLE_CONTAINER_CLASS,
21+
SCROLLABLE_CLASS,
22+
} from '__internal/ui/scroll_view/consts';
1823

1924
import 'ui/button';
2025
import 'generic_light.css!';
@@ -321,9 +326,6 @@ QUnit.module('Repaint', moduleConfig, () => {
321326
});
322327

323328
QUnit.module('Rendering Scrollable', moduleConfig, () => {
324-
const DX_SCROLLABLE_CLASS = 'dx-scrollable';
325-
const DX_SCROLLABLE_CONTAINER_CLASS = 'dx-scrollable-container';
326-
const DX_SCROLLABLE_CONTENT_CLASS = 'dx-scrollable-content';
327329
const BORDER_WIDTH = 1;
328330
const SUBMENU_PADDING = 10;
329331

@@ -333,7 +335,7 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
333335
const $submenu = $(`.${DX_SUBMENU_CLASS}`);
334336

335337
assert.strictEqual($submenu.length, 1, 'only 1 submenu exists');
336-
assert.ok($submenu.hasClass(DX_SCROLLABLE_CLASS), 'Scrollable initialized');
338+
assert.ok($submenu.hasClass(SCROLLABLE_CLASS), 'Scrollable initialized');
337339
});
338340

339341
QUnit.test('Scrollable should be initialized on a 2nd level submenu', function(assert) {
@@ -358,7 +360,7 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
358360
assert.strictEqual($submenus.length, 2, '2 submenu exists');
359361

360362
const $nestedSubmenu = $submenus.eq(1);
361-
assert.ok($nestedSubmenu.hasClass(DX_SCROLLABLE_CLASS), 'Scrollable initialized on nested menu');
363+
assert.ok($nestedSubmenu.hasClass(SCROLLABLE_CLASS), 'Scrollable initialized on nested menu');
362364
});
363365

364366
QUnit.test('Height of the submenu should not exceed content height', function(assert) {
@@ -445,7 +447,7 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
445447

446448
const $submenu = $(`.${DX_SUBMENU_CLASS}`);
447449

448-
assert.ok($submenu.find(`.${DX_SCROLLABLE_CONTENT_CLASS}`).height() > $(window).height(), 'total height of submenu exceeds the window');
450+
assert.ok($submenu.find(`.${SCROLLABLE_CONTENT_CLASS}`).height() > $(window).height(), 'total height of submenu exceeds the window');
449451
assert.strictEqual($submenu.outerHeight(), $(window).height(), 'menu uses the full height of the window');
450452
assert.strictEqual($submenu.offset().top, 0, 'menu does not cross the window border');
451453
});
@@ -460,8 +462,8 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
460462
focusStateEnabled: true,
461463
visible: true,
462464
});
463-
const $scrollableContainer = $(instance.itemsContainer()).find(`.${DX_SCROLLABLE_CONTAINER_CLASS}`);
464-
const $scrollableContent = $(instance.itemsContainer()).find(`.${DX_SCROLLABLE_CONTENT_CLASS}`);
465+
const $scrollableContainer = $(instance.itemsContainer()).find(`.${SCROLLABLE_CONTAINER_CLASS}`);
466+
const $scrollableContent = $(instance.itemsContainer()).find(`.${SCROLLABLE_CONTENT_CLASS}`);
465467

466468
assert.strictEqual($scrollableContent.position().top, 0, 'initial position');
467469

@@ -499,8 +501,8 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
499501
.press('up');
500502

501503
const $nestedSubmenu = $(`.${DX_SUBMENU_CLASS}`).eq(1);
502-
const $scrollableContainer = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTAINER_CLASS}`);
503-
const $scrollableContent = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTENT_CLASS}`);
504+
const $scrollableContainer = $nestedSubmenu.find(`.${SCROLLABLE_CONTAINER_CLASS}`);
505+
const $scrollableContent = $nestedSubmenu.find(`.${SCROLLABLE_CONTENT_CLASS}`);
504506

505507
assert.roughEqual(
506508
$scrollableContent.position().top,
@@ -525,8 +527,8 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
525527
focusStateEnabled: true,
526528
visible: true,
527529
});
528-
const $scrollableContainer = $(instance.itemsContainer()).find(`.${DX_SCROLLABLE_CONTAINER_CLASS}`);
529-
const $scrollableContent = $(instance.itemsContainer()).find(`.${DX_SCROLLABLE_CONTENT_CLASS}`);
530+
const $scrollableContainer = $(instance.itemsContainer()).find(`.${SCROLLABLE_CONTAINER_CLASS}`);
531+
const $scrollableContent = $(instance.itemsContainer()).find(`.${SCROLLABLE_CONTENT_CLASS}`);
530532

531533
keyboardMock(instance.itemsContainer())
532534
.press('up')
@@ -562,8 +564,8 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
562564
.press('up');
563565

564566
const $nestedSubmenu = $(`.${DX_SUBMENU_CLASS}`).eq(1);
565-
const $scrollableContainer = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTAINER_CLASS}`);
566-
const $scrollableContent = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTENT_CLASS}`);
567+
const $scrollableContainer = $nestedSubmenu.find(`.${SCROLLABLE_CONTAINER_CLASS}`);
568+
const $scrollableContent = $nestedSubmenu.find(`.${SCROLLABLE_CONTENT_CLASS}`);
567569

568570
assert.roughEqual(
569571
$scrollableContent.position().top,
@@ -603,8 +605,8 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
603605
.press('up');
604606

605607
const $nestedSubmenu = $(`.${DX_SUBMENU_CLASS}`).eq(1);
606-
const $scrollableContainer = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTAINER_CLASS}`);
607-
const $scrollableContent = $nestedSubmenu.find(`.${DX_SCROLLABLE_CONTENT_CLASS}`);
608+
const $scrollableContainer = $nestedSubmenu.find(`.${SCROLLABLE_CONTAINER_CLASS}`);
609+
const $scrollableContent = $nestedSubmenu.find(`.${SCROLLABLE_CONTENT_CLASS}`);
608610

609611
assert.roughEqual(
610612
$scrollableContent.position().top,
@@ -632,7 +634,7 @@ QUnit.module('Rendering Scrollable', moduleConfig, () => {
632634
$($itemsContainer).trigger($.Event('dxhoverstart', { target: $rootItem.get(0) }));
633635
this.clock.tick(0);
634636

635-
$(`.${DX_SCROLLABLE_CONTENT_CLASS}`).each((_, scrollableContent) => {
637+
$(`.${SCROLLABLE_CONTENT_CLASS}`).each((_, scrollableContent) => {
636638
assert.strictEqual(window.getComputedStyle(scrollableContent).minHeight, '0px', 'min-height = auto');
637639
});
638640
});

0 commit comments

Comments
 (0)