Skip to content

fix(NcSelect): announce selected option to screen readers#8342

Open
pringelmann wants to merge 3 commits intomainfrom
fix/8322/ncselect-a11y-selected-announcement
Open

fix(NcSelect): announce selected option to screen readers#8342
pringelmann wants to merge 3 commits intomainfrom
fix/8322/ncselect-a11y-selected-announcement

Conversation

@pringelmann
Copy link
Copy Markdown
Contributor

☑️ Resolves

🖼️ Screenshots

N/A - behavioural fix, no visual change

🚧 Tasks

  • Add aria-describedby on the search input pointing to a hidden span with the selected value. Screen readers announce it on focus.
  • Inject a visually-hidden <item>, selected marker into the #option slot for the currently selected option. Screen readers pick this up via aria-activedescendant while navigating the list.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport to stable8 for maintained Vue 2 version or not applicable

Note on aria-selected:

vue-select sets aria-selected on the <li> elements in its own render function - NcSelect can't control these attributes from the wrapper level without fragile DOM manipulation that gets overwritten on re-renders. The visually-hidden text approach works within Vue's reactivity and doesn't fight the upstream library.

@pringelmann pringelmann added this to the 9.6.0 milestone Mar 23, 2026
@pringelmann pringelmann self-assigned this Mar 23, 2026
@pringelmann pringelmann added bug Something isn't working 2. developing Work in progress feature: select Related to the NcSelect* components accessibility Making sure we design for the widest range of people possible, including those who have disabilities 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 23, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.68%. Comparing base (6eec54e) to head (4a32b69).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8342      +/-   ##
==========================================
+ Coverage   52.37%   54.68%   +2.31%     
==========================================
  Files         104      104              
  Lines        3393     3394       +1     
  Branches      989      990       +1     
==========================================
+ Hits         1777     1856      +79     
+ Misses       1360     1300      -60     
+ Partials      256      238      -18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Seems to work 👍
But I wonder if this should directly implemented in vue-select?
https://github.com/nextcloud-libraries/vue-select/

@susnux susnux requested a review from ShGKme March 24, 2026 09:46
@pringelmann
Copy link
Copy Markdown
Contributor Author

Seems to work 👍 But I wonder if this should directly implemented in vue-select? https://github.com/nextcloud-libraries/vue-select/

Good question: the aria-selected bug in vue-select (it uses typeAheadPointer instead of actual selection state) is worth filing upstream - happy to open an issue there. That said, the visually-hidden ", selected" text needs to live here regardless, since it goes through the translation system.

Comment thread src/components/NcSelect/NcSelect.vue Outdated
@pringelmann
Copy link
Copy Markdown
Contributor Author

Seems to work 👍 But I wonder if this should directly implemented in vue-select? https://github.com/nextcloud-libraries/vue-select/

Turns out this was already fixed in the fork, but seems we can't use that here since it still targets Vue 2

- Add aria-describedby to announce the selected value when the
  combobox receives focus
- Inject a visually-hidden ", selected" marker into the option
  content so screen readers identify the selected option while
  browsing the dropdown

Fixes #8322

Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
@pringelmann pringelmann force-pushed the fix/8322/ncselect-a11y-selected-announcement branch from d2acefa to e104429 Compare March 24, 2026 16:15
Comment thread src/components/NcSelect/NcSelect.vue Outdated
:name="String(option[localLabel])"
:search="search" />
:search="search"
:aria-hidden="isOptionCurrentlySelected(option) || undefined" />
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.

Why not use native markup like aria-selected and role="option"?

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.

vue-select already puts role="option" and aria-selected on the <li> elements, but sets aria-selected based on keyboard focus position (typeAheadPointer) rather than actual selection - that's the root bug. The #option slot we use only controls the content inside each <li>, not the <li> attributes, so we can't override aria-selected from here without DOM manipulation that gets stomped on re-render.

This is already solved in our fork of vue-select, but we can't use that until it is migrated to Vue 3.

This comment was marked as outdated.

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.

This is already solved in our fork of vue-select, but we can't use that until it is migrated to Vue 3.

Ah wait too fast... 🙈

Then we should push forward the migration instead of tinker with our usage of it, as we will fail the other accessibility issues as well (those we fixed in the fork).
nextcloud-libraries/vue-select#25

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.

I will review effort required to bring it to completion

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.

Not quite a drop-in replacement, but is doable. Will revert the fix and switch to nextcloud/vue-select

Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
@pringelmann pringelmann requested a review from skjnldsv April 1, 2026 09:16
@Antreesy Antreesy modified the milestones: 9.6.0, 9.7.0 Apr 15, 2026
@Antreesy Antreesy modified the milestones: 9.7.0, 9.7.1 Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: select Related to the NcSelect* components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NcSelect: selected option not announced to screen readers

4 participants