-
-
Notifications
You must be signed in to change notification settings - Fork 589
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 data-* and aria-* props to the body table component #211
Conversation
Codecov Report
@@ Coverage Diff @@
## master #211 +/- ##
==========================================
+ Coverage 93.58% 93.62% +0.04%
==========================================
Files 17 17
Lines 748 753 +5
Branches 196 196
==========================================
+ Hits 700 705 +5
Misses 40 40
Partials 8 8
Continue to review full report at Codecov.
|
@@ -163,7 +165,7 @@ class BaseTable extends React.Component { | |||
const columns = this.getColumns(); | |||
|
|||
return ( | |||
<Table className={tableClassName} style={tableStyle} key="table"> | |||
<Table className={tableClassName} style={tableStyle} key="table" {...dataOrAriaProps}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not pass them to
Lines 497 to 502 in 479bd2e
<div | |
ref={this.saveRef('tableNode')} | |
className={className} | |
style={props.style} | |
id={props.id} | |
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thought is that attributes would be assigned on the native elements that these components represent, when available. On Input
it would be on <input>
, on Table
it would go to the <table>
. Others like Select
don't use the native html elements, so then it would go to whatever wrapper makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I see your point, thats where props.id
and props.style
is assigned, so it might make the most sense.
+1 |
Can this one be merged please? I will really aid in an accessibility issue that we are having. |
Closing in favor of #227 |
Pass data-* and aria-* props to the body table component