-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix profile validation and regex error #1848
Conversation
5474b77
to
8ebcbfe
Compare
8ebcbfe
to
e1c8bd8
Compare
There was a problem hiding this 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:
- Reading "required" when reading the field label.
- 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.
@collinpreston I believe that is not related to this PR, you would need 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. |
@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 |
There was a problem hiding this 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.
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
Additional Context
Currently
first_name
andlast_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