2
\$\begingroup\$

I have 2 variations of components that I wanted to create in React. They both use Radio buttons with the only difference is one adds an image to one side of it.

The first method will use just one component but use conditional logic to display the appropriate HTML:


    getImgDiv(iconFile) {
        return (<div><img src={iconFile} /></div>);
    }
    render() {
        var radioId = this.props.radioGroup + this.props.value.replace(" ", "");
        var iconFile = this.props.iconFile;
        var innerLabel = "label-inner no-img";
        var imgDiv = "";
        if (iconFile !== undefined && iconFile !== null && iconFile !== "") {
            iconFile = "/img/" + iconFile;
            innerLabel = "label-inner"; // add appropriate class when there is an image
            imgDiv = this.getImgDiv(iconFile);
        }

        return (
            <div className="option">
                <Field
                    name={this.props.radioGroup}
                    component="input"
                    type="radio"
                    value={this.props.value}
                    id={radioId}
                />
                <label htmlFor={radioId} className="radio-buttons">
                    <div className={innerLabel}>
                        {imgDiv}
                        <div><span>{this.props.text}</span></div>
                    </div>
                </label>
            </div>
        );
    }

The second method, I was thinking of just creating 2 slightly different React classes to handle each case... 1 called RadioBtn the other RadioBtnIcon with the img section added:

function RadioBtn({ radioGroup, value, text }) {
    var radioId = radioGroup + value.replace(" ", "");
    return (
        <div className="option">
            <Field
                name={radioGroup}
                component="input"
                type="radio"
                value={value}
                id={radioId}
            />
            <label htmlFor={radioId} className="radio-buttons">
                <div className="label-inner no-img">
                    <div><span>{text}</span></div>
                </div>
            </label>
        </div>
    );
}

However, its bugging me that neither of these approaches seems particularly elegant...

Any suggestions guys?

Many Thanks,

Phil.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

You can take advantage of conditional rendering to concisely render the iconFile if it exists, and use the conditional operator to construct the outer class name. Using two separate classes for nearly identical functionality is pretty WET, I wouldn't recommend it.

Because the iconFile variable actually contains a string (if it exists), not a File, consider naming it something else, maybe iconPath:

<label htmlFor={radioId} className="radio-buttons">
    <div className={"label-inner" + (iconPath ? "" : " no-img")}>
        {iconPath && <div><img src={"/img/" + iconPath} /></div>}
        <div><span>{text}</span></div>
    </div>
</label>

You currently reference properties of this.props a lot. Consider destructuring them all immediately instead.

Since you're using ES6+ syntax, always use const, never use var.

It sounds like Field is using the component prop as a tag name. Because variable tag names in JSX need to be upper-case, and because Component could easily be mistaken for React.Component, consider using a different name, maybe Tag.

const Field = ({ name, Tag, type, value, id }) => (
    <Tag name={name} type={type} value={value} id={id} />
);
class RadioImage extends React.Component {
    render() {
        const { radioGroup, value, iconPath, text } = this.props;
        const radioId = radioGroup + value.replace(" ", "");
        return (
            <div className="option">
                <Field
                    name={radioGroup}
                    Tag="input"
                    type="radio"
                    value={value}
                    id={radioId}
                />
                <label htmlFor={radioId} className="radio-buttons">
                    <div className={"label-inner" + (iconPath ? "" : " no-img")}>
                        {iconPath && <div><img src={"/img/" + iconPath} /></div>}
                        <div><span>{text}</span></div>
                    </div>
                </label>
            </div>
        );
    }
}

ReactDOM.render(
    // The Fragments below can be replaced with <>  </> with an up-to-date Babel
    // (Stack Snippets do not support it)
    <React.Fragment>
        <RadioImage radioGroup="radioGroup" value="value1" text="text1" />
        <RadioImage radioGroup="radioGroup" value="value2" iconPath="iconPath" text="text2 with icon" />
    </React.Fragment>
    ,
    document.getElementById('root')
);
<div id="root"></div>
<script src="https://unpkg.com/react@16/umd/react.production.min.js"></script>
<script src="https://unpkg.com/react-dom@16/umd/react-dom.production.min.js"></script>

You might consider avoiding IDs when possible - they implicitly create global variables, which aren't always the best idea when avoidable. You can move the <input> inside the <label> so that no htmlFor={radioId} is needed.

You can make RadioImage a stateless functional component if you want, it doesn't look to have any need to be stateful:

const Field = ({ name, Tag, type, value, id }) => (
    <Tag name={name} type={type} value={value} id={id} />
);
const RadioImage = ({ radioGroup, value, iconPath, text }) => (
    <div className="option">
        <label className="radio-buttons">
            <Field
                name={radioGroup}
                Tag="input"
                type="radio"
                value={value}
            />
            <div className={"label-inner" + (iconPath ? "" : " no-img")}>
                {iconPath && <div><img src={"/img/" + iconPath} /></div>}
                <div><span>{text}</span></div>
            </div>
        </label>
    </div>
);
ReactDOM.render(
    // The Fragments below can be replaced with <>  </> with an up-to-date Babel
    // (Stack Snippets do not support it)
    <React.Fragment>
        <RadioImage radioGroup="radioGroup" value="value1" text="text1" />
        <RadioImage radioGroup="radioGroup" value="value2" iconPath="iconPath" text="text2 with icon" />
    </React.Fragment>
    ,
    document.getElementById('root')
);
.option {
  border: 1px solid black;
}
<div id="root"></div>
<script src="https://unpkg.com/react@16/umd/react.production.min.js"></script>
<script src="https://unpkg.com/react-dom@16/umd/react-dom.production.min.js"></script>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.