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 profile validation and regex error #1848

Merged
merged 7 commits into from
Sep 5, 2023

Conversation

rachellougee
Copy link
Contributor

@rachellougee rachellougee commented Aug 31, 2023

What are the relevant tickets?

#1813
#1788

Description (What does it do?)

This PR fixes regex error in console. The regex error is related to breaking changes in https://github.com/tc39/proposal-regexp-v-flag#how-is-the-v-flag-different-from-the-u-flag for special character ( ) [ ] { } /

How can this be tested?

Make sure no console error for form validation and you get default error message and focus if name field are entered incorrectly

  • Test create an account
  • Test update your existing profile

Additional Context

Currently first_name and last_name use build-in pattern for regex validation, that regex error from console is due to HTML pattern now uses RegExp v flag instead of u flag (link), some previously valid patterns are now errors e.g. ( ) [ { } requires escaping inside character class

@rachellougee rachellougee linked an issue Aug 31, 2023 that may be closed by this pull request
@rachellougee rachellougee force-pushed the 1813-regex-error-on-create-accountdetails-form branch from 5474b77 to 8ebcbfe Compare August 31, 2023 18:58
@rachellougee rachellougee force-pushed the 1813-regex-error-on-create-accountdetails-form branch from 8ebcbfe to e1c8bd8 Compare August 31, 2023 19:00
@collinpreston collinpreston self-assigned this Sep 1, 2023
Copy link
Contributor

@collinpreston collinpreston left a comment

Choose a reason for hiding this comment

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

So I think the reason that I removed the components and replaced them with HTML validations and a required indicator was to improve accessibility for visually impaired users using a screen reader.

From my understanding, the functionality that is important to achieve that level of accessibility is:

  1. Reading "required" when reading the field label.
  2. Focus is set to the top-most field which contains an error upon submitting the form. The field error is then read.

I also noticed that the "Highest Level of Education" and "Are you a" are not displaying on the Profile edit page.

@rachellougee
Copy link
Contributor Author

I also noticed that the "Highest Level of Education" and "Are you a" are not displaying on the Profile edit page.

@collinpreston I believe that is not related to this PR, you would need FEATURE_ENABLE_ADDL_PROFILE_FIELDS=true in your env file. You just had a PR to remove that feature flag, but it's not in this branch.

Also how do you suggest to fix this? I can add it back, but as indicated in the issue, just fixing the regex doesn't seem to solve the issue as error message isn't displayed if user types the special character in name field. Let you know if you have thoughts.

@rachellougee
Copy link
Contributor Author

@collinpreston I've fixed the regex error in the console, and it should now works if name field contains special character indicated in title. Since its build-in pattern, it can't work with ErrorMessage component from formik. But at least it works the same way as the existing full name validation. Please take a look, thank you

Copy link
Contributor

@collinpreston collinpreston 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. Tested with VoiceOver. I agree with your thoughts about a future feature which will display the error message above or below the field, in addition to the HTML5 provided tooltip.

@rachellougee rachellougee merged commit b673538 into main Sep 5, 2023
@rachellougee rachellougee deleted the 1813-regex-error-on-create-accountdetails-form branch September 5, 2023 17:01
@odlbot odlbot mentioned this pull request Sep 7, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

regex error on create-account/details form
2 participants