Solution: Ricard Garcia & Victor Hugo Gomez #6
Conversation
| @@ -0,0 +1,87 @@ | |||
| ### First meeting | |||
There was a problem hiding this comment.
Muy interesante la organización! 👏🏻
| import CreateTodo from "./components/CreateTodo"; | ||
| import TodoList from "./components/TodoList"; | ||
|
|
||
| const CUSTOM_LS_KEY = "todos"; |
There was a problem hiding this comment.
Los nombres de variables deberían ser más descriptivos. Realmente no se puede saber para qué sirve esta variable sin llegar a leer dónde se utiliza como argumento de una función. Un nombre más correcto sería: CUSTOM_LOCALSTORAGE_KEY
| componentDidUpdate() { | ||
| const { todos } = this.state; | ||
| // eslint-disable-next-line no-console | ||
| console.log(this.state); |
There was a problem hiding this comment.
Se deberían eliminar las llamadas a console.log en la versión final del proyecto.
| const selInput = document.getElementById(`input-${todoId}`); | ||
| selInput.disabled = !selInput.disabled; | ||
| const { todos } = this.state; | ||
| const updatedTodos = todos.map((todo) => { | ||
| if (todo.id === todoId) { | ||
| return { | ||
| ...todo, | ||
| done: !todo.done, | ||
| }; | ||
| } | ||
| return todo; | ||
| }); | ||
| this.setState({ todos: updatedTodos }); |
| ? "https://images.unsplash.com/photo-1483728642387-6c3bdd6c93e5?ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&ixlib=rb-1.2.1&auto=format&fit=crop&w=1955&q=80" | ||
| : "https://images.unsplash.com/flagged/photo-1551301622-6fa51afe75a9?ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&ixlib=rb-1.2.1&auto=format&fit=crop&w=1950&q=80" |
There was a problem hiding this comment.
Estas URL de imágenes se podrían guardar en una variable más arriba y usar el nombre de la variable aquí para que el código sea más legible.
| {/* <input type="checkbox" name="checkbox" /> */} | ||
| <div className="custom-checkbox d-flex flex-column justify-content-center align-items-center"> | ||
| {/* <i className="uil uil-check text-center" /> */} | ||
| </div> |
There was a problem hiding this comment.
Se debería eliminar el código comentado ya que no se utiliza.
|
|
||
| const classNames = require("classnames"); | ||
|
|
||
| function MainFooter({ todosLeft, darkMode, handleClearCompleted }) { |
There was a problem hiding this comment.
El prop darkMode se podría renombrar al algo como: isDarkModeActive para que sea más claro que es una prop de tipo boolean.
| const darkModeTodosLeft = classNames({ | ||
| "items-left footer-item": true, | ||
| "footer-item-dark": darkMode, | ||
| }); | ||
|
|
||
| const darkModeClear = classNames({ | ||
| "clear-all-div footer-item clear-button": true, | ||
| "footer-item-dark": darkMode, | ||
| }); | ||
|
|
||
| const darkModeNavLink = classNames({ | ||
| "nav-link": true, | ||
| "nav-link-dark": darkMode, | ||
| }); | ||
|
|
||
| const darkModeActive = classNames({ | ||
| active: true, | ||
| "active-dark": darkMode, | ||
| }); |
There was a problem hiding this comment.
Estas clases se podrían renombrar para acabar en Classes siempre. De esta forma al leer el código podremos ver variables llamadas: darkModeActiveClasses y que sea más obvio que es un objeto de clases.
<div className={darkModeTodosLeftClasses}>{todosLeft} todos left</div>| const inputClasses = classNames({ | ||
| "edit-todo-input": true, | ||
| "done-todo-text": done, | ||
| "edit-todo-input-dark": darkMode, | ||
| }); | ||
|
|
||
| const closeClasses = classNames({ | ||
| "close uil uil-times": true, | ||
| "close-dark": darkMode, | ||
| }); |
There was a problem hiding this comment.
Este es un muy buen ejemplo de nombrar las variables acabando en Classes 👏🏻👏🏻. De esta forma es muy claro que esa variable tiene clases.
| </div> | ||
| </div> | ||
| <form onSubmit={this.insideEditedTodo}> | ||
| <input |
There was a problem hiding this comment.
Los inputs deberían tener siempre un label asignado con un text que describa el input. Si no queremos que se vea visualmente el label podemos usar una convención muy común que usa clases llamadas .sr-only que lo que hace es esconder visualmente el elemento en la página pero si que está renderizado para los lectores de pantalla y el navegador.
.sr-only {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
border: 0;
}
No description provided.