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

required fields #83

Merged
merged 3 commits into from
Sep 2, 2021
Merged

required fields #83

merged 3 commits into from
Sep 2, 2021

Conversation

granttebeau
Copy link
Collaborator

Why

no monday tickets for this one #rip

#78
#79
#80
#81

Combined all of these tickets into one branch. Each one deals with adding a required rule to the forms so that the user isn't able to submit the form unless all fields have been filled out (as well as some other smaller changes).

This PR

Mostly adding this rule: "rules={[{ required: true, message: 'Required' }]}" or some variation of it to each field in the respective forms that needs to be required. It's different for the email and password fields. I also added a * to the end of each required field so that the user can see it's required.

Side note: I didn't add the required fields to the number of books and most recent shipment year fields because those values won't be necessary once #52 is merged.

Screenshots

Screen Shot 2021-08-16 at 12 00 34 PM

Screen Shot 2021-08-16 at 12 00 59 PM

Screen Shot 2021-08-16 at 12 01 16 PM

Screen Shot 2021-08-16 at 12 01 53 PM

Verification Steps

Log in as admin user and verify that the required fields are working as they should in the following pages:

  • submitting a report (with and without a library)
  • adding a contact to a school in the report flow
  • creating/adding a school
  • creating/adding a user

@coveralls
Copy link

coveralls commented Aug 16, 2021

Pull Request Test Coverage Report for Build 1136147456

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage remained the same at 17.018%

Files with Coverage Reduction New Missed Lines %
src/components/report/common/VisitReason.tsx 2 0%
src/components/schoolContact/SchoolContact.tsx 2 0%
src/components/schoolDirectory/CreateSchool.tsx 3 0%
Totals Coverage Status
Change from base Build 1116631719: 0.0%
Covered Lines: 250
Relevant Lines: 1249

💛 - Coveralls

Copy link
Contributor

@maxsebso maxsebso left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@rymaju rymaju left a comment

Choose a reason for hiding this comment

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

IMO the * for the placeholders is a little weird but thats fine, LGTM

@maxsebso maxsebso changed the base branch from master to Frontend-Staging September 2, 2021 17:41
…across-the-sea-frontend into GT.EverySingleRequiredField
@maxsebso maxsebso merged commit a74a852 into Frontend-Staging Sep 2, 2021
@jtavera235 jtavera235 deleted the GT.EverySingleRequiredField branch January 22, 2022 18:51
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.

4 participants