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

Issue 110 - Warn about required fields not being filled in in forms #143

Conversation

allyharrison
Copy link
Collaborator

@allyharrison allyharrison commented Jul 19, 2024

Change Summary

Extends the <Form/> component so that required fields display an error if they are left empty.

Screenshot from 2024-07-19 12-51-34

Change Form

  • The pull request title has an issue number
  • The change works by "Smoke testing" or quick testing
  • The change has tests
  • The change has documentation

Other Information

The original ticket suggested including "(required)" in the label automatically, but I have excluded from this PR.

It consumes a lot of space and feels unnecessary, especially if most fields will be required anyway. Might be better to do the inverse and label non-required fields as "(optional)".

Screenshot from 2024-07-19 12-41-38

Related issue

@allyharrison allyharrison added enhancement New feature or request frontend Task must have a front end issue difficulty:hard labels Jul 19, 2024
Copy link

vercel bot commented Jul 19, 2024

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

Name Status Preview Comments Updated (UTC)
penni ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2024 4:51am

@SodaVolcano
Copy link
Collaborator

For the "(optional)" I was referring to the subtitles (the smaller grey text) but it can be added to the title instead. I wasn't sure if "(*)" would be better as it's shorter, but if there's no subtitle then only having "(*)" as the subtitle might look a bit weird

@SodaVolcano
Copy link
Collaborator

Btw is the callout centered? The text should be a bit more to the left I think. If the callout comes with padding or margin feel free to remove it (it's not used in anywhere else apart from component showcase I think so the display shouldn't break)

@yunho7687 yunho7687 requested a review from SodaVolcano July 20, 2024 14:37
Copy link
Collaborator

@SodaVolcano SodaVolcano left a comment

Choose a reason for hiding this comment

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

lgtm!

@yunho7687 yunho7687 merged commit 42e26b1 into main Jul 22, 2024
18 checks passed
@yunho7687 yunho7687 deleted the issue-110-Warn_about_required_fields_not_being_filled_in_in_forms branch July 22, 2024 05:13
@ErikaKK
Copy link
Contributor

ErikaKK commented Jul 23, 2024

Found some bug. Cannot submit the form when fill in every field

@ErikaKK ErikaKK restored the issue-110-Warn_about_required_fields_not_being_filled_in_in_forms branch July 23, 2024 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request frontend Task must have a front end issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show/warn about required fields not being filled in in forms
4 participants