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
Story/cit 6 #14
Story/cit 6 #14
Conversation
Conflicts: citesphere/src/main/webapp/WEB-INF/views/auth/group/editItem.jsp
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
authorSpan.html(""); | ||
var affiliationsList = []; | ||
var aff = $("#affiliations").children(); | ||
for(var i=1;i<aff.length;i++){ |
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.
let's use 'each' here instead of a for loop. Easier to read and maintain.
} | ||
|
||
var affiliationString = ""; | ||
if (affiliationsList) { |
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.
there is actually a bug in this code that shows empty parentheses if there are no affiliations (affiliationsList needs to be checked if it's empty). This needs to be fixed here and above.
$("#authorList").append(" ") | ||
$("#authorModal").modal('hide'); | ||
resetAuthorCreationModal(); | ||
}); |
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.
since this is basically the same code as for adding authors, let's reuse the existing code.
$("#uriAuthor").val(authorItem.attr("data-author-uri")); | ||
$("#idAuthor").val(authorItem.attr("data-author-id")); | ||
|
||
for(var i=0;i<authorItem.children().length-1;i++){ |
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.
let's use 'each' here instead of for loop
if(authorItem.children().length-1>0){ | ||
$("#affiliationTemplate").hide(); | ||
} | ||
if($("#updateAuthorButton").length == 0){ |
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.
when would that be the case? I think I know what you're doing but I'm pretty sure that is a more elegant way. Why don't you just reuse the existing code and just change the label of the button from 'Add Author' to 'Update Author' when a user clicks on the edit button?
@@ -140,6 +180,31 @@ $(function() { | |||
}, 1000); | |||
}); | |||
|
|||
$(".author-item").on("click", function(){ |
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.
let's add an 'edit' icon to edit an author instead of clicking on the whole thing.
@@ -163,9 +228,13 @@ $(function() { | |||
function resetAuthorCreationModal() { | |||
$("#firstNameAuthor").val(""); | |||
$("#lastNameAuthor").val(""); | |||
$("#affiliationTemplate").find("input").val(""); | |||
$("#affiliations").html("<div id='affiliationTemplate' class='form-group'><label for='affiliationAuthor'>Affiliation:</label><input type='text' class='form-control' placeholder='Affiliation'></div>"); |
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.
I assume you're getting rid of the clones of affiliation inputs? Just give them all a class and then remove them by their class name.
Conflicts: citesphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/CitationManager.java
Updated for loop to each and reused existing code for update author. |
authorItem.children("span").each(function(idx, elem){ | ||
var affInput = $("#affiliationTemplate").clone(); | ||
affInput.removeAttr("id"); | ||
affInput.attr("class", affInput.attr("class")+" aff-info"); |
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.
just use 'addClass' function
var affSpan = $("<span>"); | ||
affSpan.attr("data-affiliation-id", input.attr("data-affiliation-id")); | ||
affSpan.attr("data-affiliation-name", input.val()); | ||
affSpan.val(input.val()); |
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.
isn't affSpan a span element? what does calling val() accomplish? form jquery documentation: "This method is typically used to set the values of form fields."
function resetAuthorCreationModal() { | ||
$("#firstNameAuthor").val(""); | ||
$("#lastNameAuthor").val(""); | ||
$("#affiliationTemplate").find("input").val(""); | ||
$("#uriAuthor").val(""); | ||
$("#idAuthor").attr("data-author-id",""); | ||
$(".aff-info").find("input").val(""); | ||
$(".aff-info").remove(); |
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.
why do you set the input to empty first, if you then remove the the elements?
Refactored code. |
Jenkins, test this please. |
Jenkins, test this please. |
"Created new authority entry XXX." is not reset when the modal is opened again. |
I think my changes in CIT-5 would fix the reset. |
please resolve conflicts |
Conflicts: citesphere/src/main/webapp/WEB-INF/views/auth/group/editItem.jsp
Resolved conflicts and generalized edit authors and editors. |
retest this please |
The following exception is thrown now: |
[CIT-6] Fixed exception in citation factory to handle json null. |
Changes to edit authors.