-
-
Notifications
You must be signed in to change notification settings - Fork 9k
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
[JENKINS-68952] Wrong location of email form validation on setup wizard #6966
Conversation
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.
Is there a specific reason why you dropped the exclamation mark next to the form validation?
Also, the validation looks weird, because the warnings aren't update if the input has changed.
I found that signup page do not use the exclamation mark, in my humble opinion, i think it's sufficient enough to use red for error message and red border for input field Signup page
demo.mp4In my test, I need to click the "store and continue" button so that the warnings will update |
The new version looks fairly similar to how the normal Jenkins UI does field validation, while at the same time being quite different (above the field vs. below, same line as the label vs. on a separate line, no icon vs. icon, not updating live). As a user I wonder why these (relatively minor) differences. The existing validation on the other hand is pretty clearly intentionally different. I might still wonder why, but at least it's so massively different that there's less surprise about the differences in behavior. Additionally, combining error messages and form field labels seems to make it too easy for users to miss the messages. (Who has time to read more than one word of each label? It's just going to explain what a user name is or something equally irrelevant.) The lack of icons and other highlighting (bold text or similar) except for a slight text color change makes this worse. I am confused by the Jira issue description as well. Is it wrong to have a single block of all validation errors on top of the form, as opposed to a design choice? If so, could you provide references to UI design guidelines saying so? Because as the screenshots here show, it's not like the email validation field is an outlier. It behaves consistently with the other field validations. |
I agree.
It doesn't comply with the format form validation is supposed to be used. See https://weekly.ci.jenkins.io/design-library/Validation/ how it should be done, the validation is attached to the input it belongs to, not somewhere else on the page, which is what the issue is about. |
Ah, good point. That makes sense. Still, the differences with how form validation otherwise works are awkward. |
demo.mp4I have moved the error message below the input field, but i haven't make it updating live, if neccessary, I'll do it in my spare time, thanks. |
That looks much more promising, thanks!
I wouldn't consider that as blocker for this PR, but would be nice to have if you are interested in a follow up PR to this one. |
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.
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks!
See JENKINS-68952.

Before
After

Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgrade@Restricted
or have@since TODO
Javadoc, as appropriate.@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
if applicable.eval
to ease future introduction of Content-Security-Policy directives (see documentation on jenkins.io).Desired reviewers
@mention
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are accurate, human-readable, and in the imperative moodupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).