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.

Sign up for GitHub

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

Jump to bottom

[webui] jquery code review #1917

Closed
wants to merge 5 commits into from
Closed

[webui] jquery code review #1917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d02d4f6
[webui] jquery code review
avindra Jul 7, 2016
1eee6d1
[webui] cleanup logic in repo autocompletion
avindra Jul 7, 2016
bdf617b
[webui] patchinfo.js.erb: make code more legible
avindra Jul 7, 2016
004fa9f
[webui] patchinfo.js.erb: reduce nesting in append_bug()
avindra Jul 7, 2016
b0538fe
[webui] toggle_blockreason: simplify logic
avindra Jul 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/api/app/assets/javascripts/webui/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ function change_role(obj) {

$('#' + type + '_spinner').show();
$('#' + type + '_table input').animate({opacity: 0.2}, 500);
$('#' + type + '_table input').attr("disabled", "true");
$('#' + type + '_table input').prop('disabled', true);

$.ajax({
url: url,
Expand All @@ -207,7 +207,7 @@ function change_role(obj) {
complete: function () {
$('#' + type + '_spinner').hide();
$('#' + type + '_table input').animate({opacity: 1}, 200);
$('#' + type + '_table input').removeAttr('disabled');
$('#' + type + '_table input').prop('disabled', false);
Copy link
Member

Choose a reason for hiding this comment

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

this btw already fails in jquery 2.1

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean @coolo? that the prop method doesn't work in jquery 2.1? I have tried this at build.opensuse.org in the javascript console and it works...

Copy link
Member Author

@avindra avindra Jul 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@coolo : I switched the .removeAttr(prop) to .prop(prop, false) per jQuery's recommendation. Can you give an example of this failing?

It is almost always a mistake to use .removeAttr( "checked" ) on a DOM element.

Copy link
Member

Choose a reason for hiding this comment

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

I meant to say that removeAttr('disabled') already fails in jquery 2.1 - but build.oo uses jquery 1.11.

So 👍 for the change

Copy link
Member

Choose a reason for hiding this comment

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

I happend to learn the hard way :)

os-autoinst/openQA@ac7fe8b#diff-9fb9b9a3ce0b13423f7eb2ba327405b4

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah thanks for the clarification. Also, TIL Bootstrap 3.x is not compatible with jQuery 3.x (twbs/bootstrap#16834)

Copy link
Member

Choose a reason for hiding this comment

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

yeah, but till buildservice drops bento theme, bootstrap 19 is out ;)

}
});
}
Expand Down
30 changes: 15 additions & 15 deletions src/api/app/assets/javascripts/webui/application/bento/global-navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ $(function() {

if (!global_navigation_data) return;

var html = '';

$.each(global_navigation_data, function(i,menu){
html += '<ul class="global-navigation-menu" id="menu-' + menu.id + '">';
$.each(menu.items, function(j,submenu){
html += '<li><a href="' + submenu.link +'">';
html += '<span class="global-navigation-icon '+ submenu.image +'"></span>'; /*use imagemap and css */
html += '<span>' + submenu.title + '</span>';
html += '<span class="desc">' + submenu.desc + '</span>';
html += '</a></li>';
});
html += '</ul>';
});

$('#global-navigation').after(html);
// Build up navigation menus from localization data
// then render it.

$('#global-navigation').after(global_navigation_data.reduce(function(html, menu){
return html
+ '<ul class="global-navigation-menu" id="menu-' + menu.id + '">'
+ menu.items.reduce(function(h, item){
return h + '<li><a href="' + item.link +'">'
+ '<span class="global-navigation-icon '+ item.image +'"></span>' /*use imagemap and css */
+ '<span>' + item.title + '</span>'
+ '<span class="desc">' + item.desc + '</span>'
+ '</a></li>'
}, '')
+ '</ul>'
}, ''));

$('#global-navigation li[id^=item-]').click(function(){
var name = $(this).attr('id').substring(5);
Expand Down
115 changes: 62 additions & 53 deletions src/api/app/assets/javascripts/webui/application/patchinfo.js.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,53 +1,66 @@
function append_bug(issue) {
if (issue.length > 0) {
$("#issue_spinner").show();
var issues = new Array();
issue = issue.replace(/ /g, "");
issues = issue.split(",");
jQuery.unique(issues);
$.ajax({
url: "new_tracker",
type: "get",
dataType: "json",
data: {
issues: issues,
project: $("#project").val(),
package: $("#package").val()
},
success: function (data) {
if (data.error != "") {
alert(data.error);
}
;
jQuery.each(data.issues, function () {
if ($("div#issue_" + this[0] + "_" + this[1]).length > 0) {
return;
}
else {
$("#issuelist").append("<div id='issue_" + this[0] + "_" + this[1] + "'>" +
"<a id='remove_" + this[0] + "_" + this[1] + "' onclick='$(\"#issue_" + this[0] + "_" + this[1] + "\").remove(); return false;' href='#'> " +
"<img src='<%= asset_path('bug_delete.png') %>' title='Remove Bug' alt='Remove Bug'></a> " +
"<input type='hidden' name='issueid[]' id='issueid_" + this[0] + "_" + this[1] + "' value='" + this[1] + "'/>" +
"<input type='hidden' name='issuetracker[]' id='issuetracker_" + this[0] + "_" + this[1] + "' value='" + this[0] + "'/>" +
"<input type='hidden' name='issueurl[]' id='issueurl_" + this[0] + "_" + this[1] + "' value='" + this[2] + "'/>" +
"<input type='hidden' name='issuesum[]' id='issuesum_" + this[0] + "_" + this[1] + "' value='" + this[3] + "'/>" +
"<a href=\"" + this[2] + "\" target='_blank'>" + this[0] + "#" + this[1] + "</a>" + ":<br/>" +
"<div id='issue_desc_" + this[0] + "_" + this[1] + "' onclick='change_issue_desc(\"" + this[0] +
"_" + this[1] + "\");'>" + this[3] + "</div>" +
"<div id='change_desc_" + this[0] + "_" + this[1] + "' style='display:none;'>" +
"<input id='desc_" + this[0] + "_" + this[1] + "' type='text' value='" + this[3] + "' size=" + this[3].length +
" name='desc_" + this[0] + "_" + this[1] + "'/><a onclick='changeDesc(\"ok\", \"" + this[0] + "_" + this[1] + "\"); return false;' href='#'>" +
"<img title='Change description' src='<%=asset_path('ok.png')%>', alt='Ok'></a>" +
"<a onclick='changeDesc(\"cancel\", \"" + this[0] + "_" + this[1] + "\"); return false;' href='#'>" +
"<img title='Cancel changes' src='<%=asset_path('req-decline.png')%>', alt='Cancel'></a></div></div>");
if (issue.length === 0)
return false;

}
});
$("#issue").val("");
$("#issue_spinner").hide();
$("#issue_spinner").show();
var issues = issue.replace(/ /g, '').split(',');
jQuery.unique(issues);
$.ajax({
url: "new_tracker",
type: "get",
dataType: "json",
data: {
issues: issues,
project: $("#project").val(),
package: $("#package").val()
},
success: function (data) {
if (data.error != "") {
alert(data.error);
}
});
}

// transform arrays into objects with meaningful
// names
var issuesNotAdded = data.issues.map(function(data) {
var mappedData = {
tracker : data[0],
id : data[1],
url : data[2],
sum : data[3],
};
mappedData.uniqueID = mappedData.tracker + '_' + mappedData.id;
return mappedData;
}).filter(function(issue) { // only get issues that weren't added yet
return $("div#issue_" + issue.uniqueID).length == 0;
});

$("#issuelist").append(issuesNotAdded.map(function(issue) {
var tracker = issue.tracker;
var id = issue.id;
var url = issue.url;
var sum = issue.sum;
var uniqueID = issue.uniqueID;

return $("<div id='issue_" + uniqueID + "'>" +
"<a id='remove_" + uniqueID + "' onclick='$(\"#issue_" + uniqueID + "\").remove(); return false;' href='#'> " +
"<img src='<%= asset_path('bug_delete.png') %>' title='Remove Bug' alt='Remove Bug'></a> " +
"<input type='hidden' name='issueid[]' id='issueid_" + uniqueID + "' value='" + id + "'/>" +
"<input type='hidden' name='issuetracker[]' id='issuetracker_" + uniqueID + "' value='" + tracker + "'/>" +
"<input type='hidden' name='issueurl[]' id='issueurl_" + uniqueID + "' value='" + url + "'/>" +
"<input type='hidden' name='issuesum[]' id='issuesum_" + uniqueID + "' value='" + sum + "'/>" +
"<a href=\"" + url + "\" target='_blank'>" + tracker + "#" + id + "</a>" + ":<br/>" +
"<div id='issue_desc_" + uniqueID + "' onclick='change_issue_desc(\"" + uniqueID + "\");'>" + sum + "</div>" +
"<div id='change_desc_" + uniqueID + "' style='display:none;'>" +
"<input id='desc_" + uniqueID + "' type='text' value='" + sum + "' size=" + sum.length +
" name='desc_" + uniqueID + "'/><a onclick='changeDesc(\"ok\", \"" + uniqueID + "\"); return false;' href='#'>" +
"<img title='Change description' src='<%=asset_path('ok.png')%>', alt='Ok'></a>" +
"<a onclick='changeDesc(\"cancel\", \"" + uniqueID + "\"); return false;' href='#'>" +
"<img title='Cancel changes' src='<%=asset_path('req-decline.png')%>', alt='Cancel'></a></div></div>");
}));
$("#issue").val("");
$("#issue_spinner").hide();
}
});
return false;
}

Expand Down Expand Up @@ -99,17 +112,13 @@ function moveSelectedItems(source, destination) {
function stopRKey(evt) {
var evt = (evt) ? evt : ((event) ? event : null);
var node = (evt.target) ? evt.target : ((evt.srcElement) ? evt.srcElement : null);
if ((evt.keyCode == 13) && (node.type == "text")) {
if (evt.keyCode == 13 && node.type == "text") {
return false;
}
}

function toggle_blockreason(){
if (!$("#block").is(":checked")) {
$("#block_reason").prop('disabled', true);
} else {
$("#block_reason").prop('disabled', false);
}
$("#block_reason").prop('disabled', !$("#block").is(":checked"));
}

function patchinfoReady() {
Expand Down
29 changes: 15 additions & 14 deletions src/api/app/assets/javascripts/webui/application/project.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,26 +75,27 @@ function autocomplete_repositories(project_name) {
if (project_name === "")
return;
$('#loader-repo').show();
$('#add_repository_button').attr('disabled', 'true');
$('#target_repo').attr('disabled', 'true');
$('#repo_name').attr('disabled', 'true');
$('#add_repository_button').prop('disabled', true);
$('#target_repo').prop('disabled', true);
$('#repo_name').prop('disabled', true);
$.ajax({
url: $('#target_repo').data('ajaxurl'),
data: {project: project_name},
success: function (data) {
success: function (repos) {
$('#target_repo').html('');
// suggest a name:
$('#repo_name').attr('value', project_name.replace(/:/g, '_') + '_' + data[0]);
var foundoptions = false;
$.each(data, function (idx, val) {
$('#target_repo').append(new Option(val));
$('#target_repo').removeAttr('disabled');
$('#repo_name').removeAttr('disabled');
$('#add_repository_button').removeAttr('disabled');
foundoptions = true;
});
if (!foundoptions)
$('#repo_name').attr('value', project_name.replace(/:/g, '_') + '_' + repos[0]);
if(repos.length === 0) {
$('#target_repo').append(new Option('No repos found'));
return;
}

$('#target_repo').prop('disabled', false);
$('#repo_name').prop('disabled', false);
$('#add_repository_button').prop('disabled', false);
$('#target_repo').append(repos.map(function(repo) {
return new Option(repo);
}));
},
complete: function (data) {
$('#loader-repo').hide();
Expand Down