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 Nav props to ul so css classes can be applied #107

Merged
merged 1 commit into from Jun 15, 2014
Merged

Pass Nav props to ul so css classes can be applied #107

merged 1 commit into from Jun 15, 2014

Conversation

scottwoodall
Copy link
Contributor

@@ -63,7 +63,7 @@ var Nav = React.createClass({
classes['nav-justified'] = this.props.justified;
classes['navbar-nav'] = this.props.navbar;

return (
return this.transferPropsTo(
Copy link
Contributor

Choose a reason for hiding this comment

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

The this.transferPropsTo will need to me moved to L49 so that line would end up looking like:

return this.transferPropsTo(this.renderUl());

In its current position for the non navbar use-case it would end up transferring all props to both the nav and the ul.

@pieterv
Copy link
Contributor

pieterv commented Jun 13, 2014

Hey @scottwoodall thanks for the PR, sorry my comment in #101 wasn't particularly clear on what was needed, but looks like we are on the right path now :)

@scottwoodall
Copy link
Contributor Author

Thank you for the feed back. I've made your suggested changes. Let me know if I misunderstood or you'd like to see something else.

@pieterv
Copy link
Contributor

pieterv commented Jun 14, 2014

This is perfect!

If you squash the commits i will merge it in :)

@pieterv
Copy link
Contributor

pieterv commented Jun 15, 2014

Awesome!

pieterv added a commit that referenced this pull request Jun 15, 2014
Pass Nav props to ul so css classes can be applied
@pieterv pieterv merged commit fb3bd3f into react-bootstrap:master Jun 15, 2014
@scottwoodall scottwoodall deleted the tranferPropsToNavUl branch June 16, 2014 01:19
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.

None yet

2 participants