Skip to content
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-34244] Some UX improvements in the New item form. #2324

Merged
merged 8 commits into from
May 20, 2016
6 changes: 3 additions & 3 deletions core/src/main/resources/hudson/model/View/newJob.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ THE SOFTWARE.
<div class="header">
<div class="add-item-name">
<label for="name">${%ItemName.label}</label>
<input name="name" id="name" type="text" tabindex="0" />
<input name="name" id="name" data-valid="false" type="text" tabindex="0" />
<div class="input-help">&#187; ${%ItemName.help}</div>
<div id="itemname-required" class="input-validation-message input-message-disabled">&#187; ${%ItemName.validation.required}</div>
<div id="itemname-invalid" class="input-validation-message input-message-disabled"></div>
<div id="itemtype-required" class="input-validation-message input-message-disabled">&#187; ${%ItemType.validation.required}</div>
</div>
</div>

<div class="categories flat" role="radiogroup" aria-labelledby="Items" />
<div id="items" class="categories flat" role="radiogroup" aria-labelledby="Items" data-valid="false" />

<j:if test="${!empty(app.itemMap)}">
<div class="item-copy">
Expand All @@ -59,7 +59,7 @@ THE SOFTWARE.
</div>
<label>${%CopyOption.label}</label>
<j:set var="descriptor" value="${it.descriptor}" />
<s:textbox id="from" name="from" placeholder="${%CopyOption.placeholder}" field="copyNewItemFrom"/>
<s:textbox id="from" data-valid="false" name="from" placeholder="${%CopyOption.placeholder}" field="copyNewItemFrom"/>
</div>
</div>
</j:if>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ NewJob=New {0}
JobName={0} name
ItemName.help=Required field
ItemName.label=Enter an item name
ItemName.validation.required=This field cannot be empty. Please enter a valid name and press OK button.
ItemName.validation.required=This field cannot be empty, please enter a valid name
ItemType.validation.required=Please select an item type
CopyOption.placeholder=Type to autocomplete
CopyOption.description=if you want to create a new item from other existing, you can use this option
Expand Down
11 changes: 9 additions & 2 deletions test/src/test/java/hudson/model/MyViewTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
package hudson.model;

