Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

Add React hook form validations #9835

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

ChinmayMhatre
Copy link
Member

@ChinmayMhatre ChinmayMhatre commented Nov 25, 2023

Fixes Issue

Closes #8031

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

image

Note to reviewers

@ChinmayMhatre ChinmayMhatre marked this pull request as draft November 25, 2023 11:27
@github-actions github-actions bot added issue linked Pull Request has issue linked dependencies Pull requests that update a dependency file and removed issue linked Pull Request has issue linked labels Nov 25, 2023
@github-actions github-actions bot added the issue linked Pull Request has issue linked label Nov 25, 2023
@ChinmayMhatre
Copy link
Member Author

Progress / steps done

  1. Installed react-hook-forms and resolver for zod
  2. replaced react states with zod schema
image

May need to be changed. I looked at the mongoose schema that is used for server side validation

  1. Registered the inputs using rhf thus removing value and onchange
  2. updated the Textarea component to have forwardRef which is needed by rhf.
  3. Some inputs like toggle and icon search needed to be wrapped with Controller from rhf.
  4. Each input now has a error message if needed (style and test probably need some changes as well)
image
  1. userMilestone component needed states thus used watch from rhf to watch for changes thus live updating

@ChinmayMhatre
Copy link
Member Author

@eddiejaoude let me know what you think when you're available!

date,
isGoal,
dateFormat,
title: milestoneWatch[0],
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way not to use position [0]? Because I can see this creating bugs

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't get what you mean here

Copy link
Member

Choose a reason for hiding this comment

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

I mean using [0] seems strange to me? why 0, for example it causes confusion, why not 1 or 2

@eddiejaoude
Copy link
Member

Looks good (I didn't test it yet though)

@ChinmayMhatre
Copy link
Member Author

Looks good (I didn't test it yet though)

I'll make the updates in a day or two a bit busy

Co-authored-by: Eddie Jaoude <[email protected]>
@ChinmayMhatre
Copy link
Member Author

Busy this week. Will make the change as soon as i get some time

@ChinmayMhatre
Copy link
Member Author

@eddiejaoude can you check the PR once, let me know if we should go ahead and implement it in the other forms

@ChinmayMhatre
Copy link
Member Author

@eddiejaoude just wanted to ping you incase this slipped through the cracks. I have added some comments , let me know what you think

@eddiejaoude
Copy link
Member

Thanks @ChinmayMhatre - Let's also check with the other maintainers.

Could you check inline comments

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Pull requests that update a dependency file issue linked Pull Request has issue linked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OTHER] Forms validations approach
4 participants