0
\$\begingroup\$

I created a simple Pomodoro timer using React JS.
In my example:

  • Timer should count down seconds until it reaches 00:00
  • Timers should switch between 25 and 5 minutes
  • Timer should be able to start, stop, and reset

My questions:

  • how can I improve the switching between the "work" and "relax" modes? Now I use 2 functions for this: toogleWork() and toogleRelax().
  • my functions for stop, start and reset look very simple and I always think that there is something missing here...

Thank you for your time and any suggestions you can provide.

class Timer extends React.Component {
  constructor(props) {
    super(props)
    this.state = {
      time: props.time,
    }
  }

  format(seconds) {
    let m = Math.floor(seconds % 3600 / 60);
    let s = Math.floor(seconds % 3600 % 60);
    let timeFormated = (m < 10 & '0' : '') + m + ':' + (s < 10 ? '0' : '') + s;
    return timeFormated
  }

  startTimer = () => this.interval = setInterval(this.countDown, 1000)

  countDown = () => {
    if (this.state.time !== 0) { 
      this.setState(prevState => ({time: prevState.time - 1}))
    } else {
      this.stopTimer()
    }
  }

  stopTimer = () => clearInterval(this.interval)

  resetTime = () => {
    this.stopTimer()
    this.setState({time: this.props.time})
  }
  
  componentWillUnmount() {
    clearInterval(this.interval)
  }

  render() {
    return (
      <div >
        <div className='time'>{this.format(this.state.time)}</div>
        <button onClick={this.startTimer}>Start</button>
        <button onClick={this.stopTimer}>Stop</button>
        <button onClick={this.resetTime}>Reset</button>
      </div>
    );
  }
}

class App extends React.Component{
  constructor(props){
    super(props)
    this.state = {
      work: true,
    }
  }

  toogleWork = () => this.setState({work: true})

  toogleRelax = () => this.setState({work: false})

  render() {
    return (
      <div className='App'>
        <button onClick={this.toogleWork}>Work</button>
        <button onClick={this.toogleRelax}>Relax</button>
        {this.state.work && <Timer time={1500} />}
        {!this.state.work && <Timer time={300} />}
      </div>
    )
  }
}

ReactDOM.render(
  <App />,
  document.getElementById('root')
)
.App {
  text-align: center;
}

button {
  margin: 15px 5px;
  padding: 10px;
  border: none;
}

.time {
  font-size: 32px;
}
<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>
<div id="root"></div>

\$\endgroup\$
1
\$\begingroup\$

You can replace

<button onClick={this.toogleWork}>Work</button>
<button onClick={this.toogleRelax}>Relax</button>

with

<button onClick={this.toggleState}>{this.state.work? 'Relax' : 'Work'}</button>;

and

toogleWork = () => this.setState({work: true})
toogleRelax = () => this.setState({work: false})

with

toggleState = () => this.setState(prevState => ({work: !prevState.work}));
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review and thanks for answering! To be clear, you are suggesting that the OP only show one button instead of the two at the top, so as to answer the first question (i.e. "how can I improve the switching between the "work" and "relax" modes? Now I use 2 functions for this: toogleWork() and toogleRelax().", right? It might be wise to edit your answer and explicitly state this or put the OPs question(s) in a quote above the section of your answer. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Sep 12 '18 at 15:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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