[JENKINS-36084] Responsive new job name validation #2416
Conversation
|
@tfennelly In #2501 I propose to move input box so that autocomplete will not hide warning message. What do you think? |
|
How it looks like now? |
|
@tfennelly WDYT? Can't help it, this looks weird… |
|
@daniel-beck @lanwen @michaelneale I removed the shift and added a red border. What do you think? |
|
red border looks much better |
|
The suggestion to indicate an invalid title through styling of the text field came up in today's meeting about Samat's GSoC project. I like it as well, but wonder whether a background color could make it even more noticeable without distracting so much (you want people to notice, after all!) That said, even if it's just the border, I'm now 👍 (functionality-wise) on this. Further refinements can be done later, and Samat got the live validation done. WHOEVER MERGES THIS BETTER SQUASH IT! |
| } else { | ||
| cleanValidationMessages('.add-item-name'); | ||
| $('#add-item-panel input').css('border', "1px solid #999"); |
There was a problem hiding this comment.
Can't this use CSS classes that already exist?
There was a problem hiding this comment.
| if (!isItemNameEmpty()) { | ||
| var itemName = $('input[name="name"]', '#createItem').val(); | ||
| $.get("checkJobName", { value: itemName }).done(function(data) { | ||
| var message = parseResponseFromCheckJobName(data); | ||
| if (message !== '') { | ||
| activateValidationMessage('#itemname-invalid', '.add-item-name', message); | ||
| $('input[name="name"]').css({'border': "1px solid red", "background-color": "#FADEDE"}); |
There was a problem hiding this comment.
don't add exact css, toggle class
|
@lanwen I added class toggle. Is it fine now? |
| @@ -14,6 +14,11 @@ | |||
| z-index: 5; | |||
| } | |||
|
|
|||
| .warning-input { | |||
There was a problem hiding this comment.
i'd suggest .job-name_incorrect
|
@lanwen I renamed class. Is it fine now? |
|
@michaelneale can you restart the build? It seems like a remote failure. |
|
LGTM |
|
When clearing the input field, it retains the previous validation status. Is that deliberate? |
|
@daniel-beck what do you mean by retains? If a recall correctly the behavior is the same as with blur event. |
|
@samatdav he means that if you delete the contents, it still shows an error around it (which is how I interpret what he said, but I don't think my explanation is any clearer). |
|
@daniel-beck @michaelneale oh, I see - do you mean the red border still shows up after you delete the job name which is invalid? I am actually not sure what happens because the code for replacing warning message and removing warning class name is in one place: https://github.com/jenkinsci/jenkins/pull/2416/files#diff-e146d4fdbfe6385a456aea9775f6282dR253 |
daniel-beck
added
the
unresolved-merge-conflict
|
@michaelneale @samatdav I'd guess there is no interest to proceed with this change, right? |
batmat
added
the
proposed-for-close
|
Based on latest review comments, there's also an unresolved issue when clearing the input field. Closing for now, can always be reopened or resubmitted later. |
Currently the GET request to check new job name for invalid characters is sent only on 'blur' jQuery event.
That makes a user know about the invalid character only after he entered the name and forces to go back to input field.
This change checks for invalid characters in new job name after each character (keyup event).
Additionally, it removes unnecessary warning messages when job name is empty.