1

I have a time picker that I want to set the value to this.state.start. However the value of this.state.start could be equal to this.props.normal or this.props.spec depending on whether the user has set special hours, if they have not then it falls back to using normal hours.

I'm running into an issue trying to do if-else statements to update the state of this.state.start. While it should update the value correctly (if-else statements should be correct), react doesn't allow you to update the state in the render function like I've written. How can I set this.state.start conditionally? Code below:

class NormalHours extends React.Component {
constructor(props) {
    super(props)
    this.state = {
        start: null,
    }
}
render() {
    //Browser is very angry at this part
    if(this.props.specStart == this.props.normStart || this.props.specStart == null)
    {
        //If special hours are null or equal to normal start use normal hours
        this.setState({
           start: this.props.normStart;
        });
    }
    else
    {
        //Else the hours are different so use special hours
        this.setState({
           start: this.props.specStart;
        });
    }
    return(
    <div>
        //Using Material UI; this is rendered as a textbox
        <TimePicker
          name="StartTime"
          onChange={(e, date) => {
            this.props.onSetStartTime(Utils.convertDateToTimeString(date))
          }}
          value={this.state.start}
          />
Jobokai
  • 313
  • 1
  • 5
  • 19
  • 2
    Just an FYI: You shouldn't setState in render, a change to state triggers `render()` and thus you're going to get into an unfortunate loop. Unless you handle the update with `shouldComponentUpdate`. (Which you don't currently have) – Dan Apr 05 '17 at 15:19
  • Why are normStart and specStart set on props and not on state? If they are in props you may be able to use shouldReceiveProps lifecycle method – Pineda Apr 05 '17 at 15:21
  • @Dan Yea that's the issue that I'm running into and what I'm trying to figure out how to resolve. I've tried multiple ways and still have issues with the loop. – Jobokai Apr 05 '17 at 15:22
  • @pineda I do have them set on state in my local code, wanted the focus on the This.State.Start. They will not be changing state. – Jobokai Apr 05 '17 at 15:23
  • I mean componentWillReceiveProps :) – Pineda Apr 05 '17 at 15:24

1 Answers1

1

You could have a function that sets this.start.state like so:

class NormalHours extends React.Component {
  constructor(props) {
      super(props)
      this.state = {
          start: null,
      }
      this.setStart();
  }
  setStart = () => {
    if(this.props.specStart == this.props.normStart || this.props.specStart == null)
    {
        //If special hours are null or equal to normal start use normal hours
        this.setState({
           start: this.props.normStart;
        });
    }
    else
    {
        //Else the hours are different so use special hours
        this.setState({
           start: this.props.specStart;
        });
    }
  }
  render() {
    return(
      <div>
          //Using Material UI; this is rendered as a textbox
          <TimePicker
            name="StartTime"
            onChange={(e, date) => {
              this.props.onSetStartTime(Utils.convertDateToTimeString(date))
            }}
            value={this.state.start}
            />
        </div>
      )
    }
  }

I'm not too clued up on whether calling methods in the constructor is considered bad practice or whether or not

this.state = {
  start: null
}

is even required when you're modifying state immediately after.

Dan
  • 8,041
  • 8
  • 41
  • 72
  • I was thinking about trying to run a method in the constructor but didn't know how to best approach it, I'll be trying what you have real quick. – Jobokai Apr 05 '17 at 15:27
  • Instead of instantiating start to `null` in the constructor, you could set it to `this.props.specStart` and remove your `else{}` block in `setStart()`. – Dan Apr 05 '17 at 15:28
  • That's a very good point. I think I'm running into a JS error where it doesn't like `start = () => ` pretty sure its the environment that I'm working in (and no control over that right now); however I think you've given me the right direction to go. – Jobokai Apr 05 '17 at 15:32
  • I'm now getting this message: Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the NormalHours component. – Jobokai Apr 05 '17 at 15:41
  • 1
    Proposed edit to your solution to move the method from the constructor to `componentDidMount()` so the method runs once the component is mounted and not when it is initiated. Also reformatted code a bit to move away from ES6 `= () =>` as that doesn't work in my development environment. – Jobokai Apr 05 '17 at 15:49