Skip to content

Conversation

MariannaMilovanova
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Jul 11, 2018

Codecov Report

Merging #6 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
+ Coverage   97.87%   97.95%   +0.08%     
==========================================
  Files           1        1              
  Lines          47       49       +2     
==========================================
+ Hits           46       48       +2     
  Misses          1        1
Impacted Files Coverage Δ
src/index.js 97.95% <100%> (+0.08%) ⬆️

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 f7e0ec2...f89acc3. Read the comment docs.

@coveralls
Copy link

coveralls commented Jul 11, 2018

Coverage Status

Coverage increased (+0.5%) to 93.506% when pulling f89acc3 on MariannaMilovanova:feature/add-custom-classes-to-rows into f7e0ec2 on jorrit:master.

Copy link
Owner

@jorrit jorrit left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, Marianna. Could you take a look at these review comments and address them? Thanks!

];
const keyGetter = r => r.a;

const withClasses = true;
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this line into the test function.

src/index.js Outdated
};

function headerClass(withClasses, key) {
return withClasses ? { className: `th-${key}` } : {};
Copy link
Owner

Choose a reason for hiding this comment

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

I think header-${key} is more descriptive than th-${key}.

function rowClass(withClasses, key) {
return withClasses ? { className: `tr-${key}` } : {};
}

Copy link
Owner

Choose a reason for hiding this comment

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

I think row-${key} is more descriptive than row-${key}.

README.md Outdated
| | `object` | CSS styles to apply to the parent table tag. |
| | `function` | Function returning one of the above. The function receives a state object with boolean property narrow indicating if the current presentation is narrow or wide. |
| `initialNarrow` | `bool` | Initially render the table in narrow mode when rendering serverside. Set to true when you expect the browser to be narrow to prevent rerendering client side. |
| `withClasses` | `bool` | Add classNames to each rows and headers cells by using keyGetter function. |
Copy link
Owner

Choose a reason for hiding this comment

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

Please change to 'Add unique class names to each row and header cell, respectively row-KEY and header-KEY.'.

@MariannaMilovanova
Copy link
Contributor Author

MariannaMilovanova commented Sep 5, 2018

@jorrit please see my changes

@jorrit jorrit merged commit f3e178f into jorrit:master Sep 17, 2018
@jorrit
Copy link
Owner

jorrit commented Sep 17, 2018

My apologies for the delay. 0.6.0 has been released with this feature. Thanks for your contribution!

@MariannaMilovanova
Copy link
Contributor Author

Thank you!

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.

4 participants