Skip to content

Popup: Remove .dx-toolbar class when template is used and remove replacing of $container with $result (T1322170) #32686

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

Merged
marker-dao merged 11 commits into from
Feb 27, 2026
Merged

Popup: Remove .dx-toolbar class when template is used and remove replacing of $container with $result (T1322170) #32686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f295e5a
refactor(m_popup.ts): Remove alerts
Feb 24, 2026
3da56f2
Popup: Remove dx-toolbar
Feb 24, 2026
7586714
fix(m_popup.ts): Get rid of replacing in Vue
Feb 24, 2026
52289fb
fix && revert(): Remove replacing with template wrapper && Revert all…
Feb 25, 2026
c260b69
fix(popup.tests): Fix dxPopup should render custom template with rend…
Feb 26, 2026
6c999a8
chore(m_popup.ts): Remove comments
Feb 26, 2026
b4196d1
chore(m_popup.ts): Remove comments
Feb 26, 2026
2504fb6
feat(popup): Add tests
Feb 26, 2026
204f89e
fix(popup.tests)
Feb 26, 2026
0b2ee6e
test(popup): Don't touch classes at all
Feb 27, 2026
c45c944
fix(popup): Remove adding && deleting wrapper class
Feb 27, 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
37 changes: 21 additions & 16 deletions packages/devextreme/js/__internal/ui/popup/m_popup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { EmptyTemplate } from '@js/core/templates/empty_template';
import type { TemplateBase } from '@js/core/templates/template_base';
import { noop } from '@js/core/utils/common';
import type { DeferredObj } from '@js/core/utils/deferred';
import { contains } from '@js/core/utils/dom';
import { extend } from '@js/core/utils/extend';
import { camelize } from '@js/core/utils/inflector';
import { each } from '@js/core/utils/iterator';
Expand Down Expand Up @@ -51,7 +52,6 @@ import type {
ControllerProperties,
} from '@ts/ui/overlay/overlay_position_controller';
import * as zIndexPool from '@ts/ui/overlay/z_index';
import { TOOLBAR_CLASS } from '@ts/ui/toolbar/constants';
import type { ToolbarBaseProperties } from '@ts/ui/toolbar/toolbar.base';

import PopupDrag from './m_popup_drag';
Expand All @@ -75,22 +75,17 @@ const POPUP_NORMAL_CLASS = 'dx-popup-normal';
export const POPUP_CONTENT_CLASS = 'dx-popup-content';
export const POPUP_CONTENT_SCROLLABLE_CLASS = 'dx-popup-content-scrollable';

const DISABLED_STATE_CLASS = 'dx-state-disabled';
const POPUP_DRAGGABLE_CLASS = 'dx-popup-draggable';

const POPUP_TITLE_CLASS = 'dx-popup-title';
export const POPUP_TITLE_CLOSEBUTTON_CLASS = 'dx-closebutton';

const POPUP_BOTTOM_CLASS = 'dx-popup-bottom';

const POPUP_HAS_CLOSE_BUTTON_CLASS = 'dx-has-close-button';

export const TEMPLATE_WRAPPER_CLASS = 'dx-template-wrapper';

const POPUP_CONTENT_FLEX_HEIGHT_CLASS = 'dx-popup-flex-height';
const POPUP_CONTENT_INHERIT_HEIGHT_CLASS = 'dx-popup-inherit-height';

const TOOLBAR_LABEL_CLASS = 'dx-toolbar-label';
const DISABLED_STATE_CLASS = 'dx-state-disabled';
export const TEMPLATE_WRAPPER_CLASS = 'dx-template-wrapper';

const ALLOWED_TOOLBAR_ITEM_ALIASES = ['cancel', 'clear', 'done'];

Expand All @@ -102,7 +97,11 @@ const BUTTON_OUTLINED_MODE = 'outlined';

const TOOLBAR_NAME_BASE = 'dxToolbarBase';

const HEIGHT_STRATEGIES = { static: '', inherit: POPUP_CONTENT_INHERIT_HEIGHT_CLASS, flex: POPUP_CONTENT_FLEX_HEIGHT_CLASS } as const;
const HEIGHT_STRATEGIES = {
static: '',
inherit: POPUP_CONTENT_INHERIT_HEIGHT_CLASS,
flex: POPUP_CONTENT_FLEX_HEIGHT_CLASS,
} as const;
Comment thread
marker-dao marked this conversation as resolved.

type HeightStrategiesType = typeof HEIGHT_STRATEGIES[keyof typeof HEIGHT_STRATEGIES];
type TitleRenderAction = (event?: Record<string, unknown>) => void;
Expand Down Expand Up @@ -537,9 +536,9 @@ class Popup<
if (!$content) {
return;
}

