-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#45] Agregada verificacion al login #48
base: dev
Are you sure you want to change the base?
Conversation
Durphan
commented
Feb 29, 2024
- 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 ↗︎
|
src/atoms/CoffeeBanner.jsx
Outdated
justify-content: space-between; | ||
gap: 1rem; | ||
align-items: center; | ||
justify-content: center; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Podemos agregar a la descripción del PR, una captura de la web antes - después de este cambio?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No entiendo Brahian.
¿Qué se explique este cambio pides?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listo.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
¿Los colores están definidos en ambos idiomas? Creo que deberíamos de unificarlos a todos en inglés.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I touched ChoosePayment.jsx to eliminate the use of consolo.log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.