autolab / Autolab

Course management service that enables auto-graded programming assignments.
http://www.autolabproject.com/
Apache License 2.0
752 stars 214 forks source link

Gradesheet graders should only change if the feedback/score changes. #446

Closed dlbucci closed 8 years ago

dlbucci commented 9 years ago

Currently if a grader clicks a score, does nothing and clicks away, the score will get saved with that user being the grader, which is incorrect.

zyx-billy commented 8 years ago

(Thought I'd fix the gradesheet-related issues while I'm working on the page...)

Here's a related issue that could be solved at the same time: when the feedback popover is on, clicking away also saves the feedback, which makes the "Save" button useless, and also confusing (people might make a mistake, and hope clicking away will cancel the change).

One solution for both problems could be to add a "cancel" button to the popover, and disable the close-popover-by-clicking-somewhere-else feature. OR how about we just redo the entire feedback editing box (since it's really small & inconvenient when the feedback's long: see #570), like having a large editing box anchored to the side of the table.

ymzong commented 8 years ago

@zyx-billy I think modifying the size and buttons on the feedback box suffices as our short-term solution, and we can see how well that goes before doing major changes like pulling feedback box to the side.

zyx-billy commented 8 years ago

Sure. I'll implement the first solution and see how that goes.

zyx-billy commented 8 years ago

The popover looks like this with the cancel button (using default Autolab btn): feedback_cancel_button

I hope this can make the consequences explicit, so nothing is saved (or discarded) unintentionally. However, clicking away is ignored, and it feels a bit weird. Perhaps we should add some visual cues?

nayak16 commented 8 years ago

I agree that it is weird. Why can't we close the feedback box after clicking away? Basically having the same action as pressing the Cancel button.

zyx-billy commented 8 years ago

@nayak16 So I was worried someone might accidentally click somewhere else and lose everything they wrote since the box is kinda small...

nayak16 commented 8 years ago

@zyx-billy Is this close to being resolved? We want to get all the gradesheet changes in before deploying to production.

zyx-billy commented 8 years ago

@nayak16 I can get it done by today if needed, there's not too much to change. Are we still targeting Apr12 or is it gonna be earlier?

ymzong commented 8 years ago

@zyx-billy The final pull day for Computing Services is Apr 12th, and I'd say we should aim for code freeze by April 7th (our next meeting).

zyx-billy commented 8 years ago

OK, so I'll make sure it's ready for demo on this week's meeting.

ymzong commented 8 years ago

@zyx-billy :+1:, and don't forget to base it on master!

zyx-billy commented 8 years ago

@ymzong The previous pr changed the gradesheet's JS file a lot, and I will continue modifying the same file for this issue. I think it'll be easier to have this based on the latest pr to prevent dealing with merge conflicts?

ymzong commented 8 years ago

@zyx-billy Sure, go ahead!

zyx-billy commented 8 years ago

fixed with #651