Knar - Homework week 5 #231
Conversation
| @inject("TodoStore") | ||
| @observer | ||
| class App extends React.Component { | ||
| componentWillMount() { |
There was a problem hiding this comment.
as @neveenatik noticed me componentWillMount has been deprecated in 16.3 React for that you can check this.
instead of componentWillMount we may use the constructor or you may find this useful
| ) | ||
| ) : ( | ||
| <List>{todoItems}</List> | ||
| ); |
There was a problem hiding this comment.
I see too much Js with a Jsx inside of the render method...
here we have two things to mention:
- these have to be
consts variables. - moving these to a separated component is way less code and easier to follow and debug.
And also I won't forgot to mention about line 35.
you may keep the letin case of a reassign variable to a new Value inside the new value or you could do this:
const NoTodos = () => <p id="no-todos">No todos yet... </p>
const Loading = () => <p id="no-todos">Loading... </p>
const ContentStatus = () => TodoStore.state === "Loading" ? <Loading /> : <NoTodos />
const Content = () => TodoStore.todoCount === 0 ? <ContentStatus /> : <List>{todoItems}</List>
// and inside the return you will have the <Content /> JSX component. 🙂👌| class AddTodoForm extends React.Component { | ||
| onAddTodo = e => { | ||
| e.preventDefault(); | ||
| let newTodo = this.enteredTodo.value; |
There was a problem hiding this comment.
you are not reassigning the newTodo variable in any other place.
using const is a better thing to have from the code styling and structuring in my case and from the memory usage in other case a will.
| onAddTodo = e => { | ||
| e.preventDefault(); | ||
| let newTodo = this.enteredTodo.value; | ||
| if (newTodo === "") { |
There was a problem hiding this comment.
we could use with the same behavior:
if (!newTodo) return;| import React from "react"; | ||
| class Checkbox extends React.Component { | ||
| render() { | ||
| let done = this.props.done; |
| toggleDone = async id => { | ||
| let todoItem = this.todos.find(todo => todo._id === id); | ||
| try { | ||
| await this.handleFetch(`${url}/todos/${id}`, "PATCH", { |
There was a problem hiding this comment.
if you will assign this line to a variable we may have a look to the next comment...
| } | ||
| return todo; | ||
| }); | ||
| this.todos = todoItems; |
There was a problem hiding this comment.
by assigning line 33 to a variable we can use it here instead of all from line 37 till line 46.
let's say the variable name is updated you can choose a better name.
runInAction(() => {
this.todos = this.todos.map(todo => {
if (updated._id !== todo._id) return todo;
return updated
}) // thats is 🎊
});| @action | ||
| addTodo = async newTodo => { | ||
| let todoItem = { | ||
| id: uuidv4(), |
There was a problem hiding this comment.
no need for the uuid use here.
while you are dealing with a server this is just like an extra information you are sending to the server, the server is just handling this for us.
| deleteTodo = async id => { | ||
| this.state = "Loading"; | ||
| try { | ||
| await this.handleFetch(`${url}/todos/${id}`, "DELETE"); |
There was a problem hiding this comment.
we are not using a the response data because no need for it 💯 🥇
| } | ||
| }; | ||
| @action | ||
| updateTodo = async (id, newTodo) => { |
No description provided.