feat: Calendar Schedule component finished, fix: list didn't allow updates when adding a new schedule #115
Conversation
… schedule with calendar. Still debugging.
…while adding new items to the end
| </div> | ||
| <div> | ||
| <CalendarBottomBar /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </> | ||
| {/* TODO: Doesn't work when exampleCourse is replaced with an actual course through setCourse. |
There was a problem hiding this comment.
Interesting. Does this merit creating an issue?
There was a problem hiding this comment.
I'm not sure semantically what defines an issue, but I thought I might want to be able to hand it off as a task.
There was a problem hiding this comment.
Sure, I think it's a good idea to at least keep track of it and assign/hand it off.
| function getTimeString(options: TimeStringOptions, startTime: number, endTime: number): string { | ||
| const startHour = Math.floor(startTime / 60); | ||
| const startMinute = startTime % 60; | ||
| const endHour = Math.floor(endTime / 60); | ||
| const endMinute = endTime % 60; | ||
|
|
||
| let startTimeString = ''; | ||
| let endTimeString = ''; | ||
|
|
||
| if (startHour === 0) { | ||
| startTimeString = '12'; | ||
| } else if (startHour > 12) { | ||
| startTimeString = `${startHour - 12}`; | ||
| } else { | ||
| startTimeString = `${startHour}`; | ||
| } | ||
|
|
||
| startTimeString += startMinute === 0 ? ':00' : `:${startMinute}`; | ||
| startTimeString += startHour >= 12 ? 'pm' : 'am'; | ||
|
|
||
| if (endHour === 0) { | ||
| endTimeString = '12'; | ||
| } else if (endHour > 12) { | ||
| endTimeString = `${endHour - 12}`; | ||
| } else { | ||
| endTimeString = `${endHour}`; | ||
| } | ||
| endTimeString += endMinute === 0 ? ':00' : `:${endMinute}`; | ||
| endTimeString += endHour >= 12 ? 'pm' : 'am'; | ||
|
|
||
| if (options.capitalize) { | ||
| startTimeString = startTimeString.toUpperCase(); | ||
| endTimeString = endTimeString.toUpperCase(); | ||
| } | ||
|
|
||
| return `${startTimeString} ${options.separator} ${endTimeString}`; | ||
| } |
There was a problem hiding this comment.
Nice job!
I would move this to a util file and create unit tests for it.
There was a problem hiding this comment.
It's actually from existing code from the CourseMeeting class. I'll refactor it to actually make a new CourseMeeting object from the parameters, and then just call the getTimeString() method on the CourseMeeting object.
There was a problem hiding this comment.
Actually, meeting should definitely already have been a CourseMeeting object. Remaking a new CourseMeeting object from it would just be more confusing than having a duplicate getTimeString() function. As a reminder, the issue I was running into was that at runtime I'd get an error that getTimeString wasn't a function when trying to call meeting.getTimeString() even though getTimeString was a function in the CourseMeeting class.
I do have a JSDoc function header though saying that the getTimeString() method in useFlattenedHook is a duplicate.
|
Note: need to add back in the custom ESLint rule from #110 and make sure that there is a check moving forward that it is included. |
Co-authored-by: doprz <[email protected]>
Co-authored-by: doprz <[email protected]>
Co-authored-by: doprz <[email protected]>
Co-authored-by: doprz <[email protected]>
Co-authored-by: doprz <[email protected]>
Co-authored-by: doprz <[email protected]>
…dates when adding a new schedule (#115) * Temporarily uninstalling husky cause github desktop has issues with it * Cleaned up some code. Removed unnecessary state value on injected popup * Should've fixed popup alignment issue. Still need to integrate course schedule with calendar. Still debugging. * Updated CalendarGridStories * Fix: change to ExampleCourse from exampleCourse * setCourse and calendar header need work * Update as part of merge * Fix: fixed build errors * Fix: Added Todo * Chore: Cleaned up useFlattenedCourseSchedule hook * fix: List now keeps track of state when existing items are switched, while adding new items to the end * Added back husky * Update src/views/components/calendar/Calendar/Calendar.tsx Co-authored-by: doprz <[email protected]> * refactor: added type-safety, destructuring, etc. ready for re-review * refactor: got rid of ts-ignore in openNewTabFromContentScript * Update src/views/components/calendar/CalendarHeader/CalenderHeader.tsx Co-authored-by: doprz <[email protected]> * refactor: using path aliasing Co-authored-by: doprz <[email protected]> * refactor: using path aliasing Co-authored-by: doprz <[email protected]> * refactor: using satisfies instead of as Co-authored-by: doprz <[email protected]> * refactor: using satisfies instead of as Co-authored-by: doprz <[email protected]> * style: reformatted spacing * style: eslint import order * refactor: added new constructor for UserSchedule to avoid passing down null values to child props --------- Co-authored-by: doprz <[email protected]>
Completed Feature:
Calendar schedule component now fully functional. Make and switch between schedules.
Added state to CalendarHeader and PopupMain.
Bug fix:
List items were previously only set on the initial render. Needed to allow new items to be added to the list while maintaining the list order caused by dragging elements. Although now that I think about it, we probably want the list order to persist on the backend not just the frontend
WIP feature:
Clicking on a course grid cell on the calendar page can load an ExampleCourse object in popup, but not a course object from the useFlattenedHook. Still need to debug.
I know I'm still using magic numbers, but most of the major changes I started when I thought we were on a deadline. I know the magic numbers need to be changed eventually, but there's a lot of functionality here that can be worked on, so I thought I should make a pr request.
In the future, I'll make smaller, higher quality PRs.