-
Notifications
You must be signed in to change notification settings - Fork 13
refactor(form): update validator error messages #1374
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @spliffone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on refining the user experience of form validation by updating and standardizing error messages. The changes aim to provide clearer, more informative feedback to users by making messages more concise and incorporating dynamic values directly into the error text. This ensures that users receive precise guidance when their input does not meet validation criteria. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request refactors the form validation error messages to be more concise and user-friendly. The changes are generally an improvement. However, an end-to-end test that asserts one of the old error messages was not updated, which will cause the test suite to fail. Please see the specific comment for details on how to fix the test.
|
Documentation. Coverage Reports: |
272432b to
9e984fd
Compare
68647fe to
346ede2
Compare
Danisand
left a comment
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.
Hi @spliffone
I reviewed the strings together with @michael-smt.
Please implement my suggestions and merge them.
0bf1387 to
842f2ee
Compare
|
Hi @spliffone and @michael-smt I have seen the picture for the visual regression test. It seems that the key Could you please explain me, in which situation |
|
@Danisand I found two more missing keys, feel free to have a look:
|
Yes, the error is provided when the app apply the Angular pattern directive (https://angular.dev/api/forms/PatternValidator#adding-a-pattern-validator) and you can assume to have two parameters: |
842f2ee to
8de3c59
Compare
Hi @spliffone I am not able to find these strings. Are they new? |
I see the main issue in the concatenation. In the tool tip are 2 strings concatenated. This must be avoided.
|
It is common practice to list all validation errors that is why we used sentences.
We aren't able to distinguish which error have a higher priority. |
Is the concatenation done in the inline validation text also? |
8de3c59 to
7c760c0
Compare
|
@spike-rabbit I think we should replace the current div's inside the |
7c760c0 to
38c0981
Compare
I don't think we should do that: Outside of this PR we could start evaluating:
|
The PR has enough changes I will create an issue where we should discuss it first. |
38c0981 to
d3236a4
Compare
4d5f33c to
577b5b3
Compare
577b5b3 to
8192c41
Compare


See #1373