React Pomodoro Timer Feedback/Help with async setState

Hello, everyone. I’ve just accepted that because of the way I designed this that some of the tests won’t pass.

However, I’d be grateful for anyone who would take a look at my React code to see if it could be improved in anyway.

A big sticking point for me is that I wanted to add in something to count the sessions so that way the break after the fourth session would be a long break instead of a short one.

It seems like React notices this too late for reasons maybe linked to setState being asynchonous. (It’s just a guess.) I’ve handled this by artificially adding one for computing purposes, but I’d like to know if there’s a more… idiomatic (?) or efficient way to do this. You can see what I mean on line 185.

Just as a note, the short break and session are set as one second and the long break as one minute just to make testing easier.

Thanks in advance. :grinning:

https://codepen.io/donthatedontkill/pen/vzvKrr?editors=0010

function setStateCounter(num){
    return(previousState) =>{return {...previousState, counter: previousState.counter + num}}
  }

/* Timer display filter */
class Display extends React.Component{
  constructor(props){
    super(props);
    this.addZero = this.addZero.bind(this)
  }
  
  addZero(num){
    if (num < 10){
      return ("0" + num)
    }
    else{
      return num
    }
  }
  
  render(){
    return(
    <div>{this.addZero(this.props.minutes)}:{this.addZero(this.props.seconds)}</div>
  );
  }
}

/*Timer architecture */
class Timer extends React.Component{
  constructor(props){
    super(props);
    this.state = {
      minutes: this.props.minutes,
      seconds: 1,
      timerOn: false,
      html: {
        length: this.props.type +"-length",
        label: this.props.type +"-label",
        decrement: this.props.type +"-decrement",
        increment: this.props.type +"-increment"
      },
    }
    
    this.TimerFunction = this.TimerFunction.bind(this);
    this.activateTimer = this.activateTimer.bind(this);
    this.reset = this.reset.bind(this);
    this.increaseMinutes = this.increaseMinutes.bind(this);
    this.decreaseMinutes = this.decreaseMinutes.bind(this);
  }
  
/*Buttons for changing the time limit*/
  increaseMinutes(){
        if (this.state.timerOn == false && this.state.minutes < 60){
    this.setState({
      minutes: this.state.minutes + 1,
    })}
        }
  
      decreaseMinutes(){
        if (this.state.timerOn == false && this.state.minutes > 0){
    this.setState({
      minutes: this.state.minutes - 1,
    })}}

  /*Timer mechanism*/
  TimerFunction(){
     if (this.state.minutes > 0 || this.state.seconds > 0){
      if (this.state.seconds <= 0){
        this.setState({
          minutes: this.state.minutes - 1,
          seconds: 59,
        })
      }
       else{
         this.setState({
        seconds: this.state.seconds - 1
      })
       }
      }
    else{
      clearInterval(this.timerInterval);
      this.setState({
        timerOn: false,
        minutes: this.props.minutes,
        seconds: 1,
      })
      this.props.timerSwitch(this.props.type);
    this.props.counterUp();
    }
      }
  
  /*Start/pause button */
    activateTimer(){
    if (this.state.timerOn == false){
    this.setState({
      timerOn: true
    })
     this.timerInterval = setInterval(this.TimerFunction, 1000)
      }
    if (this.state.timerOn == true){
      this.setState({
        timerOn: false,
      })
      clearInterval(this.timerInterval)
    }
  }
  
  /*Reset button*/
  reset(){
    clearInterval(this.timerInterval)
    this.setState({
      minutes: this.props.minutes,
      seconds: 0,
      timerOn: false,
    })
    this.props.reset()
  }
  
  render(){
    return(
        <div id='timer'>
        <div id={this.state.html.label}>
          {this.props.type} Length: </div>
        <div id={this.state.html.length}>
          {this.state.minutes}</div>

              <button onClick={this.increaseMinutes} id={this.state.html.increment}>Increase session minutes</button>
        <button onClick={this.decreaseMinutes} id={this.state.html.decrement}>Decrease session minutes</button>
      {(this.props.active===this.props.type) && <div id="time-left"><Display minutes={this.state.minutes} seconds={this.state.seconds}/><button id="start_stop" onClick={this.activateTimer}>Click me</button> <button onClick={this.reset}>Reset</button></div>}
      </div>
    )
  }
         }



class App extends React.Component{
  constructor(props){
    super(props);
    this.state = {
      currentTimer: "session",
      defaultSession: 0,
      shortBreak: 0,
      longBreak: 1,
      counter: 0,
    }
    this.changeTimer = this.changeTimer.bind(this);
    this.reset = this.reset.bind(this);
this.counterUp = this.counterUp.bind(this);
  }
  
  changeTimer(finishedTimer){
    if (finishedTimer == "session"){
        this.setState({
          currentTimer: "break",
        }
       )
  }
    if (finishedTimer == "break"){
      this.setState({
        currentTimer: "session",
      })
    }
  }
  
  reset(){
    this.setState({
      currentTimer: "session",
    })
    
  }
  
  /*Attempt to fix async issue with timer*/
  counterUp(){
    this.setState(setStateCounter(1))
  }
  
  render(){
    return(
      <div>
        {(this.state.counter % 4)}
        <div id="timer-label">{this.state.currentTimer} initiated</div>
        <Timer type="session" minutes={this.state.defaultSession} active={this.state.currentTimer}  timerSwitch={this.changeTimer} reset={this.reset} counterUp={this.counterUp}/> 
        {this.state.counter}
        <Timer type="break" minutes={((this.state.counter+1) < 4 || (this.state.counter+1) % 4 != 0) ? this.state.shortBreak : this.state.longBreak} active={this.state.currentTimer} timerSwitch={this.changeTimer} reset={this.reset}/>
        </div>
  )}
};

ReactDOM.render(
        <App />,
  document.getElementById('root')
);

Thanks for the feedback! I didn’t know this, so it’s great you pointed it out. Unfortunately, it doesn’t seem to fix the problem, but I updated my other function to be like this.

Thanks again!

I think it does it because in line 184 I have this:
<Timer type="break" minutes={((this.state.counter+1) < 4 || (this.state.counter+1) % 4 != 0) ? this.state.shortBreak : this.state.longBreak} active={this.state.currentTimer} timerSwitch={this.changeTimer} reset={this.reset}/>

Specifically the this.state.counter+1 is where I’ve been working around it. When I delete the +1, it doesn’t work correctly and looks like this:

when it should look like this:

I want the break length to switch to a long break (one minute instead of one seconds) every multiple of four without the counter resetting.

Sorry about the editing. Now that you say it, I definitely should have made a fork. As it stands right now, it’s only updated to use the function version of setState in the instances where that was applicable, otherwise it’s unchanged. I checked each function as I changed it, so I’m sure they’re fine.

Here’s the two pens:

Original: https://codepen.io/donthatedontkill/pen/vzvKrr

Updated: https://codepen.io/donthatedontkill/pen/aRBOap?editors=0010

I think what you’re missing is what I tried to explain about line 184. In that code, I jimmied it to add one to the real number of sessions in this part:

minutes={((this.state.counter+1) &lt; 4 || (this.state.counter+1) % 4 != 0) ? this.state.shortBreak : this.state.longBreak

If the +1 is deleted from the two conditions (and therefore the true number of sessions is evaluated instead of the number of sessions PLUS one), it doesn’t work right and switches to a long break at 5 sessions instead of 4. It updates too late. Here’s another pen with the modified code. I forked it from the one with your suggested modifications (which don’t, for me at least, change how the code functions in any significant way but is just better practice).

Updated pen with +1 deleted: https://codepen.io/donthatedontkill/pen/dgOzGB?editors=0010

I’ll try to make a video after work today. Thank you very much for your patience and your feedback up to now. I really do appreciate it.

Here’s a video I uploaded to YouTube. I hope it helps!

For anyone who comes looking, the eventual solution was to use setState to recheck props before continuing. This fixed the problem with props updating “late.”