Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4930,7 +4930,6 @@ test('Grids a11y: Fix the header filter and the column chooser focus issue and u
const dataGrid = new DataGrid('#container');
const filterIconElement = dataGrid.getHeaders().getHeaderRow(0).getHeaderCell(0).getFilterIcon();
const headerFilter = new HeaderFilter();
const columnChooser = dataGrid.getColumnChooser();
const columnChooserButton = dataGrid.getColumnChooserButton();

await t
Expand All @@ -4942,7 +4941,7 @@ test('Grids a11y: Fix the header filter and the column chooser focus issue and u
.ok()
.click(columnChooserButton)
.pressKey('tab tab tab')
.expect(columnChooser.content.focused)
.expect(Selector('.dx-closebutton').focused)
.ok();
})
.before(async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/devextreme/js/__internal/ui/lookup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,7 @@ class Lookup extends DropDownList<LookupProperties> {
hideOnParentScroll: true,
_fixWrapperPosition: false,
_overlayContentRole: 'dialog',
_preventDialogContainerFocus: true,
width: this._isInitialOptionValue('dropDownOptions.width')
? (): number => getOuterWidth(this.$element()) as number
: popupConfig.width,
Expand Down
18 changes: 12 additions & 6 deletions packages/devextreme/js/__internal/ui/overlay/overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -992,14 +992,20 @@ class Overlay<
const $currentElement = $elements?.eq(i) ?? null;
const $reverseElement = $elements?.eq(elementsCount - i) ?? null;

// @ts-expect-error is should can get function as callback
if (!$first && $currentElement.is(selectors.tabbable)) {
$first = $currentElement;
if (!$first && $currentElement) {
// @ts-expect-error is should can get function as callback
const isTabbableAndNotOverlay = $currentElement?.not(`.${OVERLAY_CONTENT_CLASS}`).is(selectors.tabbable);
if (isTabbableAndNotOverlay) {
$first = $currentElement;
}
}

// @ts-expect-error is should can get function as callback
if (!$last && $reverseElement.is(selectors.tabbable)) {
$last = $reverseElement;
if (!$last && $reverseElement) {
// @ts-expect-error is should can get function as callback
const isTabbableAndNotOverlay = $reverseElement?.not(`.${OVERLAY_CONTENT_CLASS}`).is(selectors.tabbable);
if (isTabbableAndNotOverlay) {
$last = $reverseElement;
}
}

if ($first && $last) {
Expand Down
85 changes: 80 additions & 5 deletions packages/devextreme/js/__internal/ui/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ export interface PopoverProperties extends Omit<Properties,
_overlayContentRole?: string;

_describeTarget?: boolean;

_preventDialogContainerFocus?: boolean;
}
class Popover<
TProperties extends PopoverProperties = PopoverProperties,
Expand Down Expand Up @@ -266,6 +268,57 @@ class Popover<
_syncAriaAttributes(): void {
this.setAria('role', this._getEffectiveAriaRole());
this._syncTargetAriaDescription();
this._syncFocusOptions();
}

_syncFocusOptions(): void {
if (this._getEffectiveAriaRole() === 'dialog' && !this.option('_preventDialogContainerFocus')) {
this._setOptionWithoutOptionChange('focusStateEnabled', true);
this._setOptionWithoutOptionChange('tabFocusLoopEnabled', true);
}
}

// Intentional no-op: Focus target logic is inherited from Widget,
// uses in Popup and do not need here.
_renderFocusTarget(): void {}

_getFocusTarget(): dxElementWrapper | null | undefined {
const $firstFocusableTarget = this._findTabbableBounds().$first;
if ($firstFocusableTarget?.length) {
return $firstFocusableTarget;
}

const $overlay = this.$overlayContent();
if ($overlay?.length) {
if ($overlay.attr('tabindex') !== '-1') {
$overlay.attr('tabindex', '-1');
}
return $overlay;
}

return null;
}

_focusTarget(): dxElementWrapper {
return this._getFocusTarget() ?? this.$overlayContent();
}

_restoreTargetFocus(): void {
const $target = this._getAriaDescriptionTargets();
const targetElement = $target.first().get(0);

if (targetElement && domAdapter.getBody().contains(targetElement)) {
// @ts-expect-error trigger should be typed on type 'EventsEngineType'
eventsEngine.trigger($target.first(), 'focus');
}
}

_forceFocusLost(): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please check base method. May we can move there this logic.
Also please check what exactly base method do and why.

@dmlvr dmlvr Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please check base method. Also please check what exactly base method do and why.

The base Overlay._forceFocusLost() checks if the currently focused element is inside the overlay content (this._$content) and blurs (resets) it if so. This prevents focus from being trapped on hidden/invisible elements when the overlay closes, which would break keyboard navigation.

May we can move there this logic?

No. The base Overlay class lacks the role-awareness (_getEffectiveAriaRole()) and target resolution mechanics (_getAriaDescriptionTargets()) that Popover uses to identify and refocus its trigger element.

if (this._getEffectiveAriaRole() === 'dialog') {
this._restoreTargetFocus();
} else {
super._forceFocusLost();
}
}

protected _getAriaRole(): string {
Expand Down Expand Up @@ -295,10 +348,15 @@ class Popover<

_ensurePopoverContentId(): string {
const $overlayContent = this.$overlayContent();
this._popoverContentId = this._popoverContentId
?? $overlayContent.attr('id')
?? `dx-${new Guid()}`;
this.setAria('id', this._popoverContentId, $overlayContent);
const existingId = $overlayContent.attr('id');

if (existingId) {
this._popoverContentId = existingId;
return existingId;
}

this._popoverContentId = this._popoverContentId ?? `dx-${new Guid()}`;
$overlayContent.attr('id', this._popoverContentId);

return this._popoverContentId;
}
Expand Down Expand Up @@ -479,7 +537,7 @@ class Popover<
_detachHoverableOverlay(): void {
const $overlayContent = this.$overlayContent();

if (!$overlayContent.length) {
if (!$overlayContent?.length) {
return;
}

Expand Down Expand Up @@ -882,14 +940,31 @@ class Popover<
_clean(): void {
const { target } = this.option();

const $overlayContent = this.$overlayContent();
if ($overlayContent?.length) {
const existingId = $overlayContent.attr('id');
if (existingId) {
this._popoverContentId = existingId;
}
}

this._detachEscapeKeyHandler();
this._detachEvents(target);
this._detachHoverableOverlay();

super._clean();
}

_shouldResetActiveElement(): boolean {
const activeElement = domAdapter.getActiveElement();
return domAdapter.isNode(activeElement) && !!this._$content?.get(0)?.contains(activeElement);
}

_dispose(): void {
const { visible } = this.option();
if (visible && this._shouldResetActiveElement() && this._getEffectiveAriaRole() === 'dialog') {
this._restoreTargetFocus();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we check if we have focus inside?

}
this._removeTargetAriaDescription();
this._detachEscapeKeyHandler();
super._dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2994,4 +2994,135 @@ QUnit.module('accessibility', {
assert.ok(instance.option('visible'), 'popover remains visible when pointer re-enters overlay before delay expires');
});
});

QUnit.module('dialog mode focus management and accessibility', {
beforeEach() {
this.clock = sinon.useFakeTimers();
this.$element = $('#what');
this.$target = $('#where');
},
afterEach() {
this.clock.restore();
}
}, () => {
QUnit.test('Popover in dialog mode should enable focusStateEnabled and tabFocusLoopEnabled on show', function(assert) {
const instance = new Popover(this.$element, {
target: this.$target,
toolbarItems: [{ text: 'OK' }],
visible: false,
});

instance.show();
this.clock.tick(0);

assert.strictEqual(instance.option('focusStateEnabled'), true, 'focusStateEnabled is enabled for dialog mode');
assert.strictEqual(instance.option('tabFocusLoopEnabled'), true, 'tabFocusLoopEnabled is enabled for dialog mode');
});

QUnit.test('Popover in dialog mode should move focus inside on show and restore focus to target when hidden', function(assert) {
this.$target.attr('tabindex', 0).focus();
assert.strictEqual(document.activeElement, this.$target.get(0), 'target is focused before show');

const instance = new Popover(this.$element, {
target: this.$target,
toolbarItems: [{ widget: 'dxButton', options: { text: 'OK' } }],
visible: false,
});

instance.show();
this.clock.tick(500);

const isFocusInside = $(document.activeElement).closest(wrapper()).length > 0;
assert.strictEqual(isFocusInside, true, 'focus moved inside popover wrapper on show');

instance.hide();
this.clock.tick(500);

assert.strictEqual(document.activeElement, this.$target.get(0), 'focus is restored to target after hide');
});

QUnit.test('Popover in dialog mode should loop focus from last to first element on tab keypress', function(assert) {
const instance = new Popover(this.$element, {
target: this.$target,
toolbarItems: [
{ widget: 'dxButton', options: { text: 'OK' } },
{ widget: 'dxButton', options: { text: 'Cancel' } }
],
visible: false,
});

instance.show();
this.clock.tick(500);

const bounds = instance._findTabbableBounds();
const firstFocusable = bounds.$first.get(0);
const lastFocusable = bounds.$last.get(0);

$(lastFocusable).focus();

const tabEvent = $.Event('keydown', { key: 'Tab' });
$(document).trigger(tabEvent);

assert.strictEqual(document.activeElement, firstFocusable, 'focus looped to the first element');
});

QUnit.test('Popover in dialog mode should loop focus from first to last element on shift+tab keypress', function(assert) {
const instance = new Popover(this.$element, {
target: this.$target,
toolbarItems: [
{ widget: 'dxButton', options: { text: 'OK', } },
{ widget: 'dxButton', options: { text: 'Cancel', } }
],
visible: false,
});

instance.show();
this.clock.tick(500);

const bounds = instance._findTabbableBounds();
const firstFocusable = bounds.$first.get(0);
const lastFocusable = bounds.$last.get(0);

$(firstFocusable).focus();

const shiftTabEvent = $.Event('keydown', { key: 'Tab', shiftKey: true });
$(document).trigger(shiftTabEvent);

assert.strictEqual(document.activeElement, lastFocusable, 'focus looped to the last element');
});

QUnit.test('Popover in dialog mode should focus first tabbable element inside content on show', function(assert) {
const instance = new Popover(this.$element, {
target: this.$target,
contentTemplate: function() {
return $('<div><input id="input1" /><input id="input2" /></div>');
},
toolbarItems: [{ text: 'OK' }],
visible: false,
});

instance.show();
this.clock.tick(500);

const $input1 = $('#input1');
assert.strictEqual(document.activeElement, $input1.get(0), 'first tabbable element is focused');
});

QUnit.test('Popover in dialog mode should restore focus to target on dispose when visible', function(assert) {
this.$target.attr('tabindex', 0).focus();

const instance = new Popover(this.$element, {
target: this.$target,
toolbarItems: [{ text: 'OK' }],
visible: false,
});

instance.show();

instance.dispose();

assert.strictEqual(document.activeElement, this.$target.get(0), 'focus is restored to target after dispose');
});
});
});

Loading