Skip to content
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

refactor: update toast component ♻️ #929

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Jesus-Rojas
Copy link
Contributor

Descripción

Existe demasiado condicional en la funcion toast, la mayoria de casos valida si existe o no cierto toastOption. Tambien he visto demasiados strings mágicos.

Cambios propuestos

  • Todos los campos son opcionales, la mejor opción para evitar tanto condicional es crear parámetros por defecto.
  • Actualice el componente toast para usar enums en vez de strings mágicos, esto incrementa la mantenibilidad del componente toast y se evitan bugs innecesarios.

Comprobación de cambios

  • He revisado que no haya ninguna PR (pull request) ya abierta con un problema similar, siguiendo el apartado de buenas prácticas
  • He revisado localmente los cambios para asegurarme de que no haya errores ni problemas.

Impacto potencial

En cuanto a UX no afecta, en cuanto a mantenibilidad si es necesario el cambio.

Copy link

vercel bot commented Apr 24, 2024

@Jesus-Rojas is attempting to deploy a commit to the midudev pro Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

⚠️ Esta Pull Request tiene conflictos. Por favor, resuelvelos antes de que podamos evaluar los cambios.

Copy link

⚠️ Esta Pull Request tiene conflictos. Por favor, resuelvelos antes de que podamos evaluar los cambios.

Copy link

⚠️ Esta Pull Request tiene conflictos. Por favor, resuelvelos antes de que podamos evaluar los cambios.

Copy link

✅ ¡Los conflictos han sido resuletos! Un colaborador revisará pronto la Pull Request.

@Jesus-Rojas
Copy link
Contributor Author

@midudev merge this !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant