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

Dashboard settings form QOL improvements #51

Merged
merged 4 commits into from
Jul 31, 2023
Merged

Dashboard settings form QOL improvements #51

merged 4 commits into from
Jul 31, 2023

Conversation

zahid47
Copy link
Contributor

@zahid47 zahid47 commented Jul 30, 2023

addresses #49

shortBio is marked as optional in the Prisma schema, so the initial value is null which doesn't match with the type CurrentUser. So I'm setting it as an empty string if it is null. Not sure if it is the best approach, but any advice is appreciated!

@vercel
Copy link

vercel bot commented Jul 30, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @moinulmoin on Vercel.

@moinulmoin first needs to authorize it.

@zahid47 zahid47 changed the title Settings form qol Dashboard settings form QOL improvements Jul 30, 2023
@moinulmoin
Copy link
Owner

Thanks for the PR. I will check soon

@vercel
Copy link

vercel bot commented Jul 31, 2023

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

Name Status Preview Comments Updated (UTC)
chadnext ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2023 2:43pm

moinulmoin

This comment was marked as duplicate.

Comment on lines 31 to 39
shortBio: z
.string({
required_error: "Please type a short bio.",
})
.string()
.max(150, {
message: "Short bio must be at most 150 characters.",
})
.min(10, {
message: "Short bio must be at least 10 characters.",
}),
})
.or(z.literal("")),
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
shortBio: z
.string({
required_error: "Please type a short bio.",
})
.string()
.max(150, {
message: "Short bio must be at most 150 characters.",
})
.min(10, {
message: "Short bio must be at least 10 characters.",
}),
})
.or(z.literal("")),
shortBio: z
.string()
.max(150, {
message: "Short bio must be at most 150 characters.",
})
.min(10, {
message: "Short bio must be at least 10 characters.",
})
.optional(),

@@ -48,7 +48,7 @@ export default function SettingsForm({
values: {
name: currentUser.name,
email: currentUser.email,
shortBio: currentUser.shortBio,
shortBio: currentUser.shortBio || "",
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
shortBio: currentUser.shortBio || "",
shortBio: currentUser.shortBio,

@@ -123,7 +124,7 @@ export default function ImageUploadModal({
loading="lazy"
/>
</div>
<div className=" mt-10">
<div className="mt-10 flex">
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<div className="mt-10 flex">
<div className="mt-10">

@@ -187,15 +187,18 @@ export default function SettingsForm({
)}
/>

<div>
<div className="flex">
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<div className="flex">
<div>

@moinulmoin
Copy link
Owner

All good. just need a few changes and then ready to merge. Thanks

@zahid47
Copy link
Contributor Author

zahid47 commented Jul 31, 2023

you can just call optinal method at the end to make it optional. take a look here

https://zod.dev/?id=optionals

This was my first intuition as well. But it doesn't work if you type something in the short bio field and then remove it. Because then it becomes an empty string (""), but it won't let you update your profile with bio as an empty string as min length is 10 chars.

And the flex class in the other file is to fix the alignment with "cancel" button. During loading state the alignment gets messed up and causes layout shift because of the loading icon

@zahid47 zahid47 requested a review from moinulmoin July 31, 2023 13:49
@moinulmoin
Copy link
Owner

And the flex class in the other file is to fix the alignment with "cancel" button. During loading state the alignment gets messed up and causes layout shift because of the loading icon

Hey,

you can just call optinal method at the end to make it optional. take a look here
https://zod.dev/?id=optionals

This was my first intuition as well. But it doesn't work if you type something in the short bio field and then remove it. Because then it becomes an empty string (""), but it won't let you update your profile with bio as an empty string as min length is 10 chars.

And the flex class in the other file is to fix the alignment with "cancel" button. During loading state the alignment gets messed up and causes layout shift because of the loading icon

Hey, seems you are right, I removed the min and max and I noticed font flickering issue while checking the buttons layout shift. I added few adjustments. thanks.

@moinulmoin moinulmoin merged commit 481eae4 into moinulmoin:main Jul 31, 2023
@moinulmoin moinulmoin linked an issue Jul 31, 2023 that may be closed by this pull request
@zahid47 zahid47 deleted the settings-form-qol branch July 31, 2023 14:55
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.

Dashboard settings QOL
2 participants