Skip to content

Address grids column headers accessibility issues - active descendant, what is announced by SRs #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.

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

Open
wants to merge 13 commits into
base: 20.0.x
Choose a base branch
from

Conversation

ddaribo
Copy link
Contributor

@ddaribo ddaribo commented Jun 24, 2025

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
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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grids: column headers accessibility issues
2 participants