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

feat: Add other tabs for settings page #118

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

Conversation

jsun969
Copy link
Member

@jsun969 jsun969 commented Mar 9, 2024

Description

Add other tabs for settings page

Changes Made

  • Fix button style
  • Add account & personal info tab
  • Minor fix

Related Issues

N/A

Additional Notes

Copy link

vercel bot commented Mar 9, 2024

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

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 18, 2024 0:52am

@jsun969 jsun969 marked this pull request as ready for review March 10, 2024 13:34
src/app/(account)/settings/tabs/PersonalInfoSettings.tsx Outdated Show resolved Hide resolved
src/app/(account)/settings/tabs/PersonalInfoSettings.tsx Outdated Show resolved Hide resolved
Comment on lines +30 to +36
// Avoid creating duplicate email addresses. Find email address (unverified) first.
let newEmailInst = user?.emailAddresses.find(
({ emailAddress }) => emailAddress === data.email
);
if (!newEmailInst) {
newEmailInst = await user?.createEmailAddress({ email: data.email });
}
Copy link
Member

Choose a reason for hiding this comment

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

Unhandled error when changing email address and removing email address linked to Google account.

Clerk gives error "This email address is linked to one or more Connected Accounts. Remove the Connected Account before deleting this email address."

However, the primary email is set to the new one as expected, but the old email address linked to the Google account is still there.

Comment on lines 115 to 153
function ChangePassword() {
const { user } = useUser();

const form = useForm<z.infer<typeof passwordSchema>>({
defaultValues: { oldPassword: '', newPassword: '', confirmPassword: '' },
resolver: zodResolver(passwordSchema),
});
const handleSubmit = form.handleSubmit((data) => {
user?.updatePassword({ newPassword: data.newPassword, currentPassword: data.oldPassword });
});

return (
<form className="space-y-4" onSubmit={handleSubmit}>
<h2 className="text-2xl font-bold">Change Password</h2>
<div className="mb-2 border-b-2 border-black" />
<ControlledField
control={form.control}
name="oldPassword"
label="Old Password"
type="password"
/>
<ControlledField
control={form.control}
name="newPassword"
label="New Password"
type="password"
/>
<ControlledField
control={form.control}
name="confirmPassword"
label="Confirm Password"
type="password"
/>
<Button type="submit" colour="orange">
Update password
</Button>
</form>
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't seem to handle Clerk password errors for compromised passwords and/or minimum strength.

src/app/(account)/settings/tabs/AccountSettings.tsx Outdated Show resolved Hide resolved
Comment on lines +166 to +169
const handleRemove = async () => {
await google?.destroy();
window.location.reload();
};
Copy link
Member

Choose a reason for hiding this comment

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

Unlink account should also remove the associated email address if there are two emails addresses.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there is a connection between email and google account.

Copy link
Member

Choose a reason for hiding this comment

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

On Clerk there is, linking a Google account sets the associated email as your default email.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I'll test this later

src/app/(account)/settings/Settings.tsx Outdated Show resolved Hide resolved
src/app/(account)/settings/tabs/AccountSettings.tsx Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants