-
Notifications
You must be signed in to change notification settings - Fork 320
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 TA mode #3875
base: master
Are you sure you want to change the base?
Add TA mode #3875
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@leslieyip02 is attempting to deploy a commit to the modsbot's projects Team on Vercel. A member of the Team first needs to authorize it. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3875 +/- ##
==========================================
+ Coverage 54.52% 55.01% +0.49%
==========================================
Files 274 276 +2
Lines 6076 6247 +171
Branches 1455 1515 +60
==========================================
+ Hits 3313 3437 +124
- Misses 2763 2810 +47 ☔ View full report in Codecov by Sentry. |
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.
This looks quite good so far! Thanks for even considering timetable exports and adding some tests.
However, the ability to add multiple tutorials/lab/recitations slots to the timetable in TA mode is missing. This is useful for those TA-ing multiple slots. I suggest making each tutorial/lab/recitation slot toggleable in TA mode, instead of the usual behaviour that switches between slots.
Context
Closes #3804.
Adds an option for TAs to hide non-TA lessons. The original issue requested an option to hide non-tutorial lessons, but this would hide labs and recitations as well, so I decided to implement a dropdown menu to toggle TA mode for different lesson types.
I have not been a TA before, so do let me know if this makes sense?
Implementation
TimetableState
was updated to have aTaModuleConfig
, which maps each module to an array of lesson types.TimetableContent
only renders lessons which are marked as TA and ignores other lesson types.(TA)
in the timetableColorPicker
for TA modules has a half-filled color boxToggle TA mode
Timetable.-.NUSMods.Mozilla.Firefox.2024-12-01.16-56-42.mp4
No TA-able mods
When the mod has no TA-able mods (i.e. just lectures), the menu button is disabled.
Export/Import
Mozilla.Firefox.Private.Browsing.2024-12-01.17-39-17.mp4
Exam clash
TA mods are not shown in the exam calendar, and do not contribute to exam clashes.
Left: Updated, Right: Current
I also made it so that hidden mods no longer creates a clash warning.
Other Information
I had some trouble trying to get
Tooltip
to work withDownshift
. When usingTooltip
insideDownShift
, I needed to callgetRootProps
, but it seems to be creating these warnings fromtippy.js
:Since the versions of both
@tippy.js/react
anddownshift
are quite outdated, I wasn't able to find docs to resolve the warnings.