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

Pass name prop down to the table selection input #9054

Merged
merged 1 commit into from Jan 31, 2018

Conversation

@mgrdevport
Copy link
Contributor

@mgrdevport mgrdevport commented Jan 21, 2018

Resolves #9050
This PR adds the name prop to the table getCheckboxProps function to pass it down to the checkbox/radio input per row.

@codecov
Copy link

@codecov codecov bot commented Jan 21, 2018

Codecov Report

Merging #9054 into feature-3.2 will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature-3.2   #9054   +/-   ##
===========================================
  Coverage         85.5%   85.5%           
===========================================
  Files              195     195           
  Lines             4603    4603           
  Branches          1282    1282           
===========================================
  Hits              3936    3936           
  Misses             663     663           
  Partials             4       4
Impacted Files Coverage Δ
components/table/Table.tsx 93.15% <ø> (ø) ⬆️
components/table/SelectionBox.tsx 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 842c7f2...bae4c95. Read the comment docs.

@yesmeck
Copy link
Member

@yesmeck yesmeck commented Jan 22, 2018

I think we can pass the whole object which getCheckboxProps returns to Checkbox.

@yesmeck
Copy link
Member

@yesmeck yesmeck commented Jan 22, 2018

Please PR to feature-3.2 branch.

@mgrdevport mgrdevport force-pushed the mgrdevport:#9050 branch Jan 22, 2018
@mgrdevport mgrdevport changed the base branch from master to feature-3.2 Jan 22, 2018
@mgrdevport mgrdevport force-pushed the mgrdevport:#9050 branch Jan 23, 2018
@yesmeck
Copy link
Member

@yesmeck yesmeck commented Jan 30, 2018

@mgrdevport mgrdevport force-pushed the mgrdevport:#9050 branch 2 times, most recently to 6c55466 Jan 30, 2018
@mgrdevport
Copy link
Contributor Author

@mgrdevport mgrdevport commented Jan 30, 2018

I had to add the jest config param "mapCoverage" to get accurate coverage results (locally and for codecov).
Is there any reason why the parameter was not added before?
Now I can see the correct line mappings for the code coverage result on each file.

@yesmeck
Copy link
Member

@yesmeck yesmeck commented Jan 31, 2018

I had to add the jest config param "mapCoverage" to get accurate coverage results (locally and for codecov).

I'v added it yesterday 4a92772

components/table/SelectionBox.tsx Outdated
const { checked } = this.state;

if (type === 'radio') {
return (
<Radio
disabled={disabled}
onChange={onChange}
value={rowIndex}

This comment has been minimized.

@yesmeck

yesmeck Jan 31, 2018
Member

Should we keep value?

.jest.js Outdated
@@ -38,6 +38,7 @@ module.exports = {
'!components/*/locale/index.tsx',
'!components/*/__tests__/**/type.tsx',
],
mapCoverage: true,

This comment has been minimized.

@yesmeck

yesmeck Jan 31, 2018
Member

Could you remove it? I've added this option.

@mgrdevport mgrdevport force-pushed the mgrdevport:#9050 branch to bae4c95 Jan 31, 2018
@mgrdevport
Copy link
Contributor Author

@mgrdevport mgrdevport commented Jan 31, 2018

@yesmeck
I should have a look into the master commits more often - thanks.
Value prop on the radio is back again.

@yesmeck yesmeck merged commit 5fd741b into ant-design:feature-3.2 Jan 31, 2018
4 checks passed
4 checks passed
codecov/patch 100% of diff hit (target 85.5%)
Details
codecov/project 85.5% (+0%) compared to 842c7f2
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk No dependency changes
Details
@yesmeck
Copy link
Member

@yesmeck yesmeck commented Jan 31, 2018

Thanks!

@mgrdevport mgrdevport deleted the mgrdevport:#9050 branch Jan 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants