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): Accept array of index for selectedIndex API #4124

Merged
merged 52 commits into from
Jan 5, 2019

Conversation

abhiomkar
Copy link
Collaborator

@abhiomkar abhiomkar commented Nov 28, 2018

Before this implementation selectedIndex component API (that proxies foundation's setSelectedIndex) use to accept only single number index. This change adds support for multiple indexes that sets the state of checkboxes.

New behaviour for checkbox based list

[ ] option 1
[ ] option 2
[ ] option 3
[ ] option 4

selectedIndex = [1, 3];

[ ] option 1
[x] option 2
[ ] option 3
[x] option 4

selectedIndex = [2, 3];

[ ] option 1
[ ] option 2
[x] option 3
[x] option 4

selectedIndex = [];

[ ] option 1
[ ] option 2
[ ] option 3
[ ] option 4

selectedIndex = 2;

// Throws error

Other changes:

  • getter API for selectedIndex that returns the current state of selection for all variants.
  • Tabindex is set to 0 for currently focused element and will be reverted to first selected element on focus out of list root.
  • new adapter method isFocusInsideList.

BREAKING CHANGE: Introduced new adapter isFocusInsideList for MDC List for improved accessibility.

Fixes #4122
Fixes #3611

@mdc-web-bot
Copy link
Collaborator

All 679 screenshot tests passed for commit 7420515 vs. master! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 679 screenshot tests passed for commit e5664d9 vs. master! 💯🎉

packages/mdc-list/README.md Show resolved Hide resolved
test/screenshot/spec/mdc-list/fixture.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Show resolved Hide resolved
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js 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
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
packages/mdc-list/foundation.js Outdated Show resolved Hide resolved
test/unit/mdc-list/mdc-list.test.js Outdated Show resolved Hide resolved
packages/mdc-list/foundation.js Show resolved Hide resolved
packages/mdc-list/foundation.js Show resolved Hide resolved
test/unit/mdc-list/foundation.test.js Outdated Show resolved Hide resolved
test/unit/mdc-list/foundation.test.js Outdated Show resolved Hide resolved
packages/mdc-list/README.md Outdated Show resolved Hide resolved
packages/mdc-list/README.md Show resolved Hide resolved
@kfranqueiro
Copy link
Contributor

FYI it looks like there are some closure failures to address, and there's a seemingly spurious unit test failure in the last run... Screenshot tests should pass once this is synced with master.

@codecov-io
Copy link

codecov-io commented Jan 5, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@96f663e). Click here to learn what that means.
The diff coverage is 98.47%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #4124   +/-   ##
=========================================
  Coverage          ?   98.49%           
=========================================
  Files             ?      127           
  Lines             ?     5697           
  Branches          ?      761           
=========================================
  Hits              ?     5611           
  Misses            ?       86           
  Partials          ?        0
Impacted Files Coverage Δ
packages/mdc-list/constants.js 100% <ø> (ø)
packages/mdc-list/foundation.js 100% <100%> (ø)
packages/mdc-list/index.js 98.49% <87.5%> (ø)

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 96f663e...38a34de. Read the comment docs.

@mdc-web-bot
Copy link
Collaborator

All 758 screenshot tests passed for commit 2ea1d3b vs. master! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 758 screenshot tests passed for commit 38a34de vs. master! 💯🎉

@abhiomkar abhiomkar merged commit be070a4 into master Jan 5, 2019
@abhiomkar abhiomkar deleted the fix/list_checkbox_selectedindex branch January 5, 2019 12:45
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.

5 participants