Skip to content
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(list): Update ARIA attributes for radio/checkbox based list #4055

Merged
merged 24 commits into from Nov 28, 2018

Conversation

abhiomkar
Copy link
Contributor

@abhiomkar abhiomkar commented Nov 5, 2018

This change updates the aria attributes (aria-selected or aria-checked) for single select, checkbox or radio based lists.

BREAKING CHANGE: Replaced toggleCheckbox adapter method with setCheckedCheckboxOrRadioAtIndex and added 3 more new adapter methods for improved accessibility.

Fixes #3659, #3403, #4107

packages/mdc-list/README.md Show resolved Hide resolved
packages/mdc-list/README.md Outdated Show resolved Hide resolved
packages/mdc-list/constants.js Outdated Show resolved Hide resolved
packages/mdc-list/index.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Show resolved Hide resolved
@abhiomkar
Copy link
Contributor Author

abhiomkar commented Nov 15, 2018 via email

Copy link
Contributor

@williamernest williamernest left a comment

Choose a reason for hiding this comment

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

LGTM

packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/index.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
Copy link
Contributor

@kfranqueiro kfranqueiro left a comment

Choose a reason for hiding this comment

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

This is currently failing unit test due to coverage %. One easy place for improvement is that the hasCheckboxAtIndex branch of setSelectedIndex is uncovered.

packages/mdc-list/mdc-list.scss Outdated Show resolved Hide resolved
packages/mdc-list/mdc-list.scss Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 28, 2018

Codecov Report

Merging #4055 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4055   +/-   ##
=======================================
  Coverage   98.36%   98.36%           
=======================================
  Files         126      126           
  Lines        5637     5637           
  Branches      755      755           
=======================================
  Hits         5545     5545           
  Misses         92       92
Impacted Files Coverage Δ
packages/mdc-list/constants.js 100% <ø> (ø) ⬆️
packages/mdc-list/foundation.js 100% <100%> (ø) ⬆️
packages/mdc-list/index.js 100% <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 a6b028d...b4554a6. Read the comment docs.

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

Successfully merging this pull request may close these issues.

None yet

6 participants