fix(ui5-select): fix acc finding by PetyaMarkovaBogdanova · Pull Request #13559 · UI5/webcomponents

PetyaMarkovaBogdanova · 2026-05-20T07:12:21Z

8) Select (6 failing)

Interactive accessibility failures in dropdown/listbox/dialog flow.
Repeated signals include missing accessible naming and state/role consistency checks.

Component	Issue	Not by spec	Good to have	Spec refs	Spec quote sentence	Coverage strength
Select	Dropdown/listbox/dialog naming and role/state consistency issues	Yes	No	ACC-253, ACC-264, ACC-270, ACC-272	"Inherit role=dialog of UI5 Dialog control with label='Select: + [Name of field]'" and "see Dialog"	Direct

ui5-webcomponents-bot · 2026-05-20T07:19:43Z

🚀 Deployed on https://pr-13559--ui5-webcomponents-preview.netlify.app

…o fix-acc-select

dobrinyonkov

Thanks for picking this up — the listbox naming fix is a real improvement. Four inline comments below; (1) and (4) would be good to address before merging, (2) and (3) are polish/follow-up.

dobrinyonkov · 2026-06-23T08:17:03Z

 					onClose={this._afterClose}
 					onKeyDown={this._onkeydown}
-					accessibleName={this._isPhone ? this._headerTitleText : undefined}
+					accessibleName={this._isPhone ? (this.ariaLabelText || this._headerTitleText) : undefined}


Spec mismatch — missing Select: prefix (medium).

The spec quote in the PR description literally calls for "Select: + [Name of field]". This sets the mobile dialog's accessibleName to just the field name (e.g. "Country"), not "Select: Country". The dialog role already announces "dialog", so the textual Select: prefix is what distinguishes this dialog from arbitrary other dialogs in the flow — without it, screen-reader users hear "Country, dialog" and lose the cue that this is the Select's value picker.

Could you confirm with the accessibility owner whether the prefix is required? If yes, please introduce a dedicated i18n key (don't hard-code "Select: ") and use it here, roughly:

accessibleName={this._isPhone ? this._effectivePopoverAccessibleName : undefined}

where the getter applies the localized prefix to this.ariaLabelText || this._headerTitleText.

dobrinyonkov · 2026-06-23T08:17:03Z

 						onMouseDown={this._itemMousedown}
 						onItemClick={this._handleItemPress}
 						accessibleRole="ListBox"
+						accessibleName={this.ariaLabelText || this._headerTitleText}


Duplicated expression (low).

this.ariaLabelText || this._headerTitleText now appears on both line 30 and line 71. If the precedence ever changes (e.g. a third source is added, or the Select: prefix from comment above lands), it's two places to keep in sync. Consider extracting a getter in Select.ts —

get _effectivePopoverAccessibleName() { return this.ariaLabelText || this._headerTitleText; }

and using it on both lines. Matches how _headerTitleText is already factored out. Optional but tidy.

dobrinyonkov · 2026-06-23T08:17:03Z

 						onMouseDown={this._itemMousedown}
 						onItemClick={this._handleItemPress}
 						accessibleRole="ListBox"
+						accessibleName={this.ariaLabelText || this._headerTitleText}


Borrowed i18n key — please add a dedicated one (low).

_headerTitleText resolves to INPUT_SUGGESTIONS_TITLE ("All Items"), which was authored for the Input suggestions popover. Now that every Select listbox falls back to this string when the consumer hasn't set a label, it's being used as a Select-specific accessible name even though the key name says otherwise.

Please introduce a Select-specific i18n key (e.g. SELECT_LISTBOX_LABEL / SELECT_POPOVER_TITLE) and have _headerTitleText (or a new getter) resolve to it — even if the English string happens to match. Keeps translations correctly scoped per component and avoids surprises if Input's wording ever diverges.

dobrinyonkov · 2026-06-23T08:17:03Z

 						onMouseDown={this._itemMousedown}
 						onItemClick={this._handleItemPress}
 						accessibleRole="ListBox"
+						accessibleName={this.ariaLabelText || this._headerTitleText}


No test coverage added (medium).

This PR changes ARIA wiring on two surfaces but adds no test. The branch history (two fix(...): fixed failing test commits) suggests this area is easy to regress silently. Two natural assertions belong in the existing accessibility spec at packages/main/cypress/specs/Select.cy.tsx:250:

Open the popover on a <Select accessibleName="Foo"> → the inner [ui5-list] has aria-label="Foo". Add a second case for accessibleNameRef and a third for a Select with no consumer label (asserting the fallback string).

Simulate _isPhone → the [ui5-responsive-popover] has aria-label matching the field's accessible name.

Without these, the next refactor of SelectPopoverTemplate will not catch a regression here.

…n mobile devices

dobrinyonkov

Thanks for the iterations — all four original comments are addressed: the Select: prefix is in, the dedicated i18n keys exist, the duplication is factored into _effectivePopoverAccessibleName, and there are tests for both surfaces. Three new things worth fixing before merge, plus one nit.

dobrinyonkov · 2026-06-24T07:25:27Z

+		&& value !== null
+		&& "key" in value
+		&& "defaultText" in value;
+};


