Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thomas folbrecht #1

Merged
merged 4 commits into from Mar 14, 2019

Conversation

Projects
None yet
2 participants
@tfolbrecht
Copy link
Owner

tfolbrecht commented Mar 5, 2019

No description provided.

@tfolbrecht tfolbrecht requested a review from gsamaniego41 Mar 5, 2019

@gsamaniego41
Copy link
Collaborator

gsamaniego41 left a comment

Completed MVP and stretch. 🔥
Incredible job on this app. Nice delete features. Code was easy to read. Keep it up!
✔️ Redux boilerplate built correctly
✔️ Can add a todo
✔️ Can toggle a todo
✔️ Clears all completed
✔️ Each todo has it's own delete button

Suggestion/s:

  • Folder structure: Even though this is a small app, it's still good to get in the habit of separating actions and reducers into their own respective directories.
);
}
}
const mapStateToProps = state => {

This comment has been minimized.

@gsamaniego41

gsamaniego41 Mar 14, 2019

Collaborator

Another way of mapping state to props using destructuring

const mapStateToProps = ({value, completed}) => ({
  todos: {value, completed}
});
render() {
return (
<form onSubmit={(e) => {
e.preventDefault();

This comment has been minimized.

@gsamaniego41

gsamaniego41 Mar 14, 2019

Collaborator

Curious why you didn't use this.submitHandler here 😄


return(
<ul>
{props.todos.map((task, index) => { return <Todo key={index} todo={task.value} completed={task.completed} id={task.id} />})}

This comment has been minimized.

@gsamaniego41

gsamaniego41 Mar 14, 2019

Collaborator

No need for a return in a one-liner.

Suggested change
{props.todos.map((task, index) => { return <Todo key={index} todo={task.value} completed={task.completed} id={task.id} />})}
{props.todos.map((task, index) => <Todo key={index} todo={task.value} completed={task.completed} id={task.id} />)}
switch (action.type) {
case ADDTODO:
if(action.addedTodo !== ""){
return {todos: [...state.todos, {id: state.todos.length, value: action.addedTodo, completed: false}]}

This comment has been minimized.

@gsamaniego41

gsamaniego41 Mar 14, 2019

Collaborator

id: state.todos.length Clever 👌

@gsamaniego41 gsamaniego41 merged commit 1a7c00a into master Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.