Please review my Pomodoro Clock! It’s my first app built in React.
You can find my source code here.
I had some issues with SVG filters making my app slow in Firefox mobile that I couldn’t resolve.
Please review my Pomodoro Clock! It’s my first app built in React.
You can find my source code here.
I had some issues with SVG filters making my app slow in Firefox mobile that I couldn’t resolve.
Hi @ellereeeee ,
handleIncrementTime = () => {
this.setState({ state: (this.state.time += 300000) });
};
handleDecrementTime = () => {
if (this.state.time > 300000) {
this.setState({ time: (this.state.time -= 300000) });
}
};
Reacts Documentation:
https://reactjs.org/docs/state-and-lifecycle.html
this.setState((prevState, props) => ({
counter: prevState.counter + props.increment
}));
Cheers and happy coding
Wow thank you Diego !
Hey @Diego_Perez,
I looked at the documentation for the second form of setState
and it says you’re supposed to use it when you’re updating the state with props, right? It looks like it solves an asynchronicity issue between the two.
In my case I’m not updating state with props; I’m just adding or subtracting 5 minutes to or from the state. Is there a way I could use the second form of setState
?
Hi @ellereeeee,
I cloned the repo and I am getting warnings:
Compiled with warnings.
./src/components/Info.jsx
Line 16: Using target="_blank" without rel="noopener noreferrer" is a security risk: see https://mathiasbynens.github.io/rel-noopener react/jsx-no-target-blank
./src/components/PomodoroTimer.jsx
Line 17: Do not mutate state directly. Use setState() react/no-direct-mutation-state
Line 21: Do not mutate state directly. Use setState() react/no-direct-mutation-state
Line 33: Expected '===' and instead saw '==' eqeqeq
Line 52: Expected '===' and instead saw '==' eqeqeq
Line 78: Expected '===' and instead saw '==' eqeqeq
Line 109: Expected '===' and instead saw '==' eqeqeq
Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.
The important part is:
Line 17: Do not mutate state directly. Use setState() react/no-direct-mutation-state
I added a constructor to PomodoroTimer.jsx
Now state
is this.state
and is inside the constructor
class PomodoroTimer extends Component {
constructor(props) {
super(props);
this.state = {
toggleInfo: false,
timerActive: false,
time: 1500000,
timerType: "Pomodoro",
offsetModifier: 1
};
}
handleIncrementTime = () => this.setState((prevState) => ({ time: prevState.time += 300000 }));
Compiled with warnings.
./src/components/Info.jsx
Line 16: Using target="_blank" without rel="noopener noreferrer" is a security risk: see https://mathiasbynens.github.io/rel-noopener react/jsx-no-target-blank
./src/components/PomodoroTimer.jsx
Line 25: Do not mutate state directly. Use setState() react/no-direct-mutation-state
Line 37: Expected '===' and instead saw '==' eqeqeq
Line 56: Expected '===' and instead saw '==' eqeqeq
Line 82: Expected '===' and instead saw '==' eqeqeq
Line 113: Expected '===' and instead saw '==' eqeqeq
Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.
Refactor handleDecrementTime (using functional setState [second form])
Refactor handleIncrementTime, handleDecrementTime, handleStateTime:
Following the pattern: “event handler method on the class”
Reactjs documentation:
When you define a component using an ES6 class, a common pattern is for an event handler to be a method on the class. For example, this Toggle component renders a button that lets the user toggle between “ON” and “OFF” states:
class Toggle extends React.Component {
constructor(props) {
super(props);
this.state = {isToggleOn: true};
// This binding is necessary to make `this` work in the callback
this.handleClick = this.handleClick.bind(this);
}
handleClick() {
this.setState(prevState => ({
isToggleOn: !prevState.isToggleOn
}));
}
Cheers and happy coding
note:
I suspect that the problem with Firefox mobile can be because the state
is not correctly updated.
Thanks again Diego! I get it now; it’s all about not modifying state directly.
I solved the issue using the second form of setState like so:
handleIncrementTime = () =>
this.setState(prevState => ({ time: (prevState.time += 300000) }));
handleDecrementTime = () => {
if (this.state.time > 300000) {
this.setState(prevState => ({ time: (prevState.time -= 300000) }));
}
};
I have a couple questions regarding your other suggestions though. Isn’t making a constructor and putting state inside a preference in this situation? I’m using public class fields to declare state outside the constructor and inside the class body and everything seems to work fine.
Also, aren’t I already using the “event handler method on the class” for my handlers? I understand that as meaning just declaring the handlers in the class body.
Unfortunately the laggy issue in Firefox mobile remains, but I’m pretty sure it has to do with poor SVG performance in that browser. Removing the SVG filter property from the radial timer makes my app as snappy as in other browsers.
Hi @ellereeeee,
I didn’t know about the public class fields
, thanks for explain it .
I see, nothing to do then.
Cheers and happy coding