The isI18nText type guard (and the two narrowed branches at lines 1058 and 1142) should be removed — it's working around a stale generated file, not a real type problem.

Generated i18n constants are always typed as I18nText (see packages/main/src/generated/i18n/i18n-defaults.ts — every key is declared const FOO: I18nText = {key, defaultText}). Sibling code in this same file just does:

return Select.i18nBundle.getText(SELECT_ROLE_DESCRIPTION); return Select.i18nBundle.getText(FORM_SELECTABLE_REQUIRED);

no guards needed. The TS error that prompted the workaround was almost certainly because yarn generate hadn't been run after the new keys were added to messagebundle.properties, so the imports in the generated file didn't yet exist. Running yarn generate regenerates the file and the simple form compiles fine.

Leaving the guard in is bad on two counts:

It's dead code (the branches are unreachable in a correctly generated build) — readers will wonder what edge case it covers.

The return "" fallback at line 1062 would silently produce a Select with no listbox label and no mobile dialog label if it ever did fire — masking exactly the regression this PR is supposed to prevent.

Please remove isI18nText, the I18nText named import, and inline the getText calls in both _headerTitleText and _effectivePopoverAccessibleName.

dobrinyonkov · 2026-06-24T07:25:27Z

 						onMouseDown={this._itemMousedown}
 						onItemClick={this._handleItemPress}
 						accessibleRole="ListBox"
+						accessibleName={this.ariaLabelText || this._headerTitleText}


Nit: since _effectivePopoverAccessibleName exists for the popover line, the listbox line could be factored the same way — a small getter _effectiveListAccessibleName returning this.ariaLabelText || this._headerTitleText. Not critical (the two expressions now correctly differ — the popover one carries the Select: prefix), but it keeps the template uniform. Skip if you'd rather not.

dobrinyonkov · 2026-06-24T07:25:27Z

+				.should("have.attr", "accessible-name")
+				.and("equal", _effectivePopoverAccessibleName);
+		});
+	});


This assertion is now circular. It reads the value of _effectivePopoverAccessibleName from the element and then asserts the rendered attribute equals that same value — so if the getter is broken, both sides of the equality break the same way and the test still passes.

The other new test (Select.cy.tsx:325) hard-codes "Select: Countries" for the mobile case, so coverage isn't lost — but consider either deleting this case or changing it to assert a literal string. Either way, the property-readback pattern hides regressions.

dobrinyonkov · 2026-06-24T07:25:27Z

 SELECT_OPTIONS=Select Options

+#XACT: ARIA announcement prefix for Select dialog accessible name on mobile (without trailing space)
+SELECT_POPOVER_ACCESSIBLE_NAME_PREFIX=Select:


Cleaner than embedding the space in the translation — translators can't accidentally drop trailing whitespace, and the join is done explicitly in code. Good fix. (No action needed — leaving the comment here so the rationale is visible in review.)

