Skip to content

fix(table): Fix Table aria-rowcount #1836

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

Merged
merged 1 commit into from
May 17, 2018
Merged

fix(table): Fix Table aria-rowcount #1836

merged 1 commit into from
May 17, 2018

Conversation

sschadwick
Copy link
Contributor

Currently the table component uses this code to set the aria-rowcount property:

'aria-rowcount': this.$attrs['aria-rowcount'] || (this.perPage && this.perPage > 0)
    ? '-1'  
    : null

https://www.w3.org/TR/wai-aria-1.1/#aria-rowcount
If all of the rows are present in the DOM, it is not necessary to set this attribute as the user agent can automatically calculate the total number of rows. However, if only a portion of the rows is present in the DOM at a given moment, this attribute is needed to provide an explicit indication of the number of rows in the full table.

Authors MUST set the value of aria-rowcount to an integer equal to the number of rows in the full table. If the total number of rows is unknown, authors MUST set the value of aria-rowcount to -1 to indicate that the value should not be calculated by the user agent.

According to the WAI-ARIA spec, aria-rowcount should only be set to -1 if the total number of rows is unknown. The current code has -1 set as the default for any paged tables.
Furthermore, if all of items are currently displayed on the table, we do not need to set aria-rowcount as accessibility readers will automatically count these.

I propose the following code change:

'aria-rowcount': this.$attrs['aria-rowcount'] ||
    this.items.length > this.perPage ? this.items.length : null

@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #1836 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #1836   +/-   ##
=======================================
  Coverage   61.04%   61.04%           
=======================================
  Files         154      154           
  Lines        2862     2862           
  Branches      791      791           
=======================================
  Hits         1747     1747           
  Misses        800      800           
  Partials      315      315
Impacted Files Coverage Δ
src/components/table/table.js 76.34% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4ad499...62fbd71. Read the comment docs.

@pi0 pi0 merged commit e3e5439 into bootstrap-vue:dev May 17, 2018
? '-1'
: null
'aria-rowcount': this.$attrs['aria-rowcount'] ||
this.items.length > this.perPage ? this.items.length : null
Copy link
Member

Choose a reason for hiding this comment

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

This assumes that this.items is an array. It isn't when it is a provider function. When a provider, this.items.length actually returns the argument count the provider function is expecting (i.e. 1 or 2).

PR #2149 fixes this.

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

Successfully merging this pull request may close these issues.

3 participants