Skip to content

Widgets: Remove code related width, height option as function (FilterBuilder, DateBox, DropDownEditor, DropDownButton, Lookup, Menu, Toolbar.Menu) #33177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marker-dao wants to merge 3 commits into
base: 26_1
Choose a base branch
from
+40 −37
Open

Widgets: Remove code related width, height option as function (FilterBuilder, DateBox, DropDownEditor, DropDownButton, Lookup, Menu, Toolbar.Menu) #33177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c88f6d0
Widget: remove code related width, height option as function
Apr 6, 2026
5f539c1
Widgets: Remove code related width, height option as function
Apr 6, 2026
ed07142
Merge branch '26_1' into 26_1_widget_height_width_function_removing
marker-dao Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/devextreme/js/__internal/filter_builder/m_filter_builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,10 +891,7 @@ class FilterBuilder extends Widget<any> {
// T852701
this.repaint();
},
_ignoreFunctionValueDeprecation: true,
maxHeight() {
return getElementMaxHeightByWindow(options.menu.position.of);
},
maxHeight: getElementMaxHeightByWindow(options.menu.position.of),
visible: true,
focusStateEnabled: false,
preventScrollEvents: false,
Expand Down
1 change: 1 addition & 0 deletions packages/devextreme/js/__internal/scheduler/__tests__/__mock__/create_appointment_popup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export const createAppointmentPopup = async (
const title = options.title ?? 'New Appointment';
const readOnly = options.readOnly ?? false;

// @ts-expect-error onSave
popup.show(appointmentData, { onSave, title, readOnly });
Comment on lines +170 to 171
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ts-expect-error on popup.show(...) makes the mock less type-safe and can mask real API drift. Prefer updating the AppointmentPopup.show typing (or the mock wrapper types) so the { onSave, title, readOnly } argument is accepted without suppression, matching how appointmentPopup.show is used in scheduler code.

Copilot uses AI. Check for mistakes.
await new Promise(process.nextTick);

Expand Down
1 change: 1 addition & 0 deletions packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ describe('Isolated AppointmentPopup environment', () => {

const onSave = jest.fn((newAppointment: Record<string, unknown>) => {
updateAppointment(sourceAppointment, updatedAppointment);
// @ts-expect-error Expected 0 arguments, but got 1
return addAppointment(newAppointment);
});

Expand Down
8 changes: 8 additions & 0 deletions packages/devextreme/js/__internal/ui/date_box/date_box.base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,14 @@ class DateBox extends DropDownEditor<DateBoxBaseProperties> {
this._updatePopupHeight();
}

_updatePopupWidth(): void {
if (this._strategy instanceof Calendar || this._strategy instanceof CalendarWithTime) {
return;
}

super._updatePopupWidth();
}

_dimensionChanged(): void {
super._dimensionChanged();

Expand Down
9 changes: 4 additions & 5 deletions packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,9 +866,7 @@ class DropDownEditor<
}),
// @ts-expect-error ts-error
showTitle: this.option('dropDownOptions.showTitle'),
_ignoreFunctionValueDeprecation: true,
// @ts-expect-error ts-error
width: (): number => getElementWidth(this.$element()),
width: getElementWidth(this.$element()),
height: 'auto',
shading: false,
hideOnParentScroll: true,
Expand Down Expand Up @@ -925,10 +923,10 @@ class DropDownEditor<
}

_updatePopupWidth(): void {
const popupWidth = getSizeValue(this.option('dropDownOptions.width'));
const popupWidth = getSizeValue(this.option('_cached_dropDownOptions.width'));

if (popupWidth === undefined) {
this._setPopupOption('width', () => getElementWidth(this.$element()));
this._setPopupOption('width', getElementWidth(this.$element()));
}
Comment on lines 925 to 930
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_updatePopupWidth() now reads this.option('_cached_dropDownOptions.width'), but _cached_dropDownOptions is only initialized once in _init() and is not refreshed when dropDownOptions is replaced via option('dropDownOptions', ...). This can cause user-specified dropDownOptions.width to be ignored and overwritten with the element width. Consider reading dropDownOptions.width directly here, or updating the cache whenever dropDownOptions changes (including full-object replacement).

Copilot uses AI. Check for mistakes.
}

Expand Down Expand Up @@ -1168,6 +1166,7 @@ class DropDownEditor<
case 'width':
case 'height':
super._optionChanged(args);
this._updatePopupWidth();
this._popup?.repaint();
break;
case 'opened':
Expand Down
2 changes: 2 additions & 0 deletions packages/devextreme/js/__internal/ui/m_drop_down_box.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ class DropDownBox<
position: extend(this.option('popupPosition'), {
of: this.$element(),
}),
// The overlay calls maxHeight every time it calculates the content dimensions,
// this._popupPosition?.v.location, which is updated after each repositioning
Comment on lines +299 to +300
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new comment is grammatically incomplete (second line is a sentence fragment) and is hard to understand as-is. Please rephrase into a complete explanation of why maxHeight must remain a function here (e.g., tie it explicitly to repositioning and _popupPosition.v.location).

Suggested change
// The overlay calls maxHeight every time it calculates the content dimensions,
// this._popupPosition?.v.location, which is updated after each repositioning
// maxHeight must remain a function because the overlay reevaluates it whenever
// it recalculates content dimensions, and it depends on this._popupPosition?.v.location,
// which is updated after each repositioning.

Copilot uses AI. Check for mistakes.
_ignoreFunctionValueDeprecation: true,
// @ts-expect-error ts-error
maxHeight: function () {
Expand Down
5 changes: 2 additions & 3 deletions packages/devextreme/js/__internal/ui/m_drop_down_button.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,7 @@ class DropDownButton extends Widget<DropDownButtonProperties> {
type: 'fade', duration: 400, from: 1, to: 0,
},
},
_ignoreFunctionValueDeprecation: true,
width: () => getElementWidth(this.$element()),
width: getElementWidth(this.$element()),
height: 'auto',
shading: false,
position: {
Expand Down Expand Up @@ -532,7 +531,7 @@ class DropDownButton extends Widget<DropDownButtonProperties> {
const popupWidth = getSizeValue(this.option('dropDownOptions.width'));

if (popupWidth === undefined) {
this._setPopupOption('width', () => getElementWidth(this.$element()));
this._setPopupOption('width', getElementWidth(this.$element()));
}
}

Expand Down
15 changes: 6 additions & 9 deletions packages/devextreme/js/__internal/ui/m_lookup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,10 @@ class Lookup extends DropDownList<LookupProperties> {
focusStateEnabled: false,
dropDownOptions: {
showTitle: true,
// @ts-expect-error ts-error
width() {
return getSize('width');
},
// @ts-expect-error ts-error
height() {
return getSize('height');
},
// @ts-expect-error Category 3 — There is no built-in mechanism for mandatory updates
width: () => getSize('width'),
// @ts-expect-error Category 3 — There is no built-in mechanism for mandatory updates
height: () => getSize('height'),
Comment on lines 131 to +136
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title indicates removing function-based width/height options for Lookup, but dropDownOptions.width/height are still set to functions here and type errors are suppressed. If function values are no longer supported, this can break popup sizing at runtime; instead, compute numeric values and update them on relevant resize/viewport-change events (e.g., via _dimensionChanged + visualViewport handlers).

Copilot uses AI. Check for mistakes.
shading: true,
hideOnOutsideClick: true,
animation: {},
Expand Down Expand Up @@ -218,7 +214,7 @@ class Lookup extends DropDownList<LookupProperties> {
dropDownOptions: {
_ignoreFunctionValueDeprecation: true,

width: () => getElementWidth(this.$element()),
width: getElementWidth(this.$element()),
height: function () { return this._getPopupHeight(); }.bind(this),
Comment on lines 214 to 218
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Material defaultOptionsRule, dropDownOptions.width is now computed immediately via getElementWidth(this.$element()). If the widget is initialized while hidden or before final layout, this value can be 0/stale and will prevent the base popup-width auto-update logic from running (since width becomes defined). Consider leaving dropDownOptions.width undefined and relying on _updatePopupWidth(), or recomputing it when the component becomes visible/dimensions change.

Copilot uses AI. Check for mistakes.
showTitle: false,

Expand Down Expand Up @@ -819,6 +815,7 @@ class Lookup extends DropDownList<LookupProperties> {
this.option('dropDownOptions.width', getWidth(this.$element()));
}

this._updatePopupWidth();
this._updateListDimensions();
}

Expand Down
9 changes: 3 additions & 6 deletions packages/devextreme/js/__internal/ui/menu/menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,10 @@ class Menu extends MenuBase<MenuProperties> {

_hoveredRootItem?: dxElementWrapper | null;

// eslint-disable-next-line no-restricted-globals
_showSubmenuTimer?: ReturnType<typeof setTimeout> | number;

// eslint-disable-next-line no-restricted-globals
_hideSubmenuTimer?: ReturnType<typeof setTimeout>;

// eslint-disable-next-line no-restricted-globals
_resizeEventTimer?: ReturnType<typeof setTimeout>;

// @ts-expect-error ts-error
Expand Down Expand Up @@ -343,7 +340,9 @@ class Menu extends MenuBase<MenuProperties> {
}

const containerWidth: number = getOuterWidth(this.$element());

this._toggleAdaptiveMode(this._menuItemsWidth > containerWidth);
this._overlay?.option('maxHeight', getElementMaxHeightByWindow(this.$element()));
}

_init(): void {
Expand Down Expand Up @@ -473,9 +472,7 @@ class Menu extends MenuBase<MenuProperties> {
const position = rtlEnabled ? 'right' : 'left';

return {
_ignoreFunctionValueDeprecation: true,
// @ts-expect-error ts-error
maxHeight: () => getElementMaxHeightByWindow(this.$element()),
maxHeight: getElementMaxHeightByWindow(this.$element()),
deferRendering: false,
shading: false,
// @ts-expect-error ts-error
Expand Down
8 changes: 5 additions & 3 deletions packages/devextreme/js/__internal/ui/toolbar/internal/toolbar.menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,7 @@ export default class DropDownMenu extends Widget<DropDownMenuProperties> {
preventScrollEvents: false,
_ignorePreventScrollEventsDeprecation: true,
contentTemplate: (contentElement) => this._renderList(contentElement),
_ignoreFunctionValueDeprecation: true,
// @ts-expect-error
maxHeight: () => this._getMaxHeight(),
maxHeight: this._getMaxHeight(),
position: {
// @ts-expect-error
my: `top ${rtlEnabled ? 'left' : 'right'}`,
Expand Down Expand Up @@ -324,6 +322,10 @@ export default class DropDownMenu extends Widget<DropDownMenuProperties> {
);
}

_dimensionChanged(): void {
this._popup?.option('maxHeight', this._getMaxHeight());
}

_closeOutsideDropDownHandler(
e: DxEvent<PointerInteractionEvent>,
): boolean {
Expand Down
2 changes: 1 addition & 1 deletion packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3719,7 +3719,7 @@ QUnit.module('default options', {

const lookup = $lookup.dxLookup({ dataSource: ['blue', 'orange', 'lime', 'purple'] }).dxLookup('instance');

assert.equal(lookup.option('dropDownOptions.width')(), $lookup.outerWidth(), 'popup width match with lookup field width');
assert.equal(lookup.option('dropDownOptions.width'), $lookup.outerWidth(), 'popup width match with lookup field width');

$(lookup.field()).trigger('dxclick');

Expand Down
2 changes: 1 addition & 1 deletion packages/devextreme/testing/tests/DevExpress.ui.widgets/filterBuilderParts/commonTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ QUnit.module('Rendering', function() {
const maxHeight = popup.option('maxHeight');
const positionCollision = popup.option('position.collision');

assert.ok(Math.floor(maxHeight()) < windowHeight(), 'maxHeight is correct');
assert.ok(Math.floor(maxHeight) < windowHeight(), 'maxHeight is correct');
assert.equal(positionCollision, 'flip', 'collision is correct');
} finally {
scrollTop.restore();
Expand Down
10 changes: 5 additions & 5 deletions packages/devextreme/testing/tests/DevExpress.ui.widgets/menu.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3242,20 +3242,20 @@ QUnit.module('adaptivity: render', {
});

QUnit.test('maxHeight should be 90% of maximum of top or bottom offsets when height of overlay content more windows height', function(assert) {
const scrollTop = sinon.stub(renderer.fn, 'scrollTop').returns(100);
const windowHeight = sinon.stub(implementationsMap, 'getInnerHeight').returns(700);
const offset = sinon.stub(renderer.fn, 'offset').returns({ left: 0, top: 200 });

new Menu(this.$element, {
items: this.items,
adaptivityEnabled: true
});

const scrollTop = sinon.stub(renderer.fn, 'scrollTop').returns(100);
const windowHeight = sinon.stub(implementationsMap, 'getInnerHeight').returns(700);
const offset = sinon.stub(renderer.fn, 'offset').returns({ left: 0, top: 200 });

try {
const overlay = this.$element.find('.dx-overlay').dxOverlay('instance');
const maxHeight = overlay.option('maxHeight');

assert.ok(Math.floor(maxHeight()) < windowHeight(), 'maxHeight is correct');
assert.ok(Math.floor(maxHeight) < windowHeight(), 'maxHeight is correct');
assert.ok(overlay.$wrapper().hasClass(DX_ADAPTIVE_MODE_OVERLAY_WRAPPER_CLASS), 'special class for overlay wrapper');
} finally {
scrollTop.restore();
Expand Down
Loading