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

fix(DropdownToggle): add missing proptype (color) #402

Merged

Conversation

apaatsio
Copy link
Contributor

No description provided.

@eddywashere eddywashere merged commit c137697 into reactstrap:master May 14, 2017
@virgofx
Copy link
Collaborator

virgofx commented Jun 10, 2017

This PR is invalid. It breaks HTML as it leaks the color attribute and is not BS4 compatible. Color needs to be removed.

@apaatsio
Copy link
Contributor Author

@virgofx Can you elaborate what exactly is breaking? It seems to be working fine for me.

E.g.

<DropdownToggle color="success" caret>Hello</DropdownToggle>

generates

<button type="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false" class="dropdown-toggle btn btn-success">Hello</button>

Moreover, if we remove the color property, how are we supposed to change the color of the DropdownToggle?

@virgofx
Copy link
Collaborator

virgofx commented Jun 15, 2017

Ah, I see this distinction. You were using color for the Button component (which is valid). In my case I was using non-buttons where color is invalid and causes conflicts.

So, I believe the proper solution is to allow color as an optional prop that gets passed into the DropdownToggle dynamic tag child element if and only if the tag is the Button. I can update my PR and add tests if this seems to be correct?

@apaatsio Let me know your thoughts

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

4 participants