Popover: support content reading on opening by EugeniyKiyashko · Pull Request #34311 · DevExpress/DevExtreme

EugeniyKiyashko · 2026-07-15T09:38:00Z

No description provided.

Copilot

Pull request overview

This PR improves accessibility behavior for Popover-family overlays by ensuring assistive technologies can consistently read popover content via aria-describedby when appropriate, while keeping role/aria-labelledby behavior correct across different interaction modes (tooltip-like vs dialog-like).

Changes:

Add Popover-side syncing of overlay content id and target aria-describedby, including cleanup on option changes and dispose.
Refine role/aria-labelledby behavior for Popover/Tooltip/Popup (e.g., dialog-mode vs tooltip-mode, and titleTemplate edge cases).
Extend unit (QUnit) and e2e a11y coverage for the new role/label/description behaviors across Popover, Tooltip, Popup, ActionSheet, and Lookup.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/devextreme/testing/tests/DevExpress.ui.widgets/tooltip.tests.js	Adds a11y regression tests for Tooltip role behavior with title/close button and toolbar items.
packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js	Adds tests ensuring `aria-labelledby` isn’t set when the title label element is missing due to a custom `titleTemplate`.
packages/devextreme/testing/tests/DevExpress.ui.widgets/popover.tests.js	Adds comprehensive tests for target `aria-describedby` wiring, stability, cleanup, and role/label interactions.
packages/devextreme/testing/tests/DevExpress.ui.widgets/actionSheet.tests.js	Verifies ActionSheet popover mode does not add `aria-describedby` to its target.
packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js	Ensures Lookup popover mode stays dialog-role and does not describe the input element.
packages/devextreme/js/__internal/ui/tooltip.ts	Adjusts Tooltip to preserve legacy role rules and avoid Popover aria-describedby syncing (but needs a fix to keep role syncing).
packages/devextreme/js/__internal/ui/popup/popup.ts	Prevents dangling `aria-labelledby` when a custom `titleTemplate` doesn’t render a label element; resync on `titleTemplate` changes.
packages/devextreme/js/__internal/ui/popover/popover.ts	Implements aria role computation/sync, stable overlay content id generation, and target `aria-describedby` sync/cleanup.
packages/devextreme/js/__internal/ui/lookup.ts	Switches Lookup popover role forcing to internal `_overlayContentRole` instead of direct DOM mutation.
packages/devextreme/js/__internal/ui/action_sheet.ts	Switches ActionSheet popover role forcing to internal `_overlayContentRole` instead of direct DOM mutation.
packages/devextreme/js/__internal/scheduler/header/calendar.ts	Disables target description (`_describeTarget: false`) for an interactive calendar overlay to avoid noisy descriptions.
e2e/testcafe-devextreme/tests/accessibility/popover.ts	Updates option matrix and a11y configuration/selector to cover new behaviors for visible and hidden popovers.

+  // NOTE: Tooltip manages the target relationship itself (_toggleAriaAttributes),
+  // so the inherited Popover sync must stay disabled to avoid a second describedby id.
+  _syncAriaAttributes(): void {}


+      } else if (previousElements.has(element)) {
+        // NOTE: A pre-existing token this instance did not add is foreign,
+        // so it is never claimed and cleanup cannot strip a manual description.
+        describedElements.push(element);
+      }


+// NOTE: dialog-mode popovers (toolbarItems or showTitle + showCloseButton) have no
+// accessible name unless a title is set. Providing a default dialog name is a separate
+// dialog-labeling task (see dialog-labeling-known-issue.md), so the best-practice rule
+// is disabled for the combinations where the name is intentionally absent.
+// Re-enable this rule once that task lands.


