Looking through the code for anything that catches my eye…
in src/index.js
const mapStateToProps = (state) => ({
appState: state,
});
const mapDispatchToProps = (dispatch) => ({
});
const App = connect(mapStateToProps, mapDispatchToProps)(Pomodoro);
First of all, you don’t usually map all the state. That kind of defeats the point of breaking your state up into different sections. That may not be apparent on a small app, but it’s a good habit to get into. I would expect something like:
const mapStateToProps = (state) => ({
session: state.session,
breakLength: state.breakLength,
});
And if you don’t have anything for mdtp, just leave it out.
Also, I wouldn’t do this at the root level of your app. Just do it where it is needed - that is one of the big values of redux - that you don’t need to keep state in your root component and pass it down everywhere. Just use mstp and mdtp where you need them. If you have a child that is a dumb component and you want to handle that in the parent, that can be cool, just don’t overdo it.
In src/components/break-control.js
const decrementBreak = () => {
store.dispatch(breakDecrement());
}
This is what mdtp is for.
In src/components/timer.js
useEffect(() => {
if (appState.timeLeft === 0) {
currentAudio.play();
}
});
You don’t have any dependencies for your useEffect hooks. Doesn’t that mean that they will run every render? It seems like unnecessary overhead for something that could just be run in the body of the component.
useEffect(() => {
let timer;
if (appState.running) {
timer = setTimeout(() => {
store.dispatch(tick())
}, 1000);
}
return () => clearTimeout(timer);
})
Isn’t this going to create a new timer on every render? Maybe I’m wrong.
In src/reducers/reducer.js
return Object.assign({}, state, {
running: true,
});
Most people would write that as:
return { ...state, running: true };
I see that you added in thunk here. Did you use a thunk? I didn’t see one. I wouldn’t expect there to be thunks here because of the lack of async operations. I mean, even redux is kind of overkill here, but it makes sense as a learning exercise.
All in all, it looks pretty good. Keep in mind that I’m a very picky code reviewer.
I think some of your formatting is unusual, but not atrociously so. If you’re developing locally, I might suggest a linter. They are a pain in the ass at first, but they teach you to be consistent in your formatting which makes code easier to read which makes it easier to write and debug.
But still, good work. Have fun with data visualization.