feat: added courses loading skeleton#1608
Merged
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Member
|
How's progress here? |
dsnsgithub
force-pushed
the
fix/distinguish-loading-state
branch
from
156b8d0 to
8185df6
Compare
dsnsgithub
force-pushed
the
fix/distinguish-loading-state
branch
from
8185df6 to
bdea2cd
Compare
Member
Author
I modified the colors of the calendar and course loading skeleton to be slightly lighter, so it should be ready for review. |
KevinWu098
previously requested changes
Apr 21, 2026
Member
KevinWu098
left a comment
KevinWu098
left a comment
There was a problem hiding this comment.
A handful of comments — feature looks good, but just hoping to flesh out the implementation a bit more
Member
|
i promise ill take a look at this soon, in the meanwhile do you mind handling the conflicts |
Distinguish loading state from empty state in the added courses pane by rendering a skeleton derived from a blueprint persisted to localStorage. The skeleton is rendered only when openLoadingSchedule is true AND a blueprint exists. When no blueprint is available, the loading state renders nothing, falling through to the empty state once loading completes (mirroring the calendar's empty-state behavior). Closes #1718 Co-authored-by: Dominic Seung <dominic@seung.dev>
dsnsgithub
force-pushed
the
fix/distinguish-loading-state
branch
from
7d03c42 to
b0a6a9a
Compare
dsnsgithub
force-pushed
the
fix/distinguish-loading-state
branch
from
4963ceb to
a931cdd
Compare
Co-authored-by: Dominic Seung <dominic@seung.dev>
- Move SectionTable's wrapSkeleton helper to module scope so it isn't reallocated each render. - Slim the persisted blueprint by stripping each section to an empty object. The skeleton only needs sections.length to render its stub rows, so persisting full meetings/instructors/enrollment bloated localStorage for nothing. SectionTable's skeleton mode now renders stub TableRows (with a variant="text" Skeleton per cell) instead of the full SectionTableBody, which is what makes the slim shape safe. - Subscribe AddedCoursesLoadingSkeleton to scheduleNamesChange so the cached names don't go stale while the skeleton is mounted (matches AddedSectionsGrid's pattern). - Drive the calendar's skeleton event color from theme.palette.action.disabledBackground instead of a hardcoded #9d9d9d, so it picks up dark mode. Skip colorContrastSufficient for skeleton events since the theme color is rgba() and skeleton titles are empty anyway. Co-authored-by: Dominic Seung <dominic@seung.dev>
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
- Memoize readCachedCourses via useState lazy init so it no longer hits localStorage / JSON.parse / validation on every render of AddedCoursesLoadingSkeleton (state updates from scheduleNamesChange used to retrigger it). - Add pointerEvents: none to the Skeleton wrappers in SectionTable's skeleton mode. The wrapped children already have visibility: hidden (from MUI's children-aware Skeleton), which blocks events in practice — but pointer-events: none on the wrapper is a belt-and- suspenders guard so the collapse chevron, drag handle, and syllabi popover cannot fire while the skeleton is visible. - Drop courseComment and prerequisiteLink contents from the persisted blueprint (replace with ''). They're the heaviest fields a course can carry, and SectionTable's skeleton mode never displays them. - Trim a comment in CalendarRoot that referenced the current implementation of colorContrastSufficient. Co-authored-by: Dominic Seung <dominic@seung.dev>
Stripping sections to {} + rendering stub <TableRow><Skeleton variant="text"/></TableRow>
in skeleton mode shrank localStorage but assumed every row was one
line tall. Real rows wrap when a section has multiple instructors or
multi-line meetings, so the skeleton came up short and caused a
layout shift downward when the real schedule rendered.
Revert that pair of changes: persist the full sections array and let
SectionTable's skeleton mode render the real SectionTableBody under
its children-aware Skeleton, so the placeholder matches the real
table's height per row.
Keep the other slim — courseComment and prerequisiteLink are still
dropped (replaced with empty strings) since neither contributes to
visible skeleton dimensions and courseComment in particular can be
large.
Co-authored-by: Dominic Seung <dominic@seung.dev>
Member
Author
|
@cubic-dev-ai review |
Contributor
@dsnsgithub I have started the AI code review. It will take a few minutes to complete. |
…ng-state # Conflicts: # apps/antalmanac/src/components/Calendar/CalendarRoot.tsx # apps/antalmanac/src/components/RightPane/SectionTable/SectionTable.tsx
vicksey
reviewed
May 20, 2026
Contributor
vicksey
left a comment
vicksey
left a comment
There was a problem hiding this comment.
LGTM. Tested on staging the skeleton loads and smoothly transitions to the sections when the data loads.
I just noticed 1 small thing for consistency sake: custom events don't have a skeleton on the added pane. I think it's worth adding since on the calendar pane custom events load with a skeleton.
Besides that nice pr!
The cached blueprint now stores both courses and customEvents so the skeleton renders the previous schedule's full added-pane shape, not just the SectionTables. CustomEventDetailView grows a `skeleton` prop that wraps the rendered Card in MUI's children-aware Skeleton — the hidden Card contributes layout, so each placeholder sizes to the real card it will be replaced by. AddedSectionsGrid subscribes to customEventsChange and re-persists on each event so a change in either list keeps the blueprint up to date. Legacy blueprints (plain array of courses) are still accepted on read for backwards compatibility — they just render an empty Custom Events section. Co-authored-by: Dominic Seung <dominic@seung.dev>
Member
Author
|
Custom events should now also have a skeleton (only when the user has any in their schedule). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Screen.Recording.2026-05-20.at.9.18.00.PM.mov
Issues
Fixes inability to distinguish loading state from empty state.
(from Kevin)
Closes #1718