const $toolbarContainer = $('<div>')
.addClass(POPUP_TITLE_CLASS)
.addClass(TOOLBAR_CLASS)
.insertBefore($content);

this._$topToolbar = this._renderToolbar(
Expand Down Expand Up @@ -589,7 +588,6 @@ class Popup<

const $toolbarContainer = $('<div>')
.addClass(POPUP_BOTTOM_CLASS)
.addClass(TOOLBAR_CLASS)
.insertAfter($content);

this._$bottomToolbar = this._renderToolbar(
Expand Down Expand Up @@ -689,11 +687,10 @@ class Popup<
$container: dxElementWrapper,
): dxElementWrapper {
const $result = $(template.render({ container: getPublicElement($container) }));
const resultInContainer = contains($container.get(0), $result.get(0));

if ($result.hasClass(TEMPLATE_WRAPPER_CLASS)) {
$container.replaceWith($result);
// eslint-disable-next-line no-param-reassign
$container = $result;
if (!resultInContainer) {
$container.append($result);
}

return $container;
Expand Down Expand Up @@ -801,6 +798,7 @@ class Popup<
index++;
}

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Comment thread
marker-dao marked this conversation as resolved.
item.toolbar = data.toolbar || item.toolbar || 'top';
Comment thread
marker-dao marked this conversation as resolved.

if (item && item.toolbar === toolbar) {
Expand Down Expand Up @@ -847,7 +845,12 @@ class Popup<
return BUTTON_NORMAL_TYPE;
}

_getToolbarItemByAlias(data) {
// eslint-disable-next-line @stylistic/max-len
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
_getToolbarItemByAlias(data: any): {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
template: (_: any, __: any, container: dxElementWrapper) => void;
} | boolean {
Comment thread
marker-dao marked this conversation as resolved.
const itemType = data.shortcut;

if (!ALLOWED_TOOLBAR_ITEM_ALIASES.includes(itemType)) {
Expand All @@ -860,6 +863,7 @@ class Popup<
integrationOptions: {},
type: this._getToolbarButtonType(itemType),
stylingMode: this._getToolbarButtonStylingMode(itemType),
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
}, data.options || {});
Comment thread
marker-dao marked this conversation as resolved.

const itemClass = `${POPUP_CLASS}-${itemType}`;
Expand Down Expand Up @@ -890,6 +894,7 @@ class Popup<

_toggleDisabledState(value: boolean): void {
// @ts-expect-error ts-error
// eslint-disable-next-line prefer-rest-params
super._toggleDisabledState(...arguments);
Comment thread
marker-dao marked this conversation as resolved.

this.$content()?.toggleClass(DISABLED_STATE_CLASS, Boolean(value));
Expand Down
151 changes: 149 additions & 2 deletions packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const IS_OLD_SAFARI = IS_SAFARI && compareVersions(browser.version, [11]) < 0;
const PREVENT_SAFARI_SCROLLING_CLASS = 'dx-prevent-safari-scrolling';
const CUSTOM_ITEM_CLASS = 'custom-item-class';
const TOOLBAR_LABEL_CLASS = 'dx-toolbar-label';
const TOOLBAR_CLASS = 'dx-toolbar';

themes.setDefaultTimeout(0);

Expand Down Expand Up @@ -702,7 +703,7 @@ QUnit.module('dimensions', {
integrationOptions: {
templates: {
'title': {
render: function(args) {
render: () => {
const $element = $('<span>')
.addClass(TEMPLATE_WRAPPER_CLASS)
.text('text');
Expand All @@ -714,7 +715,10 @@ QUnit.module('dimensions', {
}
}).dxPopup('instance');

assert.equal(popup.$overlayContent().text(), 'text', 'container is correct');
const popupOverlayContent = popup.$overlayContent();
const text = popupOverlayContent.text();

assert.strictEqual(text, 'text', 'container is correct');
});

QUnit.test('dimensions should be shrunk correctly with floating heights', function(assert) {
Expand Down Expand Up @@ -3082,6 +3086,149 @@ QUnit.module('templates', {

assert.strictEqual($customItemAfterRuntimeChange.length, 1, 'custom template rendered after runtime change');
});

QUnit.module('renderByTemplate', {
beforeEach: function() {
this.popup = $('#popup').dxPopup({
visible: true,
showTitle: true,
}).dxPopup('instance');
},
}, () => {
QUnit.test('toolbar container should not have dx-toolbar class when custom titleTemplate is used', function(assert) {
this.popup.option('titleTemplate', () => {
return $('<div>').addClass('custom-title').text('Custom');
});

const $overlayContent = this.popup.$overlayContent();
const $topToolbarContainer = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();

assert.ok($topToolbarContainer.hasClass(POPUP_TITLE_CLASS), 'toolbar container has dx-popup-title class');
assert.notOk($topToolbarContainer.hasClass(TOOLBAR_CLASS),
'dx-toolbar class is not on the container when custom template is used');
});

QUnit.test('bottom toolbar container should not have dx-toolbar class when custom bottomTemplate is used', function(assert) {
this.popup.option({
toolbarItems: [
{ text: 'bottom text', toolbar: 'bottom', location: 'center' },
],
bottomTemplate: () => {
return $('<div>').addClass('custom-bottom').text('Custom Bottom');
},
});

const $overlayContent = this.popup.$overlayContent();
const $bottomToolbarContainer = $overlayContent.find(`.${POPUP_BOTTOM_CLASS}`).first();

assert.ok($bottomToolbarContainer.hasClass(POPUP_BOTTOM_CLASS), 'toolbar container has dx-popup-bottom class');
assert.notOk($bottomToolbarContainer.hasClass(TOOLBAR_CLASS),
'dx-toolbar class is not on the container when custom template is used');
});

QUnit.test('titleTemplate result is appended inside container', function(assert) {
const customClass = 'my-custom-title';

this.popup.option('titleTemplate', () => {
return $('<div>').addClass(customClass).text('Custom Title');
});

const $overlayContent = this.popup.$overlayContent();
const $titleContainer = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();

assert.strictEqual($titleContainer.find(`.${customClass}`).length, 1, 'custom template content is inside the container');
assert.strictEqual($titleContainer.find(`.${customClass}`).text(), 'Custom Title', 'custom template text is correct');
});

QUnit.test('bottomTemplate result is appended inside container', function(assert) {
const customClass = 'my-custom-bottom';

this.popup.option({
toolbarItems: [
{ text: 'test', toolbar: 'bottom', location: 'center' },
],
bottomTemplate: () => {
return $('<div>').addClass(customClass).text('Custom Bottom');
},
});

const $overlayContent = this.popup.$overlayContent();
const $bottomContainer = $overlayContent.find('.' + POPUP_BOTTOM_CLASS).first();

assert.strictEqual($bottomContainer.find(`.${customClass}`).length, 1, 'custom template content is inside the container');
});

QUnit.test('template result is appended inside container', function(assert) {
this.popup.option('titleTemplate', () => {
return $('<span>')
.addClass(TEMPLATE_WRAPPER_CLASS)
.text('wrapper only');
});

const $overlayContent = this.popup.$overlayContent();
const $titleContainer = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();

assert.strictEqual($titleContainer.text(), 'wrapper only', 'template content is rendered inside the container');
assert.ok($titleContainer.hasClass(POPUP_TITLE_CLASS), 'container still has dx-popup-title class');
});

QUnit.test('template result with multiple classes preserves all classes on result element', function(assert) {
this.popup.option('titleTemplate', () => {
return $('<div>')
.addClass(TEMPLATE_WRAPPER_CLASS)
.addClass('user-class-1')
.addClass('user-class-2')
.text('multi-class');
});

const $overlayContent = this.popup.$overlayContent();
const $titleContainer = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();
const $innerElement = $titleContainer.find('.user-class-1');

assert.strictEqual($innerElement.length, 1, 'user class element is found');
assert.ok($innerElement.hasClass('user-class-2'), 'second user class is preserved');
assert.ok($innerElement.hasClass(TEMPLATE_WRAPPER_CLASS), 'dx-template-wrapper class is preserved on result element');
});

QUnit.test('template that renders result directly into container should work correctly (React-like scenario)', function(assert) {
this.popup.option('titleTemplate', (container) => {
const $container = $(container);
$('<p>').addClass('react-like-content').text('React Title').appendTo($container);
return $container;
});

const $overlayContent = this.popup.$overlayContent();
const $titleContainer = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();

assert.strictEqual($titleContainer.find('.react-like-content').length, 1, 'content is rendered inside container');
assert.strictEqual($titleContainer.find('.react-like-content').text(), 'React Title', 'content text is correct');
});

QUnit.test('container preserves its original classes after template rendering', function(assert) {
this.popup.option('titleTemplate', () => {
return $('<div>').text('test');
});

const $overlayContent = this.popup.$overlayContent();
const $titleContainer = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();

assert.ok($titleContainer.hasClass(POPUP_TITLE_CLASS), 'container still has dx-popup-title class');
});

QUnit.test('_renderByTemplate returns the original container element', function(assert) {
const popup = this.popup;

popup.option('titleTemplate', () => {
return $('<div>').text('return check');
});

const $overlayContent = popup.$overlayContent();
const $topToolbar = popup.topToolbar();
const $titleInDOM = $overlayContent.find(`.${POPUP_TITLE_CLASS}`).first();

assert.strictEqual($topToolbar.get(0), $titleInDOM.get(0), 'topToolbar reference points to the same DOM element as the title container');
});
});
});

QUnit.module('renderGeometry', {
Expand Down
Loading