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

Allow customising RadioButton icons #1639

Closed
wants to merge 1 commit into from

Conversation

danni
Copy link

@danni danni commented Sep 14, 2015

This is used the same way as for Checkbox.

@danni danni closed this Sep 14, 2015
@danni danni deleted the custom-radio-icons branch September 14, 2015 06:26
@danni danni restored the custom-radio-icons branch September 14, 2015 06:26
@danni danni reopened this Sep 14, 2015
JonatanGarciaClavo pushed a commit to JonatanGarciaClavo/material-ui that referenced this pull request Sep 25, 2015
…hen you have input background hint can not see it.
JonatanGarciaClavo pushed a commit to JonatanGarciaClavo/material-ui that referenced this pull request Sep 25, 2015
…hen you have input background hint can not see it.
JonatanGarciaClavo pushed a commit to JonatanGarciaClavo/material-ui that referenced this pull request Sep 25, 2015
…hen you have input background hint can not see it.
@@ -87,10 +91,22 @@ let RadioButton = React.createClass({
this.props.iconStyle,
this.props.disabled && styles.fillWhenDisabled);

let checkedElement = checkedIcon ? React.cloneElement(checkedIcon, {
style: this.mergeAndPrefix(onStyles, checkedIcon.props.style),
}) : React.createElement(RadioButtonOn, {
Copy link
Member

Choose a reason for hiding this comment

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

Could you keep the JSX syntax?

@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Oct 26, 2015
@oliviertassinari
Copy link
Member

You will have to rebase.

@@ -15,9 +15,11 @@ let RadioButton = React.createClass({
},

propTypes: {
checkedIcon: React.PropTypes.element,
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should use node that is less restrictive.
Could you add this to the documentation?

@mbrookes
Copy link
Member

Hi @danni, I'd like to merge this PR. Do you think you might be able to do the last bits of tidying up per @oliviertassinari's comments? I can take care of the docs separately.

@alitaheri
Copy link
Member

Being continued here: #3285

@alitaheri alitaheri closed this Feb 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants