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

fix: fix email suggestion #1429

Merged

Conversation

JoandersonPaiva
Copy link
Contributor

Olá @aprendendofelipe, esse PR traz algumas correções para a sugestão de e-mail.
No componente de cadastro a sugestão de e-mail não estava renderizando por que estávamos tentando renderizar dois components de validação ao mesmo tempo, impedindo a sugestão ser renderizada para o usuário, a solução foi incluir uma validação se o objeto de erro possuía o campo mensagem, assim nessa renderização condicional só mostramos quando retornar um erro da api não tendo conflito com a outra renderização que está ligada com o que ocorre no frontend.
image
image

A segunda correção fiz no componente do perfil, onde estávamos tentando manipular o passwordRef sendo que não possuíamos essa variável nesse componente.
image

Aproveitando que já estava com a mão na massa ataquei aquele TODO para a função suggestEmail, como essa função se repetia nos dois componentes que mexi, trouxe ela para um outro ponto onde pudesse reaproveitar ela para os dois componentes, levei um tempo pensado onde poderia colocar ela, acabei trazendo ela para uma pasta utils na raiz do projeto, acha uma boa? ah também aproveitei e refatorei a função de sugestão de e-mail mudando a variável domain de uma matriz para um objeto.
Abraços! 🤝

@vercel
Copy link

vercel bot commented May 31, 2023

@JoandersonPaiva is attempting to deploy a commit to the TabNews Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boa @JoandersonPaiva, obrigado pelo PR! 💪

levei um tempo pensado onde poderia colocar ela, acabei trazendo ela para uma pasta utils na raiz do projeto, acha uma boa?

Como é uma função do frontend, e pela estrutura de pastas atual, deve ser melhor colocar dentro de pages>interface. Talvez adicionar essa pasta utils ali e usar o index.js que já existe para exportar a função.

refatorei a função de sugestão de e-mail mudando a variável domain de uma matriz para um objeto.

Show! Veja o que acha dos comentários que fiz no código 👍

utils/email-suggestion.js Outdated Show resolved Hide resolved
utils/email-suggestion.js Outdated Show resolved Hide resolved
@vercel
Copy link

vercel bot commented May 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tabnews ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 31, 2023 11:58pm

@JoandersonPaiva
Copy link
Contributor Author

fala @aprendendofelipe!! Obrigado pelas sugestões!

Como é uma função do frontend, e pela estrutura de pastas atual, deve ser melhor colocar dentro de pages>interface. Talvez adicionar essa pasta utils ali e usar o index.js que já existe para exportar a função.

show faz sentido, já fiz essa mudança! 💪

Como o @Rafatcb apontou nesse comentário (#1113 (comment)), essa linha está duplicada.

Já que criou esse novo objeto, poderia aproveitar para deixar os itens em ordem alfabética, o que facilita as manutenções. 👍

bacana, removi a linha duplicada e ordenei os domínios 🚀

A definição dessa constante poderia sair de dentro da função

sim sim, faz sentido, trouxe essa constante para fora da função. 🤝
É isso, se tiver algum outro ponto é só falar. Abraços!! 👊

@aprendendofelipe aprendendofelipe merged commit 4aa9b10 into filipedeschamps:main Jun 1, 2023
5 checks passed
@aprendendofelipe
Copy link
Collaborator

Mais uma em produção!

Valeu @JoandersonPaiva! 💪

@JoandersonPaiva
Copy link
Contributor Author

Mais uma em produção!

Valeu @JoandersonPaiva! 💪

tmj @aprendendofelipe 🤝, aos poucos pegando mais contexto da aplicação!

@aprendendofelipe
Copy link
Collaborator

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

2 participants