[#45] Agregada verificacion al login #48
Conversation
Durphan
commented
Feb 29, 2024
Durphan
commented
- Agregada verificacion a la login page utilizando RegExp
- Agregado dos props al Button primary
- Cambio en el estilo del logo del login
- Fixeo de la llamada a una funcion en Order
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
package-lock.json
Outdated
There was a problem hiding this comment.
Existen muchos cambios en el package-lock, estos a qué se deben?
src/atoms/CoffeeBanner.jsx
Outdated
| justify-content: space-between; | ||
| gap: 1rem; | ||
| align-items: center; | ||
| justify-content: center; |
There was a problem hiding this comment.
Podemos agregar a la descripción del PR, una captura de la web antes - después de este cambio?
src/molecules/ChoosePayment.jsx
Outdated
| @@ -21,6 +21,10 @@ const SubHeaderSection = styled.div` | |||
| `; | |||
|
|
|||
| export default function ChoosePayment() { | |||
| function handleClick(){ | |||
| console.log("payment") | |||
There was a problem hiding this comment.
El console.log nos servira en el dia a dia de desarrollo, pero no deberia estar en produccion, podemos tambien dejarle asociado in TODO, para darle funcionalidad a dicho handle.
// TODO: ISSUE-01 Crear funcionalidad para elegir el método de pago al hacer click.
console.log("payment")
src/molecules/LoginForm.jsx
Outdated
| function validateEmail(emailToValidate) { | ||
| const emailRegex = /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/; | ||
| if (!emailRegex.test(emailToValidate)) { | ||
| console.log("El email debe contener un @ y .com"); |
There was a problem hiding this comment.
Quizás en este punto deberíamos retornar un String, para poder renderizar dicho mensaje de cara al usuario en lugar de sólo verlo por consola.
src/molecules/LoginForm.jsx
Outdated
| } | ||
|
|
||
| function validatePassword(passwordToValidate) { | ||
| const passwordRegex = /[0-9]/; |
There was a problem hiding this comment.
Podriamos utilizar una rgx, que haga todo lo que se valida en los ifs, entonces tenemos una función más limpia:
^(?=.*[0-9]).{5,}$
Entonces, si la password no coincide con el patrón, le retornamos el mensaje al usuario para que vuelva a ingresar la contraseña hasta que sea válida
| if (!emailRegex.test(emailToValidate)) { | ||
| setEmailError(`Asegurese de colocar un email correcto`); | ||
| } else { | ||
| setEmailError(``); |
There was a problem hiding this comment.
setEmailError, y setPasswordError, existen?
There was a problem hiding this comment.
Al parecer son setters de estados del componente.
src/atoms/p.js
Outdated
|
|
||
| import colors from '../styles/colors'; | ||
|
|
||
| export const ErrorP = styled.p` |
There was a problem hiding this comment.
Aquí me parece que quedaría claro que el atomo tenga un nombre descriptivo. I.E. Paragraph.
src/molecules/Header.jsx
Outdated
| @@ -8,7 +8,7 @@ import {TopMenu} from './TopMenu'; | |||
| const Container = styled.div` | |||
| display: flex; | |||
| align-items: center; | |||
| justify-content: space-between; | |||
| justify-content: space-around; | |||
There was a problem hiding this comment.
En lugar de mover la justificación de contenido, probar con el padding para acercarse mas al diseño.
| @@ -21,6 +21,10 @@ const SubHeaderSection = styled.div` | |||
| `; | |||
|
|
|||
| export default function ChoosePayment() { | |||
| function handleClick() { | |||
| console.log('payment'); | |||
There was a problem hiding this comment.
No debe hacerse commit de los console.log. Si una función debe ser implementada, dejar un comentario o tirar una excepción.
src/styles/colors.js
Outdated
| @@ -7,6 +7,7 @@ const wordsColor = { | |||
| negro: '#000', | |||
| gris: '#C5C5C5', | |||
| marron: '#5E3E00', | |||
| rojo: `#fc0339`, | |||
There was a problem hiding this comment.
¿Los colores están definidos en ambos idiomas? Creo que deberíamos de unificarlos a todos en inglés.
