\$\begingroup\$
\$\endgroup\$
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>
200_success
146k22 gold badges190 silver badges478 bronze badges
asked Aug 3, 2018 at 14:43
1 Answer 1
\$\begingroup\$
\$\endgroup\$
1
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}));
-
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\$2018年09月12日 15:13:44 +00:00Commented Sep 12, 2018 at 15:13
default