Nico
Nico

Reputation: 749

How to handle two similar components in react

I am very new to React. I have two components: TimePickerComponent and the TimeDurationPickerComponent.

The TimePickerComponent gets passed a TimeString(string) via props(only if initial data exists) and displays it like "08:00". Code:

class TimePickerComponent extends React.Component {
    _placeholder;
    _defaultTimeString;
    _timeString;
    _errorStatus;
    _classes;

    constructor({ placeholder, defaultTimeString, timeString, errorStatus, classes }) {
        super();
        this._placeholder = placeholder;
        this._defaultTimeString = defaultTimeString;
        this._timeString = timeString;
        this._errorStatus = errorStatus;
        this._classes = classes;
    }

    get Placeholder() {
        return this._placeholder;
    }

    get DefaultTimeString() {
        return this._defaultTimeString ? this._defaultTimeString : CONTROLS_CONSTANTS.DEFAULT_TIME_STRING;
    }

    get TimeString() {
        return this._timeString;
    }

    get ErrorStatus() {
        return this._errorStatus;
    }

    get Classes() {
        return this._classes;
    }

    render() {
        return <FormControl>
            <TextField error={this.ErrorStatus}
                label={this.Placeholder}
                defaultValue={this.TimeString ? this.TimeString : this.DefaultTimeString}
                className={this.Classes.layout}
                type="time"
                InputLabelProps={{
                    shrink: true
                }}
            />
        </FormControl>
    }
}

TimePickerComponent.propTypes = {
    placeholder: PropTypes.string.isRequired,
    defaultTimeString: PropTypes.string,
    timeString: PropTypes.string,
    errorStatus: PropTypes.bool
}

export default withStyles(styles)(TimePickerComponent);

The TimeDurationPickerComponent gets passed a TimeInMinutes(number) via props. But the display is the same as of the TimePickerComponent("08:00"). Code:

class TimeDurationPickerComponent extends React.Component {
    _placeholder;
    _timeInMinutes;
    _classes;

    constructor({ placeholder, timeInMinutes, classes }) {
        super();
        this._placeholder = placeholder;
        this._timeInMinutes = timeInMinutes;
        this._classes = classes;
    }

    get Placeholder() {
        return this._placeholder;
    }

    get TimeInMinutes() {
        return this._timeInMinutes;
    }

    get Classes() {
        return this._classes;
    }

    get TimeString() {
        let timeFormat = CONTROLS_CONSTANTS.TIME_FORMATS.HOURS_MINUTES_COLON_SEPARATED;
        let duration = moment.duration({ minutes: this.TimeInMinutes });

        //https://github.com/moment/moment/issues/463
        return moment.utc(duration.asMilliseconds()).format(timeFormat);
    }

    render() {
        return <TimePickerComponent
            placeholder={this.Placeholder}
            timeString={this.TimeString}
            classes={this.Classes}
        />
    }
}

TimeDurationPickerComponent.propTypes = {
    placeholder: PropTypes.string.isRequired,
    timeInMinutes: PropTypes.number
}

export default TimeDurationPickerComponent;

To avoid code redundancy I reused my TimePickerComponent in the TimeDurationPickerComponent and just convert the TimeInMinutes in a TimeString and pass it down to the TimePickerComponent via props.

My question now: Is this a good practice how I solved this or should I use a HigherOrderComponent for that? Or should I use an inheritance approach for that? Which solution would be the best and why?

Thank you in advance.

Upvotes: 0

Views: 1298

Answers (1)

Dakota
Dakota

Reputation: 1274

What you've done here is probably fine. It could be done with a higher order component as well but a composition based approach like what you have won't have any performance issues and to be honest it's probably more readable than using an HOC.

On another note you should be using this.props and this.state to represent your class properties. They are build into React components and are what will cause your component to automatically re-render upon change.

It also makes your code significantly more concise so for example you could reduce your second component down to something like this:

class TimeDurationPickerComponent extends React.Component {

    constructor(props) {
        super(props);
    }

    createTimeString() {
        let timeFormat = CONTROLS_CONSTANTS.TIME_FORMATS.HOURS_MINUTES_COLON_SEPARATED;
        let duration = moment.duration({ minutes: this.props.TimeInMinutes });

        //https://github.com/moment/moment/issues/463
        return moment.utc(duration.asMilliseconds()).format(timeFormat);
    }

    render() {
        return <TimePickerComponent
            placeholder={this.props.Placeholder}
            timeString={this.createTimeString()}
            classes={this.props.Classes}
        />
    }
}

Example of a component that uses flow:

// @flow

import React from 'react';

import './css/ToggleButton.css';

type Props = {
  handleClick: Function;
  label: string;
};

type LocalState = {
  active: bool,
};

class ToggleButton extends React.Component<Props, LocalState> {
  clickHandler: () => void;

  constructor(props: Props) {
    super(props);

    this.state = {
      active: true,
    };

    this.clickHandler = this.clickHandler.bind(this);
  }

  clickHandler() {
    this.setState({ active: !this.state.active });
    this.props.handleClick();
  }

  render() {
    const buttonStyle = this.state.active ? 'toggle-btn-active' : 'toggle-btn-inactive';
    return (
      <button
        className={`toggle-btn ${buttonStyle}`}
        onClick={this.clickHandler}
      >{this.props.label}
      </button>
    );
  }
}

export default ToggleButton;

Upvotes: 4

Related Questions