Skip to content

Address grids column headers accessibility issues - active descendant, what is announced by SRs - 20.0.x #15982

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

Jump to bottom
Merged
Parent: Mass Merging 20.0.x to master
merged 17 commits into from
Aug 8, 2025
Merged

Address grids column headers accessibility issues - active descendant, what is announced by SRs - 20.0.x #15982

merged 17 commits into from
Aug 8, 2025

Conversation

@ddaribo
Copy link
Contributor

@ddaribo ddaribo commented Jun 24, 2025 •
edited
Loading

Closes #15962

  • 1. FIX aria-activedescendant on headers + TEST ❗(likely a regression)
  • 2. Add aria-sort on column headers + update SPEC
  • 3. Icons should be aria-hidden (i.e. not read by SR/UA) - separate PR
  • 4. For a virtualized grid, cell row/col indexes are not announced properly

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@ddaribo ddaribo changed the base branch from master to 20.0.x June 24, 2025 13:54
@ddaribo ddaribo added ♿ a11y When the issue or PR is related to accessibility version: 20.0.x labels Jun 24, 2025
@ddaribo ddaribo force-pushed the bpachilova/column-headers-a11y-15962 branch from ecf4a33 to d4f5859 Compare July 2, 2025 12:55
@ddaribo ddaribo force-pushed the bpachilova/column-headers-a11y-15962 branch from d4f5859 to 2c9a5b3 Compare July 2, 2025 12:58
@ddaribo ddaribo marked this pull request as ready for review July 3, 2025 06:17
@ddaribo ddaribo requested a review from damyanpetev July 3, 2025 06:19
@ChronosSF ChronosSF requested a review from rkaraivanov July 14, 2025 08:09
damyanpetev
damyanpetev reviewed Jul 15, 2025
View reviewed changes
projects/igniteui-angular/src/lib/grids/cell.component.ts Outdated
Comment on lines 520 to 521
let describeBy = this.grid.headerGroupsList
.find(hg => hg.column.field === this.column.field)?.headerID || '';
Copy link
Member

Choose a reason for hiding this comment

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

Hm, is this because the column didn't have a field or because the description moved? Either way, while the header group list might be large in most cases, would generally avoid having getters for bindings that run on each change detection do any meaningful amount of work (like search) if it can be avoided.

projects/igniteui-angular/src/lib/grids/grid-base.directive.ts Outdated
Comment on lines 1849 to 1850
const totalRows = (this as any).totalItemCount ?? this.data?.length ?? 0;
return (this.paginator ? this._totalRecords : totalRows) + 1;
Copy link
Member

Choose a reason for hiding this comment

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

Eh, (this as any)? Surely that's not needed 👀
Also, rows are manipulated not just by the paginator, so that might not be the only case. Was going to ask if dataView length might do the trick, but by the look of it we do want to announce the full data set?
If so, I'm not entirely sure the virtualization index (which I believe drive row index) will match that in all cases.
Also would filtering and grouping need to be involved to determine the final count? Because those will affect the row index regardless IIRC. Basically, anything in the pipeline that manipulates the data collection is before the for-of even gets the data.

It might be the simplest way to poke the for-of state for the length of items and only detect remote/paginated scenarios to look for _totalRecords / totalItemCount

@ddaribo ddaribo force-pushed the bpachilova/column-headers-a11y-15962 branch from e9a6b89 to e8be376 Compare July 21, 2025 11:30
@ddaribo ddaribo force-pushed the bpachilova/column-headers-a11y-15962 branch from 308b795 to 5eccd74 Compare July 21, 2025 12:08
@ddaribo ddaribo requested a review from damyanpetev July 21, 2025 12:24
@ddaribo ddaribo added the ❌ status: awaiting-test PRs awaiting manual verification label Jul 21, 2025
@ddaribo ddaribo changed the title Address grids column headers accessibility issues - active descendant, what is announced by SRs Address grids column headers accessibility issues - active descendant, what is announced by SRs - 20.0.x Jul 30, 2025
@IvanKitanov17 IvanKitanov17 added ✅ status: verified Applies to PRs that have passed manual verification 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification ✅ status: verified Applies to PRs that have passed manual verification 💥 status: in-test PRs currently being tested labels Aug 8, 2025
@kacheshmarova kacheshmarova merged commit 2f9cfc0 into 20.0.x Aug 8, 2025
5 checks passed
@kacheshmarova kacheshmarova deleted the bpachilova/column-headers-a11y-15962 branch August 8, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkaraivanov rkaraivanov rkaraivanov approved these changes

@damyanpetev damyanpetev Awaiting requested review from damyanpetev

Assignees

@IvanKitanov17 IvanKitanov17

Labels

aria-support grid: hierarchical-grid grid: pivot grid: tree-grid grid version: 20.0.x ♿ a11y When the issue or PR is related to accessibility ✅ status: verified Applies to PRs that have passed manual verification

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Grids: column headers accessibility issues

5 participants

@ddaribo @damyanpetev @rkaraivanov @kacheshmarova @IvanKitanov17