Improve calendar event display + MUI TimePicker#173
Improve calendar event display + MUI TimePicker#173devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
caltech-dev | 6659257 | Commit Preview URL Branch Preview URL |
Jun 22 2026, 07:12 AM |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| start: toZonedDateTime(event.start), | ||
| end: toZonedDateTime(event.end), | ||
| calendarId: `course-${event.id}`, | ||
| _customContent: { timeGrid: html }, |
There was a problem hiding this comment.
🚩 _customContent usage with Schedule-X v4 cannot be verified without node_modules
The PR uses _customContent: { timeGrid: html } at line 140 to inject custom HTML into calendar events. This is a Schedule-X feature for custom event rendering, but with @schedule-x/calendar: ^4.6.0 specified in package.json, I cannot verify whether this specific API shape is supported in the installed version since node_modules are not available. The CSS at src/css/planner.css:45-47 hides the default time label (.sx__time-grid-event-time) which suggests the custom content is intended to fully replace the default event rendering. If _customContent is not recognized, events would still render (using the title field which is still set), but without the location info or custom styling.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified locally — _customContent.timeGrid works correctly with @schedule-x/calendar@^4.6.0. Events render with the custom HTML (title + location), and the title field is preserved as fallback. Build passes, and the custom content is visible in the running app.
| id: course.courseData.id, | ||
| title: course.courseData.number + " Section " + section.number, | ||
| title: course.courseData.number + " §" + section.number, | ||
| location: shortLocation(section.locations), |
There was a problem hiding this comment.
🚩 shortLocation always returns the first room for multi-location sections
When a section has multiple newline-separated locations (e.g. "142 KCK\nB280 MRE" or "Lecture Hall\nBAX\n103 DWN"), shortLocation returns only the first matching room. This is a single location label applied to all time intervals of the section, even though different intervals may meet in different rooms. The existing code in src/Workspace.tsx:154-158 zips locations with time entries, suggesting the data model supports per-interval locations. This means the planner might show the wrong room for some time slots of multi-location sections.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — the location data format isn't a clean 1:1 zip with time entries. E.g., Ma 1a §1 has 3 time entries but 5 location lines ("Lecture Hall BAX 387 LINDE Lecture Hall BAX"). The existing zip approach in Workspace.tsx:154-158 is also imprecise for the same reason. Per-interval location mapping would require heuristic parsing of multi-line location blocks — not worth the complexity for this PR. Showing the first room match is a reasonable default (better than no location at all).
…etc.) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Testing ResultsTested locally on Test 1: Event blocks show course + location, NOT time — PASSED
Test 2: Basement room regex fix (B-prefix) — PASSEDAdded EE 152 (location:
Test 3: Time picker width — PASSED
CI: 6/6 green (verify, CodeQL, Analyze x2, Workers Builds, Devin Review) |
…gn system) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
devin-ai-integration
Bot
changed the title
| onChange={(newVal) => { | ||
| if (!newVal) return; | ||
| const day = new Date(value); | ||
| day.setHours(hours, minutes, 0, 0); | ||
| day.setHours(newVal.hour(), newVal.minute(), 0, 0); | ||
| onChange(day); |
There was a problem hiding this comment.
🟡 Missing dayjs isValid() check allows NaN to corrupt state
MUI's TimePicker (v9.6.0) can fire onChange with an invalid Dayjs object when the user partially edits or clears a time section in the field. The code only guards against null (if (!newVal) return;) but doesn't check newVal.isValid(). When an invalid Dayjs is used, .hour() and .minute() return NaN, causing day.setHours(NaN, NaN, 0, 0) to produce an Invalid Date. This corrupted Date propagates through updateAvailableTimes (src/useAppState.ts:474) into localStorage, and breaks the scheduler at src/scheduler.ts:87-88 where availableTimes[j][0].getTime() returns NaN, making all availability comparisons fail and preventing any valid arrangements from being generated.
| onChange={(newVal) => { | |
| if (!newVal) return; | |
| const day = new Date(value); | |
| day.setHours(hours, minutes, 0, 0); | |
| day.setHours(newVal.hour(), newVal.minute(), 0, 0); | |
| onChange(day); | |
| onChange={(newVal) => { | |
| if (!newVal || !newVal.isValid()) return; | |
| const day = new Date(value); | |
| day.setHours(newVal.hour(), newVal.minute(), 0, 0); | |
| onChange(day); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| .map((l) => l.trim()) | ||
| .filter(Boolean); | ||
| // room entries: "387 LINDE", "B280 MRE", "115c BAX", "240A CNRB" | ||
| const rooms = lines.filter((l) => /^[A-Z]?\d+\w*\s+[A-Z]/.test(l)); |
There was a problem hiding this comment.
📝 Info: The shortLocation regex won't match rooms with multi-letter prefixes
The regex /^[A-Z]?\d+\w*\s+[A-Z]/ uses [A-Z]? which matches zero or one uppercase letter before the digits. Room codes with two-letter prefixes (e.g., "AB123 BUILDING") would not match. The comments indicate this is intentional based on the known data format ("387 LINDE", "B280 MRE", "115c BAX", "240A CNRB"), but if new room formats appear in the catalog, they would silently fall through to the building code fallback or return empty string — a graceful degradation rather than an error.
Was this helpful? React with 👍 or 👎 to provide feedback.
| .sx__time-grid-event .sx__time-grid-event-time { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
📝 Info: CSS hides Schedule-X event time labels globally
The rule .sx__time-grid-event .sx__time-grid-event-time { display: none; } hides the default time label that Schedule-X renders inside all time-grid events. This is presumably intentional since the custom content (_customContent) replaces the default rendering. However, if any events are added without _customContent in the future, they'll lose their time labels with no visual indication of when they occur within the grid cell.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Two changes to the Planner calendar UX:
1. Event blocks show course + location instead of time
Time is already encoded by the block's vertical position on the grid — redundant to show it inside the block. Now events display the course number + room location, which is actually useful:
shortLocation()extracts the first room-number entry fromSectionData.locations(newline-separated, messy format)/^[A-Z]?\d+\w*\s+[A-Z]/handles regular rooms (387 LINDE), basement rooms (B280 MRE), and letter-suffix rooms (240A CNRB)escapeHtml()before injection2. Time pickers replaced with MUI
TimePickerOld: native
<input type="time">— visually inconsistent with MUI design system, 12hr format (browser-dependent), while course data uses 24hr.New:
@mui/x-date-pickersTimePickerwithampm={false}— MUI outlined styling, 24hr format matching course catalog data, consistent with other MUI components in the app.LocalizationProvider+AdapterDayjsat app root (index.tsx)MuiInputAdornment-root { display: none }for compact layout@mui/x-date-pickers@9.6.0,dayjs@1.11.21Note:
NODE_VERSION=22may need to be set in Cloudflare Workers Builds dashboard.Link to Devin session: https://app.devin.ai/sessions/2a1e057e9b584456bf6c5009f6573070
Requested by: @rchalamala