feat: added courses loading skeleton#1608
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
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
I modified the colors of the calendar and course loading skeleton to be slightly lighter, so it should be ready for review. |
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
|
|
||
| return { | ||
| color: '#6d6d6d', | ||
| color: '#8d8d8d', |
There was a problem hiding this comment.
nit: sorry to nit again. is there any chance this could come from the MUI theme? is the default skeleton style not valid here?
There was a problem hiding this comment.
I would likely have to modify the existing calendar skeleton loading animation to match MUI skeleton, as the existing calendar skeleton doesn't actually use the MUI skeleton component. Instead, it uses the existing calendar system with some special CSS.
I could attempt to replicate the MUI skeleton component, but if neither the calender skeleton nor the added course skeleton use it, it would mean trying to compare and replicate it with vanilla CSS and rewriting both of these skeletons (added scope).
There was a problem hiding this comment.
thought: man this file sucks and i wrote this file
|
|
||
| export default function SectionTableLazyWrapper(props: SectionTableProps) { | ||
| return ( | ||
| <Suspense fallback={<div></div>}> |
There was a problem hiding this comment.
question: when does this codepath get hit? trying to understand where SectionTableSkeleton and AddedCoursesLoadingSkeleton differ
There was a problem hiding this comment.
This is hit right when initial section table data is loaded. I believe the section table is lazy loaded because some data takes extra time to load (haven't really dug into if that is necessary or the true reason). I know that before I made this change, the loading skeleton would have some flicker as it initially uses my skeleton, then switches over to the section table lazy loading fallback, then finally renders the full table.
I modified the fallback so that I could use the section table lazy loading as part of my skeleton.
| }; | ||
| }, []); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
issue: do we need a useEffect here? can't updating localStorage be handled in the callback to setCourses?
| } | ||
|
|
||
| // shade is #9d9d9d instead of #8d8d8d to account for gray overlay on calendar | ||
| const shimmerSx: SxProps = { |
There was a problem hiding this comment.
question: I... assume that this mirrors code from the calendar, but I just want to confirm that it is?
There was a problem hiding this comment.
I tried to mirror the loading animation as close as I could to the calendar skeleton.
Since the calendar skeleton piggybacks off of the existing calendar system instead of using the MUI skeleton component it makes it harder.
AntAlmanac/apps/antalmanac/src/components/Calendar/calendar.css
Lines 102 to 120 in 585d5c2
| return null; | ||
| } | ||
|
|
||
| export function AddedCoursesLoadingSkeleton() { |
There was a problem hiding this comment.
issue + question (repeat): still trying to figure out when each is rendered, but I'm seeing layout shift where it looks like the non-blueprint skeleton loads, then the blueprint skeleton loads... but if we have a copy in localStorage... why do we ever hit the non-blueprint?
similarly, if we have this in localStorage, why is there a separate default skeleton? if we have no blueprint, wouldn't we just render nothing / the empty state, like the calendar?
There was a problem hiding this comment.
Since my skeleton doesn't store the full names of the classes, there will still be some layout shift as I assume the length of the course.
I will try to see if I can use localStorage with the entire schedule similar to the calendar skeleton loader.
similarly, if we have this in localStorage, why is there a separate default skeleton? if we have no blueprint, wouldn't we just render nothing / the empty state, like the calendar?
I think that in attempting to "distinguish loading state from empty state" we need to signify that there might be a schedule that is not in localStorage (because it is being pulled from AntAlmanac), so a default skeleton would still make sense here.
2 participants
Summary
Distinguish loading state from empty state through a loading skeleton.
Screen.Recording.2026-04-13.at.1.39.15.AM.mov
Test Plan
If there is no data, it assumes 3 courses.
If the user has loaded a schedule in the past, it remembers how many courses/sections the user has to reconstruct a more accurate skeleton.
Issues
Fixes inability to distinguish loading state from empty state.
(from Kevin)