import com.gargoylesoftware.htmlunit.html.HtmlElement;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import org.junit.Before;
import org.junit.Test;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
Expand Down Expand Up @@ -80,9 +82,14 @@ public void testContains() throws IOException, Exception{
public void testDoCreateItem() throws IOException, Exception{
MyView view = new MyView("My", rule.jenkins);
rule.jenkins.addView(view);
HtmlForm form = rule.createWebClient().goTo("view/" + view.getDisplayName() + "/newJob").getFormByName("createItem");
form.getInputsByValue("hudson.model.FreeStyleProject").get(0).setChecked(true);
HtmlPage newItemPage = rule.createWebClient().goTo("view/" + view.getDisplayName() + "/newJob");
HtmlForm form = newItemPage.getFormByName("createItem");
// Set the name of the item
form.getInputByName("name").setValueAttribute("job");
form.getInputByName("name").blur();
// Select the item clicking on the first item type shown
HtmlElement itemType = newItemPage.getFirstByXPath("//div[@class='category']/ul/li");
itemType.click();
rule.submit(form);
Item item = rule.jenkins.getItem("job");
assertTrue("View " + view.getDisplayName() + " should contain job " + item.getDisplayName(), view.getItems().contains(item));
Expand Down
145 changes: 113 additions & 32 deletions war/src/main/js/add-item.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// Initialize all modules by requiring them. Also makes sure they get bundled (see gulpfile.js).
var $ = require('jquery-detached').getJQuery();

var getItems = function(){
var getItems = function() {
var d = $.Deferred();
$.get('itemCategories?depth=3').done(
function(data){
d.resolve(data);
}
function(data){
d.resolve(data);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All whitespace modifications or do I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@batmat No, I took advantage this revision of New Item form in order to have the same coding style convention in this file.

);
return d.promise();
};
Expand Down Expand Up @@ -40,18 +40,6 @@ $.when(getItems()).done(function(data) {
return newDesc;
}

function getItemTypeSelected() {
var item = $('input[type="radio"][name="mode"]:checked', '#createItem').val();
if (item === "copy") {
return undefined;
}
return item;
}

function getItemCopyFromSelected() {
return $('input[type="radio"][name="mode"][value="copy"]:checked', '#createItem').val();
}

function getCopyFromValue() {
return $('input[type="text"][name="from"]', '#createItem').val();
}
Expand All @@ -61,6 +49,14 @@ $.when(getItems()).done(function(data) {
return (itemName === '') ? true : false;
}

function getFieldValidationStatus(fieldId) {
return $('#' + fieldId).data('valid');
}

function setFieldValidationStatus(fieldId, status) {
$('#' + fieldId).data('valid', status);
}

function activateValidationMessage(messageId, context, message) {
if (message !== undefined && message !== '') {
$(messageId, context).html('&#187; ' + message);
Expand All @@ -82,6 +78,65 @@ $.when(getItems()).done(function(data) {
$('.input-help', context).removeClass('input-message-disabled');
}

// About Scroll-linked effect: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Scroll-linked_effects
function doSticky() {
var decorator = $('form .footer .btn-decorator');
Copy link
Contributor

@kzantow kzantow May 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐜 It's a preference (and a commonly used convention) to start jQuery variables with $

var pos = decorator.offset();
var vpH = $(window).height();
if (pos.top >= vpH) {
decorator.css({position: 'fixed'});
}

$(window).scroll(function() {
var footer = $('form .footer');
var ref1 = decorator.offset().top + decorator.outerHeight();
var ref2 = footer.offset().top + footer.outerHeight();
var vpH = $(window).height();
if (ref2 > vpH + $(window).scrollTop()) {
decorator.css({position: 'fixed'});
} else if (ref2 - 1 <= ref1) {
decorator.css({position: 'absolute'});
}
});
}

function enableSubmit(status) {
var btn = $('form .footer .btn-decorator button[type=submit]');
if (status === true) {
if (btn.hasClass('disabled')) {
btn.removeClass('disabled');
btn.prop('disabled', false);
}
btn.focus();
} else {
if (!btn.hasClass('disabled')) {
btn.addClass('disabled');
btn.prop('disabled', true);
}
}
}

function getFormValidationStatus() {
if (getFieldValidationStatus('name') && (getFieldValidationStatus('items') || getFieldValidationStatus('from'))) {
return true;
}
return false;
}

function cleanItemSelection() {
$('.categories').find('li[role="radio"]').attr("aria-checked", "false");
$('#createItem').find('input[type="radio"][name="mode"]').removeAttr('checked');
$('.categories').find('.active').removeClass('active');
setFieldValidationStatus('items', false);
}

function cleanCopyFromOption() {
$('#createItem').find('input[type="radio"][value="copy"]').removeAttr('checked');
$('input[type="text"][name="from"]', '#createItem').val('');
setFieldValidationStatus('from', false);
}


//////////////////////////////////
// Draw functions

Expand Down Expand Up @@ -112,18 +167,21 @@ $.when(getItems()).done(function(data) {

function select(e) {
e.preventDefault();
$('li[role="radio"]').attr("aria-checked", "false");
$(this).find('input[type="radio"][name="mode"]').removeAttr('checked');
$(this).parents().find('.active').removeClass('active');
cleanCopyFromOption();
cleanItemSelection();

$(this).attr("aria-checked", "true");
$(this).find('input[type="radio"][name="mode"]').prop('checked', true);
$(this).addClass('active');

$('input[type="text"][name="from"]', '#createItem').val('');
cleanValidationMessages('.add-item-copy');
if (isItemNameEmpty()) {
setFieldValidationStatus('items', true);
if (!getFieldValidationStatus('name')) {
activateValidationMessage('#itemname-required', '.add-item-name');
$('input[name="name"][type="text"]', '#createItem').focus();
} else {
if (getFormValidationStatus()) {
enableSubmit(true);
}
}
}

Expand Down Expand Up @@ -186,9 +244,16 @@ $.when(getItems()).done(function(data) {
} else {
cleanValidationMessages('.add-item-name');
showInputHelp('.add-item-name');
setFieldValidationStatus('name', true);
if (getFormValidationStatus()) {
enableSubmit(true);
}
}
});
} else {
enableSubmit(false);
setFieldValidationStatus('name', false);
cleanValidationMessages('.add-item-name');
activateValidationMessage('#itemname-required', '.add-item-name');
}
});
Expand All @@ -198,26 +263,42 @@ $.when(getItems()).done(function(data) {
if (getCopyFromValue() === '') {
$('#createItem').find('input[type="radio"][value="copy"]').removeAttr('checked');
} else {
$('.categories').find('li[role="radio"]').attr("aria-checked", "false");
$('#createItem').find('input[type="radio"][name="mode"]').removeAttr('checked');
$('.categories').find('.active').removeClass('active');
cleanItemSelection();
$('#createItem').find('input[type="radio"][value="copy"]').prop('checked', true);
setFieldValidationStatus('from', true);
if (!getFieldValidationStatus('name')) {
activateValidationMessage('#itemname-required', '.add-item-name');
setTimeout(function() {
$('input[name="name"][type="text"]', '#createItem').focus();
}, 400);
} else {
if (getFormValidationStatus()) {
enableSubmit(true);
}
}
}
});

// Client-side validation
$("#createItem").submit(function(event) {
if (isItemNameEmpty()) {
activateValidationMessage('#itemname-required', '.add-item-name');
$('input[name="name"][type="text"]', '#createItem').focus();
if (!getFormValidationStatus()) {
event.preventDefault();
} else {
if (getItemTypeSelected() === undefined && getItemCopyFromSelected() === undefined) {
activateValidationMessage('#itemtype-required', '.add-item-name');
if (!getFieldValidationStatus('name')) {
activateValidationMessage('#itemname-required', '.add-item-name');
$('input[name="name"][type="text"]', '#createItem').focus();
event.preventDefault();
} else {
if (!getFieldValidationStatus('items') && !getFieldValidationStatus('from')) {
activateValidationMessage('#itemtype-required', '.add-item-name');
$('input[name="name"][type="text"]', '#createItem').focus();
}
}
}
});

// Disable the submit button
enableSubmit(false);

// Do sticky the form buttons
doSticky();
});
});
19 changes: 17 additions & 2 deletions war/src/main/js/widgets/add/addform.less
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@

.btn-decorator {
position: absolute;
bottom: -1px;
left: -1px;
bottom: 0px;
padding: 20px 30px;
width: auto;
border-top-right-radius: 10px;
border: 1px solid #aaccbb;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: I would prefer to add border-width: 1px 1px 0 0;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too but we are using the this design in job configuration. I can push another PR to change both.

background: rgba(245, 249, 239, 0.75);
text-align: center;
z-index: 10;

button[type=submit] {
padding: 5px 20px;
Expand All @@ -44,6 +44,21 @@
font-size: 12px;
line-height: 27px;
}

button[type=submit]:hover:not(.disabled), button[type=submit]:focus:not(.disabled) {
background-color: rgb(68, 119, 136);
border-color: rgb(51, 85, 102);
color: rgb(238, 238, 238);
cursor: pointer;
}

button[type=submit].disabled {
background-color: #ccc;
border-color: #aaa;
color: rgb(140, 140, 140);
cursor: auto;
opacity: 0.75;
}
}
}

Expand Down