…tooltip (#34467)

Raushen · 2026-07-27T14:20:05Z

+    return isDialog ? 'dialog' : 'tooltip';
+  }
+
+  _getEffectiveAriaRole(): string {


Should it be on the parent level? Popup or overlay?

we remane overlayContentRole to popoverContent role and save this method here

Raushen · 2026-07-27T14:21:09Z

+  // technologies, so the title labels the overlay only in dialog mode.
+  _toggleAriaLabel(): void {
+    if (this._getEffectiveAriaRole() === 'tooltip') {
+      this.$overlayContent().attr('aria-labelledby', null);


We need to use setAria() method

Raushen · 2026-07-27T14:22:53Z

  _toggleAriaLabel(): void {
    const { title, showTitle } = this.option();
-    const shouldSetAriaLabel = showTitle && Boolean(title);
+    const $label = this._$topToolbar?.find(`.${TOOLBAR_LABEL_CLASS}`).eq(0);


Why we need to search in DOM? Toolbar should provide its own method like getLabel()

Toolbar doesn't have this method - https://github.com/DevExpress/DevExtreme/blob/feature/26_2_popover_support_content_reading/packages/devextreme/js/__internal/ui/toolbar/toolbar.base.ts#L445

we have to update Toolbar

Raushen · 2026-07-27T14:26:06Z

    const titleId = shouldSetAriaLabel ? new Guid().toString() : null;

-    this._$topToolbar?.find(`.${TOOLBAR_LABEL_CLASS}`).eq(0).attr('id', titleId);
+    $label?.attr('id', titleId);


Why we mutate DOM through private API in other component?

Toolbar doesn't have this method - https://github.com/DevExpress/DevExtreme/blob/feature/26_2_popover_support_content_reading/packages/devextreme/js/__internal/ui/toolbar/toolbar.base.ts#L445

we have to update Toolbar

Raushen · 2026-07-27T14:27:39Z

-      id: this._contentId,
-    });
+  // NOTE: dxTooltip keeps its legacy role behavior: only toolbarItems make it
+  // a dialog; the Popover showTitle/showCloseButton predicate does not apply.


Do we really need comments here?
How about set protected modifier so it would clear we overload this method in other places?

set protected and remove comment

Raushen · 2026-07-27T14:28:49Z

+
+  _getEffectiveAriaRole(): string {
+    // eslint-disable-next-line @typescript-eslint/naming-convention
+    const { _overlayContentRole } = this.option();


Or we need to rename the property?

we have to remane to _popoverContentRole

Raushen · 2026-07-27T18:43:58Z

+
+  _ensurePopoverContentId(): string {
+    const $overlayContent = this.$overlayContent();
+    const existingId = $overlayContent.attr('id');


We should not rely buisness logic to DOM

we use lazyInit here instead of DOM manipulation

Raushen · 2026-07-27T18:48:40Z

+  }
+
+  _getActualAriaRole(): string | undefined {
+    return this.$overlayContent().attr('role') ?? undefined;


Why we do not use private _overlayContentRole here and we have to override the logig?

I assume the original idea was to have a sort of safeguard. If the user set a custom role via elementAttr, we would check for its existence here and avoid overwriting it. However, this only worked with the _lastAppliedRole variable. Now we remove it

we have to remove it

Raushen · 2026-07-27T18:52:00Z

+    const { target } = this.option();
+    const elements: Element[] = [];
+
+    $(target).each((_, node) => {


For now a target supports only one element.

we have to update it, check only one target

Raushen · 2026-07-27T18:56:09Z

+    return Boolean(target) && Boolean(_describeTarget) && this._getEffectiveAriaRole() === 'tooltip';
+  }
+
+  _getAriaDescriptionTargets(): dxElementWrapper {


We have to reuse getTargetElement logic from here.

let's create getTargetElement

Raushen · 2026-07-27T19:09:14Z

+
+    const id = this._ensurePopoverContentId();
+
+    if (this._ariaDescriptionId && this._ariaDescriptionId !== id) {


Why id can be changed?

analiz and refactor _syncAriaAttributes

Raushen · 2026-07-27T19:11:54Z

          this.option('fullScreen', false);
        }
        break;
      case 'target':


Should we sync aria attributes when target changed?

EugeniyKiyashko self-assigned this Jul 15, 2026

Copilot AI review requested due to automatic review settings July 15, 2026 09:38

EugeniyKiyashko requested review from a team as code owners July 15, 2026 09:38

EugeniyKiyashko added the 26_2 label Jul 15, 2026

EugeniyKiyashko marked this pull request as draft July 15, 2026 09:38

Copilot started reviewing on behalf of EugeniyKiyashko July 15, 2026 09:38 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Popover: support content reading on opening

5ccace2

EugeniyKiyashko force-pushed the feature/26_2_popover_support_content_reading branch from 869b7fe to 5ccace2 Compare July 15, 2026 12:44

Raushen and others added 4 commits July 22, 2026 03:04

Apply aria role to containers

f5e4c3b

Add jest tests

1b99a65

Fix QUnit tests

3ccd065

Popover: update aria-describedby logic in popover and remove it from …

72272ac

…tooltip (#34467)

Raushen reviewed Jul 27, 2026

View reviewed changes

dmlvr and others added 3 commits July 28, 2026 09:16

Merge branch 'main' into feature/26_2_popover_support_content_reading

a85bed3

refactoring

048ec5f

remove multiple target tests

6947bd8


		const id = this._ensurePopoverContentId();

		if (this._ariaDescriptionId && this._ariaDescriptionId !== id) {

Uh oh!

Conversation

EugeniyKiyashko commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Raushen Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmlvr Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmlvr Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmlvr Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmlvr Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmlvr Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Raushen Jul 27, 2026 •

edited

Loading

dmlvr Jul 28, 2026 •

edited

Loading

dmlvr Jul 28, 2026 •

edited

Loading

dmlvr Jul 28, 2026 •

edited

Loading

dmlvr Jul 28, 2026 •

edited

Loading

dmlvr Jul 28, 2026 •

edited

Loading