fix: sat display #1380
fix: sat display #1380
Conversation
AureliaSindhu
had a problem deploying
to
staging
GitHub Actions
Failure
AureliaSindhu
had a problem deploying
to
staging
GitHub Actions
Error
KevinWu098
left a comment
KevinWu098
left a comment
There was a problem hiding this comment.
This looks like it does work for regular weeks, but on finals weeks, the week begins on Saturday. You can see this on the schedule, Fall 2025 exams are Dec 6–12 (Dec 6 is a Saturday).
In updating the start of the week, we have regressed in this area. Although it's outside the scope of the original issue, we should A. fix this and B. cannot proceed with this PR as it breaks finals functionality (which is more critical than the occasional Saturday class)
Your branch:
Production:
We may be able to dynamically update the start of the week based on if we're in finals viewing mode.
|
/deploy |
|
sorry i forgot to push my changes but I created a separate locale (en-us-finals) that starts the week on Saturday, stored separately from the regular en-us locale. So it now should detects when finals start on Saturday and dynamically switches to this Saturday-start locale only during finals viewing mode. |
|
/deploy |
KevinWu098
left a comment
KevinWu098
left a comment
There was a problem hiding this comment.
Really great work! Some minor questions regarding logic/fixed.
Additionally, I'd suggest that, when writing a PR, you include screenshots of the new behavior working (or a short video). Also, for your test plan, including courses which trigger certain cases (e.g. saturday classes, saturday final) etc would be helpful for your reviewer!
Some examples of what I used:
- Drama 198 (Fall 2025) — Saturday and Sunday classes
- Math 2B (Fall 2025) — Saturday finals
apps/antalmanac/src/components/Calendar/Toolbar/CustomEventDialog/CustomEventDialog.tsx
Show resolved
Hide resolved
AureliaSindhu
temporarily deployed
to
staging-1380
GitHub Actions
Inactive
AureliaSindhu
temporarily deployed
to
staging-1380
GitHub Actions
Inactive
|
Is this ready for review? |
KevinWu098
left a comment
KevinWu098
left a comment
There was a problem hiding this comment.
A few nits. Also, make sure to update your PR description with a demo of changes (before/after), as well as update your test plan with example courses
| return momentLocalizer(moment); | ||
| }, [culture]); | ||
|
|
||
| const calendarView = showFinalsSchedule |
There was a problem hiding this comment.
issue: this seems overly complex. can we simplify the ternary?
AureliaSindhu
temporarily deployed
to
staging-1380
GitHub Actions
Inactive
AureliaSindhu
requested a deployment
to
staging-1380
GitHub Actions
In progress
Summary
Fixed the calendar week layout so Saturday appears after Friday instead of before Sunday. Changed the week start day from Saturday (
dow: 6) to Sunday (dow: 0).Before: Sa | Su | M | Tu | W | Th | F
After: Su | M | Tu | W | Th | F | Sa
Test Plan
Issues
Saturday was displayed for the intial week.
Closes #1358