React Pomodoro Timer Feedback/Help with async setState

React Pomodoro Timer Feedback/Help with async setState
0

#1

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')
);

#2

@AdamWier Not sure if this fixes your issue, but in general it is recommended to set state with a function. See below where I rewritten your this.setState inside your counterUp method. This helps with setState’s asynchronous nature to guarantee you have the most recent value of state when you need to use a state property value to update the next state.

  counterUp() {
    this.setState(({ counter }) => ({
      counter: counter + 1
    }));
  }

I have used destructuring above, but the first argument of the function actually is the most current state, so without destructuring you would just use something like:

  counterUp() {
    this.setState((prevState) => ({
      counter: prevState.counter + 1
    }));
  }

#3

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!


#4

That is strange. I thought when I tested it with this code, it updated the same as “hack” version. I guess I was mistaken on exactly what I should have been seeing.

I just tested both versions and it seems to me like they are doing the same thing. What is the version I suggested not doing that your “hack” version does which you need it to do?


#5

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:


#6

when it should look like this:


#7

So you only want to have the break session count increasing up till 4 and the reset to 0?

Maybe create a video and post your exact steps using the code that “works”, so we can see how you are expecting the page to update the way you want. I am having trouble recreating what you are trying to explain in words.

Also, make another pen which contains only your original code, so I can test against it. I think you have modified your original to incorporate the code I suggested?


#8

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.


#9

Here’s the two pens:

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

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


#10

Again, I must be missing something, because when I test both pens above, after performing the same clicks on each button, they both end up looking like below after the same click sequence.

image

If you are seeing a difference between what gets displayed after the same sequence of clicks, then I will need to see a video of exactly what you are doing to see the difference.


#11

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.


#12

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