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-#77: changed the traditional form to react hook form #407

Merged
merged 3 commits into from
Jun 8, 2024

Conversation

Ameerjafar
Copy link
Contributor

@Ameerjafar Ameerjafar commented Jun 7, 2024

Summary

I have converted the react traditional form to the react hook form.

Description

I have done the react hook form to reduce the lot of re-renders at the same time users can easily identify the error while they are trying to add the blog in the application and for the form validation i have used zod library.

Images

2024-06-08.11-43-27.mp4

Issue(s) Addressed

Enter the issue number of the bug(s) that this PR fixes

Prerequisites

Copy link

vercel bot commented Jun 7, 2024

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

Name Status Preview Comments Updated (UTC)
wanderlust ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 8, 2024 6:13am
wanderlust-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 8, 2024 6:13am

Copy link

github-actions bot commented Jun 7, 2024

Hey @Ameerjafar! Thanks for sticking to the guidelines! High five! 🙌🏻

@Ameerjafar
Copy link
Contributor Author

@krishnaacharyaa can you kindly review this one sir.

@krishnaacharyaa
Copy link
Owner

@Ameerjafar can you kindly add the video

@krishnaacharyaa
Copy link
Owner

image

Kindly update the validations to these and messages also,

And once go through https://github.com/krishnaacharyaa/wanderlust/pull/83/files, might be of help

Copy link

github-actions bot commented Jun 8, 2024

Hey @Ameerjafar! Thanks for sticking to the guidelines! High five! 🙌🏻

@krishnaacharyaa
Copy link
Owner

@Ameerjafar in the video, when no categories is selected it says "max three categories" and when you click 2 categories it won't go...

Shoudln't that error come when you click more than 3 categories?
And when you haven't clicked it should say please select a category something and when you click a category the error message should go

Copy link

github-actions bot commented Jun 8, 2024

Hey @Ameerjafar! Thanks for sticking to the guidelines! High five! 🙌🏻

Copy link
Owner

@krishnaacharyaa krishnaacharyaa left a comment

Choose a reason for hiding this comment

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

Hi @Ameerjafar , thank you
When 1 category I didn't see the error message vanishing in the video.

And please also select 3 categories and see if lets you select 4th one

@@ -28,6 +28,38 @@ export const signUpSchema = z
message: 'Confirm Password do not match',
path: ['confirmPassword'],
});
const isValidImageLink = (value: string) => {
const imageLinkRegex = /\.(jpg|jpeg|png|webp)$/i;
Copy link
Owner

Choose a reason for hiding this comment

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

Do we actually need a function? Can't we inline using the .validate option which zod provides directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.validate is when we are giving some url to test whether that url meets the defined schema or not @krishnaacharyaa

Copy link
Owner

Choose a reason for hiding this comment

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

@Ameerjafar okay if we can't inline then fine, but i guess inline/arrow function can be written

})
.max(3, {
message: 'Easy there! Not more than 3 categories.',
}),
Copy link
Owner

Choose a reason for hiding this comment

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

😂

 (max 3 categories) 
</span>
<Asterisk />
<br></br>
{errors.categories && (
<span className="text-sm text-red-500">{`${errors.categories.message}`}</span>
Copy link
Owner

Choose a reason for hiding this comment

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

The error section of the categories should go below the categories selection ig, because in every other thing error is below, only in the cateogry it is above the actual category section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done sir

@Ameerjafar Ameerjafar closed this Jun 8, 2024
@Ameerjafar Ameerjafar reopened this Jun 8, 2024
Copy link

github-actions bot commented Jun 8, 2024

Hey @Ameerjafar! Thanks for sticking to the guidelines! High five! 🙌🏻

Copy link
Owner

@krishnaacharyaa krishnaacharyaa left a comment

Choose a reason for hiding this comment

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

Thanks for the efforts as always @Ameerjafar

@krishnaacharyaa krishnaacharyaa merged commit af322ce into krishnaacharyaa:main Jun 8, 2024
5 checks passed
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.

Refactor Form vailidation using the react-hook-form and zod
2 participants