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

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

Merged
merged 1 commit into from May 17, 2018

Conversation

Projects
None yet
3 participants
@sschadwick
Contributor

sschadwick commented May 16, 2018

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

This comment has been minimized.

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

8 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test-beta Your tests passed on CircleCI!
Details
ci/circleci: test-latest Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing e4ad499...62fbd71
Details
codecov/project 61.04% remains the same compared to e4ad499
Details
deploy/netlify Deploy preview ready!
Details
? '-1'
: null
'aria-rowcount': this.$attrs['aria-rowcount'] ||
this.items.length > this.perPage ? this.items.length : null

This comment has been minimized.

@tmorehouse

tmorehouse Nov 10, 2018

Member

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