Solution: Jonathan Cedeño & Alfonso García#12
Solution: Jonathan Cedeño & Alfonso García#12jonathancede wants to merge 33 commits intoassembler-institute:mainfrom
Conversation
first step of structure
Header and newtask
danilucaci
left a comment
danilucaci
left a comment
There was a problem hiding this comment.
Muy buen proyecto! Felicidades! Seguimos 💪
|
|
||
| this.state = { | ||
| tasks: [], | ||
| isDark: false, |
There was a problem hiding this comment.
Muy buen uso de la propiedad de estado para indicar el dark mode! Como mejora se podría usar un nombre más descriptivo como isDarkModeActive.
| handleDeleteTask(event, taskId) { | ||
| const { tasks } = this.state; | ||
|
|
||
| const updatedTasks = tasks.filter((task) => task.id !== taskId); | ||
|
|
||
| this.setState({ tasks: updatedTasks }); | ||
| } | ||
|
|
||
| handleClearCompleted() { | ||
| const { tasks } = this.state; | ||
|
|
||
| const updatedTasks = tasks.filter((task) => !task.isCompleted); | ||
|
|
||
| this.setState({ tasks: updatedTasks }); | ||
| } |
There was a problem hiding this comment.
Muy limpio y bien implementado 👏🏻👏🏻
| if (task.id === taskId) { | ||
| return { | ||
| ...task, | ||
| isEditing: true, |
There was a problem hiding this comment.
Muy interesante la solución para indicar que el todo está siendo editado actualmente.
| toggleDarkLightMode() { | ||
| const { isDark } = this.state; | ||
| if (isDark === true) this.setState({ isDark: false }); | ||
| else this.setState({ isDark: true }); | ||
| } |
There was a problem hiding this comment.
Muy bien implementado!
Como mejora podríais usar el patrón de functional updates:
this.setState((prevState) => ({
isDark: !prevState.isDark,
}));| this.state = { | ||
| isChecked: props.defaultChecked, | ||
| taskId: props.taskId, | ||
| }; |
There was a problem hiding this comment.
No es recomendable inicializar el state basandose props ya que puede generar muchos bugs. Una solución más sencilla es pasar los props de checked o taskId desde el componente padre y en el componente Checkbox usar el spread operador para pasar todos los props.
<input
type="checkbox"
onChange={this.toggleChange}
/>| if (isDark === true) mode = <img className="icon" src={SUN_PATH} alt="Sun" />; | ||
| else mode = <img className="icon" src={MOON_PATH} alt="Moon" />; |
There was a problem hiding this comment.
Los bloques if o else se deberían incluir siempre con los símbolos {} para una mejor legibilidad y para evitar que el largo de línea no pase de los 72 caracteres recomendados.
| main { | ||
| height: 85vh; | ||
| } | ||
| .list-header { | ||
| height: 20vh; | ||
| margin-bottom: 2vh; | ||
| } | ||
| .main-list { | ||
| padding-top: 7vh; | ||
| padding-bottom: 5vh; | ||
| margin: 0 auto; | ||
| width: 90vw; | ||
| max-width: 800px; | ||
| } | ||
|
|
||
| .camouflaged-button { | ||
| background-color: transparent; | ||
| border: 0; | ||
| } | ||
|
|
||
| .list-container { |
There was a problem hiding this comment.
Debería haber un espaciado más uniforme entre las clases, es decir, tener un salto de línea entre cada bloque de definición de estilos.
|
|
||
| function NewTask({ saveNewTask }) { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
En este caso no haría falta tener el Fragment dado que el componente Formik ya es un container.
| .form-wrapper { | ||
| box-shadow: rgb(16 16 16 / 60%) 0 8px 30px; | ||
| border-radius: 8px; | ||
| background-color: white; | ||
| } | ||
|
|
||
| .isCompleted-wrapper { | ||
| height: 75px; | ||
| width: 75px; | ||
| } | ||
|
|
||
| .check-new-task { | ||
| position: relative; | ||
| left: -14px; | ||
| top: -14px; | ||
| } | ||
|
|
||
| .text-input-wrapper { | ||
| height: 40px; | ||
| width: 100%; | ||
| padding-right: 15px; | ||
| } | ||
|
|
||
| .text-input { | ||
| width: 100%; | ||
| border: none; | ||
| } | ||
|
|
There was a problem hiding this comment.
Muy buen ejemplo de cómo poner un salto de línea entre cada bloque de CSS 👏🏻
3 participants
No description provided.