Skip to content

fix(Table) allow empty labels #1587

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
Feb 4, 2018
Merged

fix(Table) allow empty labels #1587

merged 1 commit into from
Feb 4, 2018

Conversation

mosinve
Copy link
Member

@mosinve mosinve commented Feb 4, 2018

fixes #1582

@mosinve mosinve requested a review from pi0 February 4, 2018 10:37
@codecov
Copy link

codecov bot commented Feb 4, 2018

Codecov Report

Merging #1587 into dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #1587   +/-   ##
=======================================
  Coverage   59.96%   59.96%           
=======================================
  Files         153      153           
  Lines        2850     2850           
  Branches      782      782           
=======================================
  Hits         1709     1709           
  Misses        821      821           
  Partials      320      320
Impacted Files Coverage Δ
src/components/table/table.js 69.73% <100%> (ø) ⬆️

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 7202c35...7e3a8d1. Read the comment docs.

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Nice!

@pi0 pi0 merged commit 5ee0923 into dev Feb 4, 2018
@pi0 pi0 deleted the fix-table-allow-empty-labels branch February 4, 2018 15:11
@tmorehouse
Copy link
Member

Note this can break ARIA compliance when a column doesn't have a header or an aria label for blind users, as they cannot see the visual context of the column, they need a label.

The field's key should be used as the aria-label on the <th :aria-label="field.key">, and the td's in hte column will need an aria-label when in stacked mode (since the thead is hidden).

tmorehouse added a commit that referenced this pull request Nov 10, 2018
When the fields's label was set to a blank string, this left non-sighted users at a disadvantage as to what the column is about.

This adds the field's key (humanized) to the `aria-label` to provides hint to non-sighted users.

Alternatively if a field title is provided, then the aria-label is not altered.

Addresses ARIA issues introduced by PR #1587
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.

The table uses key for the label when assigned an empty string
3 participants