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

Encryption password UX #492

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Encryption password UX #492

merged 1 commit into from
Sep 20, 2023

Conversation

benalleng
Copy link
Collaborator

@benalleng benalleng commented Aug 25, 2023

Closes #490 after Mutiny-node/#727 is merged

Adds similar logic to prevent #462 from cropping up here

@cloudflare-pages
Copy link

cloudflare-pages bot commented Aug 25, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 07f839d
Status: ✅  Deploy successful!
Preview URL: https://85ebde16.mutiny-web.pages.dev
Branch Preview URL: https://issue490.mutiny-web.pages.dev

View logs

@benalleng benalleng force-pushed the issue490 branch 2 times, most recently from 31af0b4 to 41aba8e Compare August 28, 2023 13:35
@benalleng benalleng changed the title Must be a new password to encrypt Encryption password UX Aug 28, 2023
@benalleng

This comment was marked as resolved.

@benalleng benalleng marked this pull request as draft August 28, 2023 20:54
@benalleng benalleng force-pushed the issue490 branch 4 times, most recently from 3419331 to 447d302 Compare August 29, 2023 00:54
@benalleng
Copy link
Collaborator Author

benalleng commented Aug 29, 2023

Upon learning how modular forms works more, I don't think createEffect is a good route afterall, the standard validate after submitting is sufficient. It continuosly checks on its own after submitting once already.

Copy link
Collaborator

@futurepaul futurepaul left a comment

Choose a reason for hiding this comment

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

looks / works great just needs rebase. thank you!

createForm<EncryptPasswordForm>({
initialValues: {
existingPassword: "",
password: "",
confirmPassword: ""
},
validate: (values) => {
setError(undefined);
Copy link
Collaborator

Choose a reason for hiding this comment

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

makes more sense to put this in handleFormSubmit because that's where the setter happens

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just tried this, but I remembered why its here, we need the error to reset on any change to the form and not only on submit since an error prevents the user from submitting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Due to the encryptButtonDisabled the encrypt button is unclickable until the user has made a change to any of the forms

Copy link
Collaborator

Choose a reason for hiding this comment

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

ahh k makes sense

@benalleng benalleng force-pushed the issue490 branch 2 times, most recently from 0d1ae85 to d8e7d28 Compare September 20, 2023 21:25
Copy link
Collaborator

@futurepaul futurepaul left a comment

Choose a reason for hiding this comment

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

<3

@futurepaul futurepaul merged commit 8f90cc1 into master Sep 20, 2023
4 checks passed
@futurepaul futurepaul deleted the issue490 branch September 20, 2023 21:54
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.

Disallow same password when changing password
3 participants