-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[added] TabbedArea NavItem renderTab() className [fixes] #516 #517
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
Conversation
@@ -108,12 +108,14 @@ const TabbedArea = React.createClass({ | |||
}, | |||
|
|||
renderTab(child) { | |||
let key = child.props.eventKey; | |||
let {eventKey, className, tab, ...other} = child.props; |
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.
For what reason all ...other
props could be applied from child to parent ?
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.
The variable 'child' is not accurate. It is the existing element being
rendered. In the 'render' function, it calls:
ValidComponentChildren.map(this.props.children, renderTabIfSet, this)
I think you are right and that I should remove this. It would copy the
TabPane properties to the corresponding Nav header.
On Tue, Apr 14, 2015 at 12:53 PM, Alexander Shemetovsky <
notifications@github.com> wrote:
In src/TabbedArea.js
#517 (comment)
:@@ -108,12 +108,14 @@ const TabbedArea = React.createClass({
},renderTab(child) {
- let key = child.props.eventKey;
- let {eventKey, className, tab, ...other} = child.props;
For what reason all ...other props could be applied from child to parent ?
—
Reply to this email directly or view it on GitHub
https://github.com/react-bootstrap/react-bootstrap/pull/517/files#r28363804
.
Can you add some tests to ensure that classNames are getting applied as you expect? This way this feature is not accidentally broken in the future. Also, after you add some tests can you squash the commits. There's no need for multiple commits for this change. |
Closes #516 |
[added] TabbedArea NavItem renderTab() className TabbedArea TabPane rendering doesn't carry across the className property. This means it is not easy for users to customize the look/feel of the tabs. Added className to the properties copied in the renderTab function and also pass additional properties down via ...other
fa9f6a7
to
1d8b7c7
Compare
@mtscout6 Test added to TabbedAreaSpec and squashed commits. Once Travis CI finishes, I think this should be good. |
LGTM /cc @react-bootstrap/collaborators How does this look? |
Just one quick question, this will apply the className passed to Should it be another prop like |
@dozoisch Good call! |
Currently there is no className being passed back to the renderPane function. @dozoisch If we want to support custom classes on either one, we should agree on a naming scheme and open a different ticket. How do the following properties look: I'll open a new issue? |
@thehuey That looks good to me! |
The issue of the classNames is different, and will require some more discussion. Let's continue that in #560. I also updated the 1.0.0 roadmap documentation to address this issue. |
[added] TabbedArea NavItem renderTab() className [fixes] #516
TabbedArea TabPane rendering doesn't carry across the className
property. This means it is not easy for users to customize
the look/feel of the tabs.
Added className to the properties copied in the renderTab function
and also pass additional properties down via ...other