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

Reset Password #89

Merged
merged 12 commits into from
Apr 15, 2024
Merged

Reset Password #89

merged 12 commits into from
Apr 15, 2024

Conversation

vs2961
Copy link
Contributor

@vs2961 vs2961 commented Apr 13, 2024

Tracking Info

Resolves #83

Changes

Added forgot password page

Testing

Created an account with my email, and then tried the forget password function.

Confirmation of Change

Screenshot 2024-04-13 at 9 27 21 AM
Screenshot 2024-04-13 at 9 27 06 AM

@vs2961 vs2961 requested a review from adhi0331 as a code owner April 13, 2024 16:28
@vs2961 vs2961 changed the title Feature/vs2961/react contexts Reset Password Apr 13, 2024
Copy link

github-actions bot commented Apr 13, 2024

Visit the preview URL for this PR (updated for commit ba0e108):

https://pia-dev-60cea--pr89-feature-vs2961-react-ojhdflu8.web.app

(expires Mon, 22 Apr 2024 19:18:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: b780ee12a240535f7ca0729d49968573a1f3e284

Copy link
Member

@adhi0331 adhi0331 left a comment

Choose a reason for hiding this comment

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

Looks really good! Left a comment on one suggestion I have but other than that I think its good to go.

</Button>
{resetSent && (
<h1 className="mt-1 flex items-center text-sm font-light text-green-700">
<CheckCircle2 className="mr-1 text-sm" /> A reset password has been sent to your
Copy link
Member

Choose a reason for hiding this comment

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

This should only popup if the reset email was sent properly. If I were to try to request a password for an account that we don't have an email for then we need some sort of message for that.

Copy link
Member

Choose a reason for hiding this comment

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

Though I would like to note when I tried using an email that isn't associated with an account it never sent the password reset link which is good.

Copy link
Contributor Author

@vs2961 vs2961 Apr 15, 2024

Choose a reason for hiding this comment

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

I assumed that this was a sort of security measure, so that someone couldn't just figure out what emails do and don't have an account. This is inherently built into the firebase function that sends the reset emails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are people that take this approach, where it says something like "A reset password was sent if your email was found in the database"

Copy link
Member

Choose a reason for hiding this comment

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

That is a good point. I think the message you suggested will work.

@vs2961 vs2961 requested a review from adhi0331 April 15, 2024 00:54
Copy link
Member

@adhi0331 adhi0331 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@adhi0331 adhi0331 merged commit cc5b451 into main Apr 15, 2024
4 checks passed
@adhi0331 adhi0331 deleted the feature/vs2961/react-contexts branch April 15, 2024 20:48
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.

[MISC] - Implement Forgot Password field
2 participants