-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
1-1334: show short error message when validation fails #4617
1-1334: show short error message when validation fails #4617
Conversation
The error message only tells the user that the name doesn't match the pattern. Because we already show the pattern above the input, we don't need to repeat it in the error message. This makes for a shorter and more concise message and better UX. At the same time, for API users, we can keep the more detailed message that includes info about the pattern, the example, and the description.
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
@@ -137,6 +137,7 @@ const CreateFeature = () => { | |||
handleCancel={handleCancel} | |||
mode="Create" | |||
clearErrors={clearErrors} | |||
featureNaming={projectInfo.featureNaming} |
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.
Will this always exists? According to the type in featureForm it can be undefined.
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.
Yes. So we send it down. If we have it, all good, otherwise featureForm handles that too. Am I missing something that makes this not work in some cases?
<dd> | ||
<code>{featureNaming?.pattern}</code> | ||
</dd> | ||
{Boolean(featureNaming?.example) && ( |
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.
Could use conditional render component for readability
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.
Yeah, I thought about that, but decided against it because of the layers of nesting. Felt like it would take up a lot of extra vertical space for not a lot of extra value. However, I do see your point and would be happy to switch to the component instead if you think that's better
<dd>{featureNaming?.example}</dd> | ||
</> | ||
)} | ||
{Boolean(featureNaming?.description) && ( |
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.
Could also be used here
The error message only tells the user that the name doesn't match the
pattern. Because we already show the pattern above the input, we don't
need to repeat it in the error message. This makes for a shorter and
more concise message and better UX.
At the same time, for API users, we can keep the more detailed message
that includes info about the pattern, the example, and the description.