fix(ui5-select): fix acc finding

9a48ffa

ui5-webcomponents-bot temporarily deployed to preview May 20, 2026 07:19 Inactive

fix(ui5-select): fix acc finding

fa01dc5

PetyaMarkovaBogdanova marked this pull request as ready for review May 20, 2026 14:29

Merge branch 'main' into fix-acc-select

0be0532

ui5-webcomponents-bot temporarily deployed to preview May 20, 2026 14:44 Inactive

PetyaMarkovaBogdanova added 2 commits June 2, 2026 17:51

fix(ui5-select): fixed failing test

33c5c37

Merge branch 'fix-acc-select' of github.com:SAP/ui5-webcomponents int…

f16945d

…o fix-acc-select

PetyaMarkovaBogdanova temporarily deployed to netlify-preview June 2, 2026 15:31 — with GitHub Actions Inactive

ui5-webcomponents-bot temporarily deployed to preview June 2, 2026 15:37 Inactive

fix(ui5-webcomponents): fixed failing test

04a2ef0

PetyaMarkovaBogdanova temporarily deployed to netlify-preview June 3, 2026 06:44 — with GitHub Actions Inactive

ui5-webcomponents-bot temporarily deployed to preview June 3, 2026 06:51 Inactive

Merge branch 'main' into fix-acc-select

317ba3a

PetyaMarkovaBogdanova temporarily deployed to netlify-preview June 4, 2026 10:33 — with GitHub Actions Inactive

PetyaMarkovaBogdanova requested a review from dobrinyonkov June 22, 2026 07:28

dobrinyonkov reviewed Jun 23, 2026

View reviewed changes

PetyaMarkovaBogdanova added 2 commits June 23, 2026 14:47

fix(ui5-select): add accessible name for the listbox in the popover o…

310b4d3

…n mobile devices

Merge branch 'main' into fix-acc-select

1528770

PetyaMarkovaBogdanova temporarily deployed to netlify-preview June 23, 2026 11:59 — with GitHub Actions Inactive

PetyaMarkovaBogdanova added 2 commits June 23, 2026 18:12

fix(ui5-select): fixed failing tests

4a41098

fix(ui5-select): ts error is fixed

466d381

PetyaMarkovaBogdanova temporarily deployed to netlify-preview June 23, 2026 15:18 — with GitHub Actions Inactive

fix(ui5-select): fix ts error

55e0544

PetyaMarkovaBogdanova temporarily deployed to netlify-preview June 23, 2026 15:37 — with GitHub Actions Inactive

dobrinyonkov reviewed Jun 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ui5-select): fix acc finding#13559

fix(ui5-select): fix acc finding#13559
PetyaMarkovaBogdanova wants to merge 12 commits into
mainfrom
fix-acc-select

PetyaMarkovaBogdanova commented May 20, 2026 •

edited

Loading

Uh oh!

ui5-webcomponents-bot commented May 20, 2026 •

edited by sap-ui5-webcomponents-release Bot

Loading

Uh oh!

dobrinyonkov left a comment

Uh oh!

dobrinyonkov Jun 23, 2026

Uh oh!

dobrinyonkov Jun 23, 2026

Uh oh!

dobrinyonkov Jun 23, 2026

Uh oh!

dobrinyonkov Jun 23, 2026

Uh oh!

dobrinyonkov left a comment

Uh oh!

dobrinyonkov Jun 24, 2026

Uh oh!

dobrinyonkov Jun 24, 2026

Uh oh!

dobrinyonkov Jun 24, 2026

Uh oh!

dobrinyonkov Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

PetyaMarkovaBogdanova commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

8) Select (6 failing)

Uh oh!

ui5-webcomponents-bot commented May 20, 2026 • edited by sap-ui5-webcomponents-release Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dobrinyonkov left a comment

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!

dobrinyonkov left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PetyaMarkovaBogdanova commented May 20, 2026 •

edited

Loading

ui5-webcomponents-bot commented May 20, 2026 •

edited by sap-ui5-webcomponents-release Bot

Loading