Solution: Erick Noiztbander#10
Conversation
danilucaci
left a comment
danilucaci
left a comment
There was a problem hiding this comment.
Buen proyecto Erick! Felicidades! Faltan muchos requisitos principales pero lo conseguido está bastante bien 👏🏻. Seguimos!
| // src="https://images.pexels.com/photos/1232594/pexels-photo-1232594.jpeg?auto=compress&cs=tinysrgb&dpr=3&h=750&w=1260" | ||
| // src="https://images.pexels.com/photos/6439051/pexels-photo-6439051.jpeg?auto=compress&cs=tinysrgb&h=750&w=1260" |
There was a problem hiding this comment.
En este caso como es un div se debería quitar el código comentado en la versión de entrega del proyecto,
| @@ -0,0 +1,55 @@ | |||
| import { createGlobalStyle } from "styled-components"; | |||
There was a problem hiding this comment.
Es muy interesante la solución al cambio de light a dark mode pero en este caso sería más eficiente hacerlo con css normal sin importar la librería de Styled Components. Normalmente se usa una de las 2 cosas, o todo con SCSS o todo CSS in JS como Styled Components. Pero muy bien por tu parte por implementarlo y que funcione 👏🏻
|
|
||
| import "./Footer.scss"; | ||
|
|
||
| class Footer extends Component { |
There was a problem hiding this comment.
En este caso podría ser una función normal dado que no se utiliza el state.
| inputText: event.target.value, | ||
| }); | ||
|
|
||
| // console.log(inputText); |
There was a problem hiding this comment.
Se debería eliminar todo el código comentado en la versión final de la aplicación.
| placeholder="New Todo" | ||
| value={inputText} | ||
| onChange={this.formManagement} | ||
| className="font-bold font-big full-width input-no-border" |
There was a problem hiding this comment.
Muy interesante el uso de utility classes para conseguir el input. Te recomiendo echarle un vistazo a tailwind 👀.
| /> | ||
| </label> | ||
| <span className="font-bold font-big full-width">{toDo}</span> | ||
| <div className={`d-flex ${divHidden}`}> |
There was a problem hiding this comment.
El efecto de mostrar o no los botones se podrían conseguir con solo CSS lo cual evitaría cambiar el estado de la aplicación por cada hover que hagamos.
|
|
||
| const LOCAL_STORAGE_KEY = "toDoListSaved"; | ||
|
|
||
| function loadLocalStorageData() { |
There was a problem hiding this comment.
Una forma de optimizar esta función sería pasar el nombre de local storage como parámetro de la función. De esta forma podemos evitar usar variables en el estado global.
There was a problem hiding this comment.
Algo así:
function loadLocalStorageData(storageKey) {
const prevToDos = localStorage.getItem(storageKey);
if (!prevToDos) {
return null;
}
try {
return JSON.parse(prevToDos);
} catch (error) {
return null;
}
}| selectedToDoToDelete(id) { | ||
| const { toDoList } = this.state; | ||
|
|
||
| const filteredToDoList = toDoList.filter((e, index) => index !== id); |
There was a problem hiding this comment.
No es muy recomendable usar el segundo parámetro index de los métodos de array como [].map o [].filter dado que no podemos asegurar que siempre sea el mismo index que tiene nuestro elemento. Mejor sería usar una librería como uuid que permite crear ids de forma segura.
| updateToDo(id, toDo) { | ||
| const { toDoList } = this.state; | ||
|
|
||
| const updatedList = toDoList.map((e, index) => { |
There was a problem hiding this comment.
Los nombres de variable como e deberían ser una palabra completa como element para indicar de forma más clara para qué se usa la variable.
| // noActiveToDos() { | ||
| // // eslint-disable-next-line no-unused-vars | ||
| // const { toDoList } = this.state; | ||
| // toDoList.toDoList.filter( | ||
| // (e, index) => {isDone !== false} | ||
| // ); | ||
| // this.setState({ | ||
| // toDoList: filteredToDoList, | ||
| // }); | ||
| // } |
There was a problem hiding this comment.
Commented code should not be pushed to GitHub.
| } | ||
|
|
||
| editToDo() { | ||
| // const { editMode } = this.state; |
There was a problem hiding this comment.
Commented code should not be pushed to GitHub.
3 participants
No description provided.