Chris

[ChrisMunene]

No description provided.

[jbperry004]

Instead of passing props as an argument here and breaking it down, use destructuring (kind of like pattern matching from CS51) to pass in {formSubmitted, newTodoChanged, newTodo}, etc. so that you don't always have to access them as properties of props.

[jbperry004]

If you have a function that has only a return value and nothing before that (like this), you can shorten the definition to something like:

const NewTodoForm = ({formSubmitted, ... }) => <form>...</form>

[jbperry004]

Avoid using the form component - instead, just implement the state functionality yourself.

1. Rewrite this component from an SFC (stateless functional component) to a class-based component (like the ToDoContainer)

2. Add a constructor with a default state of {todo: ''}

3. Write a function which updates this LOCAL state to the new value of the text input when the input is changed, and pass it to the input tag below as the onChange prop.

4. Write a function which adds the new todo to the higher state when the user clicks the submit button, and pass this in to the button tag as an onClick prop.

[jbperry004]

Good use of the key prop here!

[jbperry004]

Use a ternary operator for more precise conditional rendering:

{props.todos.length > 0 ? <div><ul> ... </ul></div> : <p>No todos ... </p>}

[jbperry004]

Where do you ever use this done property?

[jbperry004]

good use of the spread operator!

[jbperry004]

Consider an alternate method of removing todos from the list than index / position in the list and splicing on that - splice is a bit messy here since it causes side-effects on the todos variable. Instead, you could store the index of each each todo in this.state.todos as an id property in your todo object. Then, when you call removeTodo with a given index, instead of splicing, you can use the cleaner .filter() method to exclude the todo with that index as its id and then set the state to that updated array.

[ChrisMunene]

Tried this approach but it causes this bug where once I update state with the filtered array the indexes are modified due to the removal of one element such that when I try to delete the second element it doesn't work. Fixed it by changing the ID everytime I do the filtering but I don't think it's the most elegant solution.

[jbperry004]

todo.title.includes(event.target.value) is a boolean value itself, so no need to check equality with true

[jbperry004]

By the way you currently implement this searching functionality, there is no way of recovering the results that don't match the query once you exit from the query. This is because you set the state as part of your searching. Instead consider this option:

You can store a filter property in the state of the ToDoContainer, which is updated by the input in the ToDoList whenever it is changed. Then, instead of changing the todos in the state, you can just conditionally render only the todos in your list which have a match with this.state.filter