senior-knights / course-schedulizer

📝 Create semester schedules without stress
https://senior-knights.github.io/course-schedulizer/
MIT License
10 stars 4 forks source link

Feature/teachingload item format #171

Closed kvlinden closed 3 years ago

kvlinden commented 3 years ago

This (simple, practice) PR reformats the section & non-teaching duty entries on the Teaching Loads page to include the hours for each item. To test, load the Teaching Loads tab and check that the correct hours are listed for all sections and/or non-teaching duties.

kvlinden commented 3 years ago

Sorry to miss that error. I've now parsed out the added load hours spec so that the popup works as it did before.

I'll have to say that concatenating and then parsing these strings is a bit clunky. Note, also, that the popup only works on the first course/duty on the list (per #143). I'd guess that supporting the popup for any course/duty on the list (#120) will require creating a span for each course/duty on the list rather than treating them as a single string.

Jonri2 commented 3 years ago

Note, also, that the popup only works on the first course/duty on the list (per #143).

You can access any of the sections through the paginator at the top of the popup. It's a bit weird that the first is always brought up at first no matter which you click on, but there is a way to access any of them. #143 suggests an edit button for each course which would fix the weirdness of the UI.

kvlinden commented 3 years ago

So, perhaps this wasn't such a "(simple, practice)" PR after all. Either this third time's a charm or charkour can take his turn at finding a bug. :-)

The regex now handles float or int load values, and only matches before commas or end-of-string. The user can still mess things up by putting a fake load spec with comma in a duty name. There may be a way to prevent this as well.

charkour commented 3 years ago

Thanks for the PR! I appreciate the code comments.

kvlinden commented 3 years ago

Ok, I added the unit tests and double-checked that the popups are still functional.