-
Notifications
You must be signed in to change notification settings - Fork 10
Add calendar to mentor attendance view #475
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
Conversation
csm_web
|
Project |
csm_web
|
Branch Review |
master
|
Run status |
|
Run duration | 02m 06s |
Commit |
|
Committer | Alec Li |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
84
|
View all changes introduced in this branch ↗︎ |
f62efe4
to
44ce955
Compare
955843e
to
29ec88a
Compare
29ec88a
to
23b087c
Compare
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.
Code looks okay, but a couple things:
- There is a weird bug when I "Mark All As Present" and 'Save' after I click on the section attendance for the date the attendance modal auto opens to. I couldn't find consistent behavior, but sometimes the presences will all turn blank and the calendar won't update. Sometimes the presences will stay as present but the calendar won't update. If I click onto another date then click back onto the original date, then it seems like everything is fine.
- You could add an icon to the calendar for previous and sections on the current day where there is a nonzero number of attendances not selected to warn mentors.
- The 'Save' button could have the same behavior as the 'Update' button, where if the saved results aren't different than the backend, the 'Save' button is gray and unclickable -- otherwise, it is green.
- The text portion of the attendance modal on the left side could have a fixed size. There is a scroll bar for dates already, but maybe a scroll bar for students too would be nice. This is more a problem for really big sections, but I notice that the box is already too big for the page with five students.
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.
as mentioned during the meeting, there is currently no way for mentors to reset attendance back to blank once they mark it as saved. i think that this could be nice to roll out in the "attendance" update, but its not super necessary if its too much work.
beyond that, i experienced the same issue that edward mentioned above with the calendar not properly updating when i save the attendance (i only noticed this only occurs for unexcused/excused absences).
Latest changes should address most of the comments above.
|
Regarding a fixed height table, I've tried a few things, but the scrollbar is hard to position (there's always a padding on the right, and the border radius also makes it hard to shift the element), and there's some other small things visually that I'm not too satisfied with. It'd probably require a little bit more tweaking (and perhaps similar scrollbars on other pages that have a lot of students, like the roster). Those enhancements for larger sections can probably be put into their own separate PR, since we still need to make optimizations regarding large sections (see #408 as an example). For now though, all of the sections we have right now shouldn't have any issues. The tabs sidebar should still look a little bit better now with the latest changes though, since I added back the border on the last tab when active (not sure why it was disabled before). |
…udent wotd date selection
ef8bcd9
to
10e4e0e
Compare
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.
excellent work alec
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.
- Mentors should now also be able to mark attendances as empty
I wasn't actually able to do this, but I also think this is a minor bug that doesn't impact functionality thaaaat much. I noticed that the drop down does seem to have a 4th, empty option, but it wasn't selectable.
despite that, approving! Alec is my goat fr :)
Dismissing since we have two other approvals already.
* Add calendar to mentor attendance view * Fix timezone bug when comparing dates in the mentor attendance calendar * Default to most recent past occurrence instead of most recent future occurrence * Fix initial attendance bug, add icon for unmarked attendances * Update cypress tests for changes in initial date selection, update student wotd date selection
The current calendar has a few annoyances, which could benefit from some QOL improvements.
In particular, the date selection is a horizontal scroll at the top, which is not the easiest to navigate---users are more used to vertical tabs, especially in situations where there are many options, as horizontal scrolling becomes unwieldy. In this new version of the page, the tabs are displayed vertically, on the left of the attendance form.
Additionally, a calendar has been added for better visual representation and selection of the attendance date. This calendar also provides some information about attendance history at a glance, listing the number of present students out of the total students for every section occurrence. The calendar can be toggled on or off with a button on the top left of the container.
Old calendar:

New calendar:
