Skip to content

Implement Course Detail Panel Component #18

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
norioriroiron wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Implement Course Detail Panel Component #18

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7c39ae
implement responsiveness
norioriroiron Jun 10, 2026
07f74a2
implement close on esc press
norioriroiron Jun 10, 2026
94ef745
implement focus when panel opens
norioriroiron Jun 10, 2026
3832ae0
add course details to panel
norioriroiron Jun 10, 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
65 changes: 54 additions & 11 deletions src/components/CourseDetailPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Course } from '@/types/course';
import { useEffect, useRef } from 'react';

interface Props {
course: Course | null;
Expand All @@ -16,15 +17,43 @@ interface Props {
// shell. Rendering the actual course fields and styling is in scope for the ticket.

export default function CourseDetailPanel({ course, onClose }: Props) {
const panelRef = useRef<HTMLElement>(null);
const prevFocusRef = useRef<HTMLElement | null>(null);

useEffect(() => {
if (course) {
prevFocusRef.current = document.activeElement as HTMLElement;
panelRef.current?.focus();
} else {
prevFocusRef.current?.focus();
prevFocusRef.current = null;
}
}, [course]);

useEffect(() => {
const handleKey = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
onClose();
}
};

document.addEventListener('keydown', handleKey);
return () => document.removeEventListener('keydown', handleKey);
}, [onClose]);

if (course === null) return null;

return (
<aside
className="absolute inset-y-0 right-0 z-10 w-80 overflow-y-auto border-l border-gray-200 bg-white p-4 shadow-lg"
className="absolute inset-y-0 right-0 z-10 w-90 max-sm:w-5/6 overflow-y-auto border-l border-gray-200 bg-white p-4 shadow-lg"
aria-label={`Details for ${course.code}`}
ref={panelRef}
tabIndex={-1}
>
<div className="flex items-start justify-between gap-2">
<h2 className="text-lg font-bold text-gray-900">{course.code}</h2>
<div className="flex items-start justify-between gap-2 border-b border-gray-200 pb-1">

@AJaccP AJaccP Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few enhancements for the header part of the course detail panel:

  • Pull the title into the header block alongside the code so they read as one unit, with the code slightly larger than the title (e.g. text-xl for the course code and credit value, text-base for the course title). The section below the border can then have the description, precludes and prerequisites.
  • Give the code + credit a red accent (e.g. text-red-600) for a touch of Carleton red.

<h2 className="text-lg font-bold text-gray-900">
{course.code} [{course.credits} credit]
</h2>
<button
type="button"
onClick={onClose}
Expand All @@ -35,14 +64,28 @@ export default function CourseDetailPanel({ course, onClose }: Props) {
</button>
</div>

{/*
TODO(volunteer) — flesh out the panel body. Fields available on `course`:
title, credits, description, prereqRaw, precludes.
Prereq: render `course.prereqRaw` as plain text for now (a richer view of
the parsed `course.prereq` AST is a later enhancement).
Also: visual design, Escape-to-close + focus management, mobile treatment
See the course-detail-panel ticket.
*/}
<div className="space-y-3">

@AJaccP AJaccP Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add slightly more spacing between the heading div and body div here (e.g. give this a mt-4)

<div className="font-semibold">{course.title}</div>

<div>
<h3 className="font-semibold">Description</h3>

@AJaccP AJaccP Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unlike precludes and prereqRaw, the Description block isn't guarded for an empty value, so an empty description would render a lone "Description" heading with nothing under it. Descriptions are unlikely to be missing for any course, but for consistency please wrap it the same way:

{course.description && (
  <div>
    <h3 className="font-semibold">Description</h3>
    <p>{course.description}</p>
  </div>
)}

<p>{course.description}</p>
</div>

{course.precludes && course.precludes.length > 0 && (
<div>
<h3 className="text- font-semibold">Precludes</h3>

@AJaccP AJaccP Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On lines 77 and 84 it looks like there's a stray text- class leftover that's not doing anything. This could be removed so it's just className="font-semibold", matching description on line 71.

<p>{course.precludes.join(', ')}</p>
</div>
)}

{course.prereqRaw && (
<div>
<h3 className="text- font-semibold">Prerequisite(s)</h3>
<p>{course.prereqRaw}</p>
</div>
)}
</div>
</aside>
);
}
Loading