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

Gg/form validation cherrypick #65

Merged
merged 8 commits into from
Aug 26, 2024
Merged

Conversation

gabrielegranello
Copy link
Collaborator

@gabrielegranello gabrielegranello commented Aug 23, 2024

What does this PR do?

After making a mess with all the commits which made the review impossible, I used the cherry pick command to actually select the meaningful ones. Essentially:

  1. It creates a new schema for passing the data to the API.
  2. It adds a spinning wheel while loading the data
  3. It adds the useForm react command instead of useState. This allows also to validate the form data.

Copy link

vercel bot commented Aug 23, 2024

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

Name Status Preview Comments Updated (UTC)
fairhold-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 26, 2024 7:01am

@gabrielegranello gabrielegranello marked this pull request as ready for review August 23, 2024 14:42
@gabrielegranello gabrielegranello requested a review from a team August 23, 2024 14:42
Copy link
Collaborator

@zz-hh-aa zz-hh-aa left a comment

Choose a reason for hiding this comment

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

Looks good to me, nice work on the cherry pick!

Small thing for future: maybe it would have been better separation of concerns to leave the loading spinner in a separate branch / PR?

</label>
if (view === "form") {
return (
<div className="flex -centeitemsr justify-center text-black mt-5">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Think this is a typo, maybe centeritems or something was meant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch!

@gabrielegranello
Copy link
Collaborator Author

Looks good to me, nice work on the cherry pick!

Small thing for future: maybe it would have been better separation of concerns to leave the loading spinner in a separate branch / PR?

Yes you are 100% sorry about that. Actually the spinner was an independent PR originally, but then it was caught in all the rebasing mess. Lesson learnt for the future

@gabrielegranello gabrielegranello merged commit e759a00 into main Aug 26, 2024
3 checks passed
@gabrielegranello gabrielegranello deleted the gg/form-validation-cherrypick branch August 26, 2024 07:03
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.

2 participants