Bugfix: Pomodoro is not reset after refreshing or switching the view.#87
Bugfix: Pomodoro is not reset after refreshing or switching the view.#87
Conversation
| const [isClockPlaying, setIsClockPlaying] = useState(false); | ||
| const [workDurationMinutes, setWorkDurationMinutes] = useState(25); | ||
| const [breakDurationMinutes, setBreakDurationMinutes] = useState(5); | ||
| const [m, setM] = useState(25); |
There was a problem hiding this comment.
[minute, setMinute] = useState(25):
[second, setSecond] = useState(0);
in this way, it could be more obvious for anyone who read m anywhere to know it stands for minutes.
There was a problem hiding this comment.
good point. corrected.
| localStorage.setItem("S", s) | ||
| }) | ||
|
|
||
| function setData(func, key) { |
There was a problem hiding this comment.
setData sounds not so direct for me. and it could be more in consistent with localStorage.setItem by
| function setData(func, key) { | |
| function localStorage_loadItem(itemName, itemSetter, itemType) {} |
There was a problem hiding this comment.
I used the name loadItemFromLocalStorage.
| useEffect ( () => { | ||
| setData((data) => setStartTime(data), "startTime") | ||
| setData((data) => setIsUserBreaking(data == "true"), "isUserBreaking") | ||
| setData((data) => setClockKey(Number(data)), "ClockKey") |
There was a problem hiding this comment.
I suggest to handle the data type (Number(), == "true" ) inside setData function via kwargs(function parameters).
so that the code could be easier to understand here
| setData((data) => setClockKey(Number(data)), "ClockKey") | |
| setData(setIsUserBreaking, "isUserBreaking", "bool") | |
| setData(setClockKey, "ClockKey", "number") |
How do you think?
There was a problem hiding this comment.
good point, corrected.
Apollo1840
left a comment
Apollo1840
left a comment
There was a problem hiding this comment.
It looks clear and effective, No more feedbacks from me. @changdiqing . Another option is to use redux store to do this, but I think we could leverage the pro and con a bit, and then make another PR, if the redux store is a better option.
| loadItemFromLocalStorage("breakDurationMinutes", setBreakDurationMinutes, "number") | ||
| loadItemFromLocalStorage("Minute", setMinute, "number") | ||
| loadItemFromLocalStorage("Second", setSecond, "number") | ||
| loadItemFromLocalStorage("Second", setSecond, "number") |
There was a problem hiding this comment.
Thanks. Fixed.
One con of local storage is that it can only store string. The item has to be converted to its original type while loading. Another option to avoid the conversion I did is to stringify and parse the item during storing and loading.
2 participants
Hi, @changdiqing @Apollo1840,
I fixed the problem with useEffect and localStorage.
useEffect updates the m (minutes) and s (seconds) and saves them to localStorage.
After refreshing or switching the view, m and s are read from the localStorage and set to be the initialRemainingTime of the timer.
Please check the PR. Thanks.