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.

Sign up for GitHub

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

Jump to bottom

Add Custom Events and Schedule Notes to Skeleton Mode #702

Merged
merged 52 commits into from
Nov 30, 2023
Merged

Add Custom Events and Schedule Notes to Skeleton Mode #702

merged 52 commits into from
Nov 30, 2023

Conversation

KevinWu098
Copy link
Member

@KevinWu098 KevinWu098 commented Sep 19, 2023 •
edited
Loading

Summary

  1. Custom Events are now viewable in Skeleton Mode. Creation and Editing functionality is disabled.
  2. Schedule Notes are now viewable in Skeleton Mode. Creation and Editing functionality is disabled.
  3. Styling has been cleaned up for both skeleton and live mode.
  4. Schedule Select Popover in the Calendar Toolbar is disabled.

Before and Afters

Empty Schedule

Before
Before - Empty Schedule
After
After - Empty Schedule

Custom Events Box

Before
Before - Custom Events
After
After - Custom Events

Skeleton Mode

Before
Before - Skeleton Mode
After
After - Skeleton Mode

Test Plan

  1. Load a schedule with courses, custom events, and a note in the first schedule (the one at the top of the dropdown)
  2. Change PETERPORTAL_WEBSOC_ENDPOINT to trigger skeleton mode
  3. Check that custom events are there and are not editable
  4. Check that notes are there and are not editable
  5. For the Calendar Toolbar, check that the Schedule Dropdown, Finals Button, Undo, Delete, and Add Custom Event buttons are disabled
  6. For the App Bar, check that Save/Load, and Export are disabled. Notably, Import remains enabled because refactoring it to subscribe to the AppStore properly is somewhat out of scope. Import will still fail though!
  7. For the edge case where there are no course events in any schedule, Skeleton Mode will not trigger. This is the current intended behavior until a better "Ping Websoc" option is created.
  8. CustomEventDialog was refactored. Test that custom events can still be created as long as day and schedule are checked. Make sure that editing the event from both the Calendar and the Added pane still work (and fills the appropriate data).

Issues

Closes #700 and Closes #720

Future Follow-up

  1. Refactor Import.tsx and properly subscribe to AppStore for the status of SkeletonMode: Disable Import in Skeleton Mode, Subscribe to Skeleton Mode #771
  2. Utilize react-hook-form to clean up CustomEventDialog.tsx

@KevinWu098 KevinWu098 requested review from MinhxNguyen7 and removed request for teresa-liang September 19, 2023 07:54
ap0nia
ap0nia approved these changes Sep 20, 2023
View reviewed changes
Copy link
Collaborator

@ap0nia ap0nia left a comment

Choose a reason for hiding this comment

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

Good improvements. Left some suggestions 🚀

ap0nia
ap0nia requested changes Sep 20, 2023
View reviewed changes
Copy link
Collaborator

@ap0nia ap0nia left a comment

Choose a reason for hiding this comment

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

Oh, I meant to request changes. Oops lol

@KevinWu098 KevinWu098 removed a link to an issue Oct 2, 2023
Skeleton Mode doesn't trigger when no courses are added #704
Closed
@KevinWu098 KevinWu098 mentioned this pull request Oct 3, 2023
Closed
@KevinWu098 KevinWu098 mentioned this pull request Oct 16, 2023
Closed
MinhxNguyen7
MinhxNguyen7 approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@MinhxNguyen7 MinhxNguyen7 left a comment

Choose a reason for hiding this comment

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

Works and LGTM!

@KevinWu098 KevinWu098 dismissed ap0nia’s stale review October 30, 2023 19:04

I think everything is addressed

@KevinWu098 KevinWu098 linked an issue Oct 30, 2023 that may be closed by this pull request
Too much gap between courses in Added Pane #720
Closed
@KevinWu098 KevinWu098 mentioned this pull request Oct 30, 2023
Closed
MinhxNguyen7
MinhxNguyen7 approved these changes Nov 24, 2023
View reviewed changes
Copy link
Member

@MinhxNguyen7 MinhxNguyen7 left a comment

Choose a reason for hiding this comment

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

Ship it

@KevinWu098
Copy link
Member Author

KevinWu098 commented Nov 24, 2023 •
edited
Loading

Ship it

This PR is cursed to be forever be locked in merge conflict hell 😭 -> I merged in #726 's changes and re-tested; hopefully everything's running ok still

@MinhxNguyen7
Copy link
Member

There are more merge conflicts 💀

@KevinWu098
Copy link
Member Author

KevinWu098 commented Nov 30, 2023 •
edited
Loading

There are more merge conflicts 💀

i love coding :)

MinhxNguyen7
MinhxNguyen7 approved these changes Nov 30, 2023
View reviewed changes
Copy link
Member

@MinhxNguyen7 MinhxNguyen7 left a comment

Choose a reason for hiding this comment

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

Idk, seems alright

@KevinWu098 KevinWu098 merged commit 97edefc into main Nov 30, 2023
@EricPedley EricPedley deleted the skeleton-details branch February 6, 2024 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MinhxNguyen7 MinhxNguyen7 MinhxNguyen7 approved these changes

@ap0nia ap0nia Awaiting requested review from ap0nia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Too much gap between courses in Added Pane Support Custom Events and Schedule Notes in Skeleton Mode
3 participants
@KevinWu098 @MinhxNguyen7 @ap0nia