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

Add filename, line number, better collapsibility to annotations #1617

Merged
merged 42 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
ed3800a
Add line number
damianhxy Sep 29, 2022
4654ffa
Move active class
damianhxy Sep 29, 2022
e37b22b
Add lineNumber when updating
damianhxy Sep 29, 2022
64ce5f6
Change score field to type number
damianhxy Sep 29, 2022
b0e2b3f
Consistently display annotation scores to 1 d.p.
damianhxy Sep 29, 2022
4d5fb75
Fix plus_fix
damianhxy Sep 29, 2022
6fdcc3f
Sort annotations after updates
damianhxy Sep 29, 2022
8cb7acc
Add sorting logic
damianhxy Sep 29, 2022
027c446
Match get_correct_filename behavior
damianhxy Sep 29, 2022
083576d
Round grades to 1dp
damianhxy Oct 4, 2022
1e215a8
Merge branch 'master' into generalized-feedback
damianhxy Oct 5, 2022
a1e55d4
Fix indentation
damianhxy Oct 5, 2022
e812aec
Disable accordion (to allow multiple sections open at once)
damianhxy Oct 5, 2022
5eabd15
annotation.js cleanup
damianhxy Oct 5, 2022
0242217
Remove max-height on collapsible
damianhxy Oct 5, 2022
69c0556
Run attachChangeFileEvents once at the end
damianhxy Oct 5, 2022
55ca96b
Group annotations by filename
damianhxy Oct 5, 2022
ff9e8ab
Implement group_by for js
damianhxy Oct 6, 2022
b7220f5
Add collapsible arrows
damianhxy Oct 6, 2022
ce66d94
Add collapsible arrows (after update)
damianhxy Oct 6, 2022
098a9e2
Increase arrow size
damianhxy Oct 7, 2022
df52f66
Disable user-select on header
damianhxy Oct 7, 2022
9fb0d91
Add spacing between annotations of different files
damianhxy Oct 7, 2022
adabb78
Remove debug statement
damianhxy Oct 7, 2022
6d8fb16
Use jquery load for annotations
damianhxy Oct 7, 2022
15e1a6a
Remove dead code
damianhxy Oct 7, 2022
b244566
Revert filelist from instance to local variable
damianhxy Oct 7, 2022
9f89967
Merge branch 'master' into generalized-feedback
damianhxy Oct 7, 2022
6be26bc
Fixed annotation switching problems not updating score (#1621)
najclark Oct 11, 2022
31fa344
Force flatpicker interface for date/datetime picker on mobile (#1620)
damianhxy Oct 11, 2022
441b5bb
Validate against empty annotation scores
damianhxy Oct 13, 2022
4871185
Fix annotation score validation on backend
damianhxy Oct 13, 2022
6091cb0
Simplify badge color update logic
damianhxy Oct 13, 2022
9b83c56
Prevent blank annotation scores on update
damianhxy Oct 13, 2022
8c45c45
Simplify badge color update logic (fix)
damianhxy Oct 13, 2022
bc06be6
Show revised due date for manage extensions (#1623)
michellexliu Oct 19, 2022
8c3ef3b
Bump nokogiri from 1.13.6 to 1.13.9 (#1625)
dependabot[bot] Oct 21, 2022
833db77
make submission summary fill up space (#1624)
20wildmanj Oct 23, 2022
5654639
Merge branch 'generalized-feedback' of https://github.com/autolab/Aut…
michellexliu Oct 23, 2022
31721a7
round 2 decimal places
michellexliu Oct 23, 2022
e5721eb
plusFix round to 2 decimal places
michellexliu Oct 27, 2022
c64157f
Change grades panel to use 2dp
damianhxy Oct 27, 2022
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
8 changes: 4 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,14 @@ GEM
net-ldap (0.17.1)
newrelic_rpm (8.8.0)
nio4r (2.5.8)
nokogiri (1.13.6)
nokogiri (1.13.9)
mini_portile2 (~> 2.8.0)
racc (~> 1.4)
nokogiri (1.13.6-arm64-darwin)
nokogiri (1.13.9-arm64-darwin)
racc (~> 1.4)
nokogiri (1.13.6-x86_64-darwin)
nokogiri (1.13.9-x86_64-darwin)
racc (~> 1.4)
nokogiri (1.13.6-x86_64-linux)
nokogiri (1.13.9-x86_64-linux)
racc (~> 1.4)
oauth2 (1.4.10)
faraday (>= 0.17.3, < 3.0)
Expand Down
114 changes: 21 additions & 93 deletions app/assets/javascripts/annotations.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

$(document).ready(function () {
$('.skip-main').remove(); // removes skip main anchor tag
$(".collapsible-body").show(); //expands all collapsible initially
$('.collapsible').collapsible();
$('.collapsible').collapsible({ accordion: false });
//get line number in URL, if it exists
var urlParams = new URLSearchParams(location.search);

Expand Down Expand Up @@ -138,93 +137,12 @@ function plusFix(n) {

// function called after create, update & delete of annotations
function fillAnnotationBox() {

retrieveSharedComments();

var annotationsByProblem = {}
$(".collapsible.expandable").find('li').remove();
for (var i = 0; i < annotations.length; i++) {
var problem = getProblemNameWithId(annotations[i].problem_id)
if (!annotationsByProblem[problem]) {
annotationsByProblem[problem] = []
}
annotations[i].problem = problem;
annotationsByProblem[problem].push(annotations[i]);
}

for (var problem in annotationsByProblem) {
var problemElement = $("#li-problem-" + problem);
var score = 0;
for (var i = 0; i < annotationsByProblem[problem].length; i++) {
var annotation = annotationsByProblem[problem][i];
var points = parseFloat(annotation.value);
if (isNaN(points)) points = 0;
score += points;
}

var annotationsSummary = $(".annotationSummary");

if (problemElement) {
problemElement.remove();
}

var newLi = $("<li />");
newLi.addClass('active');
annotationsSummary.find("ul").append(newLi)
newLi.attr("id", "li-problem-" + problem);
var collapsible = $('<div />');
collapsible.addClass('collapsible-header');
collapsible.addClass('active');
collapsible.append(
'<h4 style="text-transform:capitalize;">' + problem + '<div class="summary_score">' + plusFix(score) + '</div>' +
'</h4>'
)
newLi.append(collapsible);

var listing = $('<div />');
newLi.append(listing);
listing.addClass("collapsible-body");
listing.addClass("active");
listing.css('display', 'block');

// sorts the annotation by line order
annotationsByProblem[problem].sort(function (annotation1, annotation2) { return annotation1.line - annotation2.line });
for (var i = 0; i < annotationsByProblem[problem].length; i++) {
var annotation = annotationsByProblem[problem][i];

var annotationElement = $('<div />');
annotationElement.addClass('descript');
annotationElement.attr('id', 'li-annotation-' + annotation.id);

var pos = annotation.position ? annotation.position : 0;
var line = annotation.line + 1;
var link = $('<a />');
link.addClass('descript-link');
link.attr('data-header_position', pos);
link.attr('data-line', line);
link.attr('data-remote', true);
link.attr('href', `./view?header_position=${pos}&line=${line}`);

var pointBadge = $('<span />');
pointBadge.addClass('point_badge');
if (annotation.value > 0) {
pointBadge.addClass('positive');
} else if (annotation.value < 0) {
pointBadge.addClass('negative');
} else {
pointBadge.addClass('neutral');
}
pointBadge.text(plusFix(annotation.value));
link.append(pointBadge);
link.append(annotation.comment);
annotationElement.append(link);
listing.append(annotationElement);

attachChangeFileEvents();
}
}
// Reloads the grades part upon update
$('.problemGrades').load(document.URL + ' .problemGrades');
$('#annotationPane').load(document.URL + ' #annotationPane', function() {
$('.collapsible').collapsible({ accordion: false });
attachChangeFileEvents();
});
}

// Sets up the keybindings
Expand Down Expand Up @@ -534,11 +452,16 @@ function newAnnotationFormCode() {
var problem_id = $(this).find(".problem-id").val();
var line = $(this).parent().parent().data("lineId");

if (comment == undefined || comment == "") {
if (comment === undefined || comment === "") {
box.find('.error').text("Annotation comment can not be blank!").show();
return;
}

if (score === undefined || score === "") {
michellexliu marked this conversation as resolved.
Show resolved Hide resolved
box.find('.error').text("Annotation score can not be blank!").show();
return;
}

if (problem_id == undefined) {
if ($('.select').children('option').length > 0)
box.find('.error').text("Problem not selected").show();
Expand Down Expand Up @@ -587,15 +510,20 @@ function initializeBoxForm(box, annotation) {
box.find('.annotation-form').submit(function (e) {
e.preventDefault();
var comment = $(this).find(".comment").val();
var shared_comment = $(this).find("#shared-comment").is(":checked");
var score = $(this).find(".score").val();
var problem_id = $(this).find(".problem-id").val();
var shared_comment = $(this).find("#shared-comment").is(":checked");

if (comment == undefined || comment == "") {
if (comment === undefined || comment === "") {
box.find('.error').text("Annotation comment can not be blank!").show();
return;
}

if (score === undefined || score === "") {
box.find('.error').text("Annotation score can not be blank!").show();
return;
}

var annotationObject = getAnnotationObject(box.data('annotationId'));
annotationObject.comment = comment;
annotationObject.value = score;
Expand All @@ -619,9 +547,9 @@ function newAnnotationBox(annotation) {
var shared_comment = annotation.shared_comment;

if (annotation.value < 0) {
box.find('.value').parent().removeClass('positive').addClass('negative');
} else if (!annotation.value > 0) { // I am a little hesitant about using == 1 here -> what if it's negative 0?
box.find('.value').parent().removeClass('positive').addClass('neutral');
box.find('.value').parent().removeClass('neutral').addClass('negative');
} else if (annotation.value > 0) {
box.find('.value').parent().removeClass('neutral').addClass('positive');
}

box.find('.submitted_by').text(annotation.submitted_by);
Expand Down
1 change: 1 addition & 0 deletions app/assets/javascripts/init_handin_datetimepickers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ $(document).ready(function() {
var defaults = {
enableTime: true,
altInput: true,
disableMobile: true,
defaultDate: moment(elt.val(), elt.data("date-format")).toDate(),
parseDate: (datestr, format) => {
return moment(datestr, format, true).toDate();
Expand Down
2 changes: 2 additions & 0 deletions app/assets/javascripts/initialize_datetimepickers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
$(datetimeElts[i]).flatpickr({
enableTime: true,
altInput: true,
disableMobile: true,
defaultDate: defaultDate,
formatDate: (date, format) => {
return moment(date).format(format);
Expand All @@ -34,6 +35,7 @@
$(dateElts[i]).flatpickr({
altInput: true,
defaultDate: defaultDate,
disableMobile: true
})
}
});
Expand Down
41 changes: 33 additions & 8 deletions app/assets/stylesheets/annotations.css
Original file line number Diff line number Diff line change
Expand Up @@ -538,22 +538,42 @@
border: none;
padding: 5px;
padding-left: 10px;
user-select: none;
}

.annotationSummary .collapsible-header h4{
margin: 3px;
font-size: 1.1em;
}

.annotationSummary .collapsible-header .dropdown-arrow{

}

.annotationSummary .collapsible-body{
padding: 0 1rem;
border: none;
}

.annotationSummary .collapsible-body .file_annotations {
margin-bottom: 10px;
}

.annotationSummary .collapsible {
overflow: auto;
}

/* Annotations collapsible icons */

.annotationSummary .collapsible i.expand-icon,
.annotationSummary .collapsible i.collapse-icon {
font-size: 2rem;
}

.annotationSummary .collapsible li.active i.expand-icon {
display: none;
}

.annotationSummary .collapsible li:not(.active) i.collapse-icon {
display: none;
}

.annotationSummary .summary_score {
float: right;
font-weight: normal;
Expand All @@ -575,6 +595,15 @@
background: #818181;
}

.annotationSummary .line_number {
background: #818181;
margin-left: -8px;
border-top-left-radius: 100vw;
padding-left: 8px;
padding-right: 3px;
border-bottom-left-radius: 100vw;
}

.annotation-comment {
margin-bottom:0;
padding-bottom:0 !important;
Expand Down Expand Up @@ -788,10 +817,6 @@
opacity: 0;
}
}
.collapsible {
max-height: 50vh;
overflow: auto;
}

.descript {
cursor: pointer;
Expand Down
30 changes: 27 additions & 3 deletions app/assets/stylesheets/style.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -1347,10 +1347,34 @@ a.skip-main:active {
font-size: 1em;
z-index: 999;
}

// used in submission_history_table.html.erb
table.sub {
display: block;
overflow-x: scroll;
display: table;
}
.sub th{

.sub th {
vertical-align: top;
}

.submissions {
overflow-x: scroll;
}

.submission-history-button {
white-space: nowrap;
text-transform: unset !important;
font-size: 13px;
width: 100%;
border-radius: 5px;
color: #0882af;
box-shadow: 0px -0.5px 0.5px 0px rgba(0, 0, 0, 0.15),
0px 0.5px 0.5px 0.5px rgba(0, 0, 0, 0.15),
0px 0.5px 0.5px -0.5px rgba(0, 0, 0, 0.15);
display: flex;
justify-content: center;
}

.submission-history-button:hover {
color: white !important;
}
4 changes: 4 additions & 0 deletions app/controllers/annotations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def update
tweaked_params.delete(:submission_id)
tweaked_params.delete(:filename)
ActiveRecord::Base.transaction do
# Remove effect of annotation to handle updating annotation problem
@annotation.update({ "value" => "0" })
@annotation.update_non_autograded_score

@annotation.update(tweaked_params)
@annotation.update_non_autograded_score
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/submissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -455,9 +455,9 @@ def view
@annotations = []
end

if Archive.archive? @filename
files = Archive.get_files(@filename)
end
files = if Archive.archive? @filename
Archive.get_files(@filename)
end
# extract information from annotations
@annotations.each do |annotation|
description = annotation.comment
Expand Down
6 changes: 5 additions & 1 deletion app/helpers/submission_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
module SubmissionHelper
def plus_fix(f)
f > 0 ? "+#{f}" : "#{f}"
if f > 0
sprintf("+%.2f", f.round(2))
else
sprintf("%.2f", f.round(2))
end
end
end
2 changes: 1 addition & 1 deletion app/models/annotation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Annotation < ApplicationRecord
belongs_to :submission
belongs_to :problem

validates :comment, :filename, :submission_id, :problem_id, presence: true
validates :comment, :value, :filename, :submission_id, :problem_id, presence: true

def as_text
if value
Expand Down
Loading