Skip to content

Improve calendar event display + MUI TimePicker #173

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

Open
devin-ai-integration wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve calendar event display + MUI TimePicker #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67ccab6
improve calendar event display: show course + location, hide time
devin-ai-integration[bot] Jun 22, 2026
ec39000
fix shortLocation regex to match basement rooms (B280 MRE, 115c BAX, …
devin-ai-integration[bot] Jun 22, 2026
6659257
replace native time inputs with MUI TimePicker (24hr, consistent desi…
devin-ai-integration[bot] Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
"@hello-pangea/dnd": "^18.0.1",
"@mui/icons-material": "^9.1.0",
"@mui/material": "^9.1.0",
"@mui/x-date-pickers": "^9.6.0",
"@preact/signals": "^2.9.1",
"@schedule-x/calendar": "^4.6.0",
"@schedule-x/events-service": "^4.6.0",
"@schedule-x/react": "^4.1.0",
"@schedule-x/theme-default": "^4.6.0",
"dayjs": "^1.11.21",
"fzf": "^0.5.1",
"ics": "^3.12.0",
"lz-string": "^1.5.0",
Expand Down
79 changes: 58 additions & 21 deletions src/Planner.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,29 @@ import { createViewWeek, CalendarConfig } from "@schedule-x/calendar";
import { createEventsServicePlugin } from "@schedule-x/events-service";
import { useCalendarApp, ScheduleXCalendar } from "@schedule-x/react";
import { Temporal } from "temporal-polyfill";
import { TimePicker } from "@mui/x-date-pickers/TimePicker";
import dayjs from "dayjs";

import "@schedule-x/theme-default/dist/index.css";

import "./css/planner.css";

const hasWeekendCourse = false;

function formatTime(date: Date): string {
return `${String(date.getHours()).padStart(2, "0")}:${String(
date.getMinutes(),
).padStart(2, "0")}`;
/** Extract a concise room label from the raw newline-separated locations string. */
function shortLocation(raw: string): string {
if (!raw) return "";
const lines = raw
.split("\n")
.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));

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

📝 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

if (rooms.length > 0) return rooms[0];
// fall back to building codes (short all-caps tokens)
const buildings = lines.filter((l) => /^[A-Z]{2,}$/.test(l));
if (buildings.length > 0) return buildings[0];
return "";
}

function TimeInput({
Expand All @@ -29,18 +41,30 @@ function TimeInput({
label: string;
}) {
return (
<input
type="time"
className="planner-time-input"
aria-label={label}
value={formatTime(value)}
onChange={(e) => {
if (!e.target.value) return;
const [hours, minutes] = e.target.value.split(":").map(Number);
<TimePicker
value={dayjs(value)}
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);
Comment on lines +46 to 50

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 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.

Suggested change
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);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}}
ampm={false}
slotProps={{
textField: {
size: "small",
slotProps: { htmlInput: { "aria-label": label } },
sx: {
width: "5.5rem",
"& .MuiInputBase-input": {
p: "4px 8px",
fontSize: "0.8rem",
textAlign: "center",
},
"& .MuiInputAdornment-root": { display: "none" },
},
},
}}
/>
);
}
Expand All @@ -66,7 +90,8 @@ function CourseToDates(courses: CourseStorage[]): DateData[] {
for (const interval of day) {
dates.push({
id: course.courseData.id,
title: course.courseData.number + " Section " + section.number,
title: course.courseData.number + " §" + section.number,
location: shortLocation(section.locations),

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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).

start: interval!.start,
end: interval!.end,
courseData: course.courseData,
Expand Down Expand Up @@ -103,14 +128,26 @@ function courseColors(id: number) {
};
}

function escapeHtml(s: string): string {
return s.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
}

function toExternalEvents(calEvents: DateData[]) {
return calEvents.map((event, idx) => ({
id: `${event.id}-${idx}`,
title: event.title,
start: toZonedDateTime(event.start),
end: toZonedDateTime(event.end),
calendarId: `course-${event.id}`,
}));
return calEvents.map((event, idx) => {
const loc = event.location;
const html = loc
? `<span class="sx-event-title">${escapeHtml(event.title)}</span><br/><span class="sx-event-location">${escapeHtml(loc)}</span>`
: `<span class="sx-event-title">${escapeHtml(event.title)}</span>`;

return {
id: `${event.id}-${idx}`,
title: event.title,
start: toZonedDateTime(event.start),
end: toZonedDateTime(event.end),
calendarId: `course-${event.id}`,
_customContent: { timeGrid: html },

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🚩 _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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

};
});
}

function ScheduleCalendar({ calEvents }: { calEvents: DateData[] }) {
Expand Down
24 changes: 13 additions & 11 deletions src/css/planner.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@
display: none;
}

.planner-time-input {
border: 1px solid lightgrey;
border-radius: 4px;
width: 100%;
min-width: 0;
max-width: 4.5rem;
text-align: center;
background-color: white;
font-size: 0.875rem;
line-height: 1.5;
/* custom event content */
.sx-event-title {
font-weight: 600;
font-size: 0.75rem;
line-height: 1.2;
}

.planner-time-input::-webkit-calendar-picker-indicator {
.sx-event-location {
font-size: 0.65rem;
opacity: 0.8;
line-height: 1.2;
}

/* hide the default time label Schedule-X renders inside events */
.sx__time-grid-event .sx__time-grid-event-time {
display: none;
}
Comment on lines +29 to 31

@devin-ai-integration devin-ai-integration Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

📝 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

6 changes: 5 additions & 1 deletion src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { StrictMode } from "react";
import ReactDOM from "react-dom/client";
import { LocalizationProvider } from "@mui/x-date-pickers/LocalizationProvider";
import { AdapterDayjs } from "@mui/x-date-pickers/AdapterDayjs";
import App from "./App";

import "./css/tailwind.css";
Expand All @@ -10,6 +12,8 @@ const root = ReactDOM.createRoot(

root.render(
<StrictMode>
<App />
<LocalizationProvider dateAdapter={AdapterDayjs}>
<App />
</LocalizationProvider>
</StrictMode>,
);
1 change: 1 addition & 0 deletions src/react-app-env.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface CourseData {
interface DateData {
id: number;
title: string;
location: string;
start: Date;
end: Date;
courseData: CourseData;
Expand Down