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

Automatically colour and (optionally) hide passed exams #1

Merged
merged 9 commits into from
Jun 12, 2016

Conversation

whisperity
Copy link
Contributor

I've added a few presentational features to NPU in regards to exams (what's better to do than learning right now 🍺 ).

Subjects from which you have a passing grade will now have their exams coloured green to easily distinguish them as you no longer NEED to do an exam. Subjects you have only received failing grades will have a red background.
To make exam registration an easier process, a new user option is also introduced: it is now possible to hide exams from subjects you have passed (these are the now green background ones). Selecting this option will also remove the subject from the "subject:" filter drop-down.

Subject grades are selected from the latest registered mark in accordance with usual University Rules. The same colouring scheme is applied to "my signed exams" page.

Tested to work with ELTE, BME and BCE Neptuns. I've tried my best to keep the coding style to yours. 😃

@solymosi
Copy link
Owner

Thanks for being the first contributor to NPU 😃
This looks very good -- I'll quickly test it on Neptun and merge it in if all is good.

@@ -1205,6 +1438,10 @@ var npu = {
dec = tmp_arr.join("");
return dec;
},

isPassingGrade: function(gradeStr) {
return gradeStr != "Elégtelen";
Copy link
Owner

Choose a reason for hiding this comment

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

If we are doing this, then let's also add Nem felelt meg and Fail for the 3-grade evaluation and the English UI, just to be comprehensive.

Copy link
Owner

Choose a reason for hiding this comment

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

Oh, and I almost forgot Nem jelent meg and Did not attend.

@solymosi
Copy link
Owner

Added a few comments, otherwise it looks good 👍

@whisperity
Copy link
Contributor Author

Neptun build 441 (2016. 03. 16.) apparently broke this.

@whisperity whisperity closed this May 8, 2016
@whisperity whisperity reopened this May 23, 2016
@whisperity
Copy link
Contributor Author

Well, actually, it was easier to fix than I thought. 💨

} else if (npu.getLanguage() == "en") {
return gradeStr != "Fail" && gradeStr != "Did not attend";
} else if (npu.getLanguage() == "de") {
// ???
Copy link
Owner

Choose a reason for hiding this comment

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

Apparently, the German version uses Hungarian grade names, so there is no need for the de branch.

@whisperity
Copy link
Contributor Author

Looks like there's a small oversight still in the script which will need further attention.

  1. List all subjects and use the exam hiding option
  2. It works as intended, completed subjects are removed from the filter listbox.
  3. Switch to a subject not yet completed.
  4. The exam list is filtered, but the listbox is yet again full.

Obviously it happens because the "Hide subjects" function works by taking the subject names for the completed rows and removing them from the listbox.


I'm thinking about locally caching the list of completed subjects once they appear in the user's browser. This has the drawback of bogusly ignoring a subject making it unfilterable in the future...

The other approach would be completely overriding Neptun's features and making the filter happen locally. So from Neptun all the exams are always queried, and from it, NPU filters using the listbox's current selection. This latter option - if implemented properly - has the upside of making it possible to hide subjects (from the listbox) which do not have any exam available (for example, practice courses).

}
}, 100);
}, 500);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neptun server response times on BME is a lot higher than on others like ELTE or Unideb. I'm talking about ~800ms versus 40-80 ones.

In the long run, the script might as well just be made sentient that it listens to the reponse times and saves it in its database and uses timeout intervals accordingly.

@whisperity
Copy link
Contributor Author

Is there anything else that should be implemented or is there a chance we go live with this? 😄

@solymosi
Copy link
Owner

Let me test your recent changes one last time. If everything works fine, I'll release it 👍

@solymosi solymosi merged commit 23e233e into solymosi:master Jun 12, 2016
@solymosi
Copy link
Owner

solymosi commented Jun 12, 2016

Unfortunately I've discovered a few consistency issues caused by the subject filtering override (+ extensive flickering when switching between terms), and wasn't able to find an easy fix for them. This means that the subject select box filtering will have to be removed for now 😢

I've prepared another PR (#3) which does that, and also removes the subrow expansion override, an additional simplification. May I ask you to test in your Neptun whether the rest of the features work as expected?

I feel bad about removing so much of your new code, as you clearly put a lot of effort in it. However, I would feel just as bad for not putting the highly useful exam list filtering and coloring features into the hands of our users as soon as possible. We can re-add the select box filtering feature once we find a solution for the inconsistencies.

@whisperity whisperity deleted the examhiding branch June 12, 2016 22:16
@whisperity whisperity mentioned this pull request Jun 14, 2016
whisperity added a commit to whisperity/npu that referenced this pull request Jun 15, 2016
…tbox

This approach should fix the flickering and abusive behaviour mentioned in solymosi#1 and solymosi#5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants