Open
Conversation
WalkthroughAdds a Course Comparison feature: new UI components and pages to select up to four courses, persist selections in localStorage with a custom event for sync, and render a comparison page with table, charts, and review highlights. Updates course list, course page, navbar, and introduces compare-related components and docs. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant CourseComparisonPage
participant useCourses
participant CourseSelector
participant ComparisonTable
participant ComparisonCharts
participant ReviewHighlights
participant ReviewDatabase
User->>Browser: Navigate to /courses/compare
Browser->>CourseComparisonPage: render (reads URL params)
CourseComparisonPage->>useCourses: fetch courses
useCourses-->>CourseComparisonPage: courses list
User->>CourseSelector: search & select courses
CourseSelector->>useCourses: request filtered courses
useCourses-->>CourseSelector: filtered results
CourseSelector-->>CourseComparisonPage: onCoursesChange(selected)
CourseComparisonPage->>ComparisonTable: pass selected courses
CourseComparisonPage->>ComparisonCharts: pass selected courses
CourseComparisonPage->>ReviewHighlights: pass selected courses
ReviewHighlights->>ReviewDatabase: fetch course UUIDs & reviews
ReviewDatabase-->>ReviewHighlights: reviews per course
ComparisonTable-->>User: render comparison table
ComparisonCharts-->>User: render charts
ReviewHighlights-->>User: render pros/cons/recent reviews
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@docs/COURSE_COMPARISON_FEATURE.md`:
- Around line 148-150: Add a language tag to the fenced code block that contains
the example route "/courses/compare?courses=MAL401,MAL402" to satisfy
markdownlint MD040; replace the opening fence ``` with ```text (or another
appropriate language) so the block becomes a labeled code block.
In `@src/app/courses/compare/page.tsx`:
- Around line 21-28: The preselection is case-sensitive so courseIds from
searchParams.get('courses') may not match courses; in the useEffect where you
build preselectedCourses (the courses.filter(...) and setSelectedCourses(...)
logic), normalize both sides to the same case before comparing: map the parsed
courseIds to lower-case and check includes against c.id.toLowerCase(), and if
your Course objects have a separate code field (e.g., c.code) also check
c.code.toLowerCase() so either identifier matches; then slice(0,4) and call
setSelectedCourses as before.
In `@src/components/courses/compare/ComparisonCharts.tsx`:
- Around line 69-173: The RadarChart is rendering duplicate series because each
<Radar> uses the same dataKey="overall" and radarData is shaped per course;
reshape radarData to be one object per metric (e.g., { metric: 'Overall',
[courseCodeA]: val, [courseCodeB]: val, ... } for overall, difficulty, workload,
reviews), update PolarAngleAxis to use dataKey="metric", and change each <Radar>
in the courses.map loop to use dataKey={course.code} (and keep stroke/fill using
colors[idx % colors.length]) so each series reads its own course field instead
of the shared "overall".
In `@src/components/courses/compare/CourseSelector.tsx`:
- Around line 69-74: The remove-course button in CourseSelector is an icon-only
control and needs accessibility and form-safety fixes: update the button element
used with onClick={() => handleRemoveCourse(course.id)} to include type="button"
and an appropriate aria-label (e.g., aria-label={`Remove ${course.name}` or a
generic "Remove course") so screen readers can announce its purpose; locate the
button in CourseSelector.tsx where handleRemoveCourse is invoked and add those
attributes.
In `@src/components/courses/compare/ReviewHighlights.tsx`:
- Around line 24-147: The Tabs use defaultValue so the active tab doesn't update
when courses changes; convert to a controlled Tabs by adding state (e.g., const
[activeTab, setActiveTab] = useState<string | null>(courses[0]?.id || null)) and
pass value={activeTab} and onValueChange={setActiveTab} to the Tabs component,
update TabsTrigger to use course.id as before, and add a useEffect watching
courses to reset activeTab to courses[0]?.id (or null) when the list changes so
removed course ids don't leave the Tabs pointing at a stale/non-existent id.
In `@src/components/courses/CompareButton.tsx`:
- Around line 25-29: The getComparisonList function should guard against
malformed JSON from localStorage by wrapping the JSON.parse call in a try-catch:
after retrieving stored = localStorage.getItem(STORAGE_KEY), attempt to parse it
inside try and on success return the parsed array; on catch return an empty
array (optionally remove the bad key with localStorage.removeItem(STORAGE_KEY)
or log the error) so that JSON.parse errors cannot throw at runtime.
- Around line 174-176: The Badge rendering calls
course.overall_rating.toFixed(1) which will throw if overall_rating is
null/undefined; update the rendering around the Badge (the block with Badge and
course.overall_rating) to guard against nullish values by using a safe fallback
such as const displayRating = (typeof course.overall_rating === 'number') ?
course.overall_rating.toFixed(1) : '–' (or 'N/A' / '0.0') and then render "⭐
{displayRating}", or conditionally render the Badge only when
course.overall_rating is a number; reference the Badge component and
course.overall_rating to locate where to apply the change.
In `@src/components/courses/CompareCoursesButton.tsx`:
- Around line 11-15: getComparisonList currently calls JSON.parse on
localStorage data which can throw on malformed values; wrap the parse in a
try/catch in getComparisonList (and keep the existing typeof window guard) so
parsing failures return an empty array instead of crashing, and after parsing
validate the result is an array of strings (otherwise return []); reference
getComparisonList and STORAGE_KEY when making this change.
In `@src/components/courses/course_page/AddToComparison.tsx`:
- Around line 16-20: The getComparisonList function can throw when localStorage
contains malformed JSON; wrap the localStorage access/JSON.parse in a try/catch
and return an empty array on any error, and additionally validate the parsed
value is an array of strings before returning it. Specifically update
getComparisonList to: check typeof window, read
localStorage.getItem(STORAGE_KEY) inside try, JSON.parse safely, ensure the
result is an Array and every item is a string (otherwise fallback), and
swallow/log the error then return [].
🧹 Nitpick comments (5)
src/components/courses/ItemCard.tsx (1)
94-209: Avoid nesting an interactive button inside the outer Link.
CompareButtonrenders a button (see src/components/courses/CompareButton.tsx lines 37-89), and it’s nested inside the outerLink, which creates invalid nested interactive elements and can confuse keyboard/screen-reader users. Consider moving theLinkto just the “View Details” affordance or making the card itself navigable without an anchor wrapper.💡 Possible restructure (Link only wraps “View Details”)
- return ( - <Link href={link} className="block w-full group"> - <Card + return ( + <Card className={`h-full overflow-hidden transition-all duration-300 border border-border/60 bg-card/40 backdrop-blur-sm hover:border-primary/50 hover:bg-card/60 hover:shadow-xl hover:shadow-primary/5 ${ isHovered ? 'scale-[1.02]' : 'scale-100' } ${className || ''}`} onMouseEnter={() => setIsHovered(true)} onMouseLeave={() => setIsHovered(false)} > ... - <CardFooter className="px-4 py-3 bg-background/40 backdrop-blur-sm border-t border-border/40 flex justify-between items-center group-hover:bg-primary/5 transition-all duration-300"> + <CardFooter className="px-4 py-3 bg-background/40 backdrop-blur-sm border-t border-border/40 flex justify-between items-center group-hover:bg-primary/5 transition-all duration-300"> {type === 'course' ? ( <> <CompareButton course={item as Course} /> - <div className="flex items-center gap-2"> + <Link href={link} className="flex items-center gap-2"> <div className="text-xs font-bold font-mono tracking-wide text-primary"> View Details </div> <div className="p-1.5 rounded-md bg-primary/10 group-hover:bg-primary/20 group-hover:translate-x-1 transition-all duration-300"> <ChevronRight className="h-3.5 w-3.5 text-primary" /> </div> - </div> + </Link> </> ) : ( - <> + <Link href={link} className="flex items-center gap-2"> <div className="text-xs font-bold font-mono tracking-wide text-primary"> View Details </div> <div className="p-1.5 rounded-md bg-primary/10 group-hover:bg-primary/20 group-hover:translate-x-1 transition-all duration-300"> <ChevronRight className="h-3.5 w-3.5 text-primary" /> </div> - </> + </Link> )} </CardFooter> - </Card> - </Link> + </Card> );src/app/courses/compare/page.tsx (1)
18-19: Avoid double‑fetching courses on the compare page.This page calls
useCourses, andCourseSelectoralso callsuseCourses, which likely triggers two full fetch cycles. Consider lifting the data into the page and passing it down to avoid duplicate network work.Example adjustment in this file
- <CourseSelector - selectedCourses={selectedCourses} - onCoursesChange={handleCoursesChange} - maxCourses={4} - /> + <CourseSelector + selectedCourses={selectedCourses} + onCoursesChange={handleCoursesChange} + maxCourses={4} + courses={courses} + />Additional change needed in
src/components/courses/compare/CourseSelector.tsx(outside this file): accept acoursesprop and remove the internaluseCoursescall so data is fetched once.Also applies to: 102-106
src/components/courses/CompareButton.tsx (3)
40-40: Remove unused state variablecomparisonCount.The
comparisonCountstate is updated in the effect but never read or rendered. This adds unnecessary overhead.♻️ Suggested fix
export function CompareButton({ course }: CompareButtonProps) { const [isInComparison, setIsInComparison] = useState(false); - const [comparisonCount, setComparisonCount] = useState(0); useEffect(() => { const updateState = () => { const list = getComparisonList(); setIsInComparison(list.includes(course.id)); - setComparisonCount(list.length); };
92-92: Consider documenting the requirement thatcoursesmust contain all potentially selected courses.The
coursesprop must include all courses that could be in the comparison list. If a user adds courses on one page and navigates to another page with a different/partialcoursesarray,selectedCourseswill be incomplete, causing a mismatch between the badge count and displayed items.Consider either:
- Fetching course details by IDs when needed, or
- Storing full course objects (not just IDs) in localStorage, or
- Adding a comment documenting this constraint for maintainers.
182-189: Consider addingaria-labelto the remove button for better accessibility.The remove button only contains an icon, which may not be clear to screen reader users.
♿ Suggested fix
<Button variant="ghost" size="sm" onClick={() => handleRemoveCourse(course.id)} className="ml-2" + aria-label={`Remove ${course.code} from comparison`} > <X className="h-4 w-4" /> </Button>
Comment on lines
+148
to
+150
| ``` | ||
| /courses/compare?courses=MAL401,MAL402 | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced code block.
markdownlint MD040 flags this; add a language (e.g., text) to keep docs lint‑clean.
Proposed fix
-```
+/```text
/courses/compare?courses=MAL401,MAL402</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @docs/COURSE_COMPARISON_FEATURE.md around lines 148 - 150, Add a language tag
to the fenced code block that contains the example route
"/courses/compare?courses=MAL401,MAL402" to satisfy markdownlint MD040; replace
the opening fence withtext (or another appropriate language) so the
block becomes a labeled code block.
</details>
<!-- fingerprinting:phantom:medusa:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+21
to
+28
| useEffect(() => { | ||
| if (!isLoading && courses.length > 0 && searchParams) { | ||
| const courseIds = searchParams.get('courses')?.split(',') || []; | ||
| if (courseIds.length > 0) { | ||
| const preselectedCourses = courses.filter((c) => | ||
| courseIds.includes(c.id) | ||
| ); | ||
| setSelectedCourses(preselectedCourses.slice(0, 4)); |
There was a problem hiding this comment.
Make URL preselection robust to code/id casing.
Direct links (e.g., docs examples) often use course codes; current logic matches only c.id case‑sensitively, so preselection may silently fail.
Proposed fix
- if (!isLoading && courses.length > 0 && searchParams) {
- const courseIds = searchParams.get('courses')?.split(',') || [];
- if (courseIds.length > 0) {
- const preselectedCourses = courses.filter((c) =>
- courseIds.includes(c.id)
- );
- setSelectedCourses(preselectedCourses.slice(0, 4));
- }
- }
+ if (!isLoading && courses.length > 0 && searchParams) {
+ const rawIds = searchParams.get('courses')?.split(',') || [];
+ const normalized = rawIds.map((id) => id.trim().toLowerCase()).filter(Boolean);
+ if (normalized.length > 0) {
+ const preselectedCourses = courses.filter(
+ (c) =>
+ normalized.includes(c.id.toLowerCase()) ||
+ normalized.includes(c.code.toLowerCase())
+ );
+ setSelectedCourses(preselectedCourses.slice(0, 4));
+ }
+ }🤖 Prompt for AI Agents
In `@src/app/courses/compare/page.tsx` around lines 21 - 28, The preselection is
case-sensitive so courseIds from searchParams.get('courses') may not match
courses; in the useEffect where you build preselectedCourses (the
courses.filter(...) and setSelectedCourses(...) logic), normalize both sides to
the same case before comparing: map the parsed courseIds to lower-case and check
includes against c.id.toLowerCase(), and if your Course objects have a separate
code field (e.g., c.code) also check c.code.toLowerCase() so either identifier
matches; then slice(0,4) and call setSelectedCourses as before.
Comment on lines
+69
to
+173
| // Data for Radar Chart | ||
| const radarData = courses.map((course, idx) => ({ | ||
| course: course.code, | ||
| overall: course.overall_rating, | ||
| difficulty: course.difficulty_rating, | ||
| workload: course.workload_rating, | ||
| reviews: Math.min(course.review_count / 10, 5), // Normalize reviews to 0-5 scale | ||
| fill: colors[idx % colors.length], | ||
| })); | ||
|
|
||
| // Data for Credits Comparison | ||
| const creditsData = courses.map((course, idx) => ({ | ||
| name: course.code, | ||
| credits: course.credits, | ||
| fill: colors[idx % colors.length], | ||
| })); | ||
|
|
||
| return ( | ||
| <div className="space-y-6"> | ||
| {/* Ratings Comparison Bar Chart */} | ||
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | ||
| <CardHeader> | ||
| <CardTitle className="flex items-center gap-2"> | ||
| 📊 Ratings Comparison | ||
| </CardTitle> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Compare overall rating, difficulty, and workload across courses | ||
| </p> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <ResponsiveContainer width="100%" height={300}> | ||
| <BarChart data={ratingsData}> | ||
| <CartesianGrid strokeDasharray="3 3" className="stroke-muted" /> | ||
| <XAxis | ||
| dataKey="metric" | ||
| className="text-xs" | ||
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | ||
| /> | ||
| <YAxis | ||
| domain={[0, 5]} | ||
| className="text-xs" | ||
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | ||
| /> | ||
| <Tooltip | ||
| contentStyle={{ | ||
| backgroundColor: 'hsl(var(--card))', | ||
| border: '1px solid hsl(var(--border))', | ||
| borderRadius: '0.5rem', | ||
| }} | ||
| /> | ||
| <Legend /> | ||
| {courses.map((course, idx) => ( | ||
| <Bar | ||
| key={course.id} | ||
| dataKey={course.code} | ||
| fill={colors[idx % colors.length]} | ||
| radius={[4, 4, 0, 0]} | ||
| /> | ||
| ))} | ||
| </BarChart> | ||
| </ResponsiveContainer> | ||
| </CardContent> | ||
| </Card> | ||
|
|
||
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> | ||
| {/* Radar Chart */} | ||
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | ||
| <CardHeader> | ||
| <CardTitle className="flex items-center gap-2"> | ||
| 🎯 Multi-Metric Radar | ||
| </CardTitle> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Visual overview of all metrics | ||
| </p> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <ResponsiveContainer width="100%" height={300}> | ||
| <RadarChart data={radarData}> | ||
| <PolarGrid className="stroke-muted" /> | ||
| <PolarAngleAxis | ||
| dataKey="course" | ||
| tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 12 }} | ||
| /> | ||
| <PolarRadiusAxis | ||
| angle={90} | ||
| domain={[0, 5]} | ||
| tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 10 }} | ||
| /> | ||
| <Tooltip | ||
| contentStyle={{ | ||
| backgroundColor: 'hsl(var(--card))', | ||
| border: '1px solid hsl(var(--border))', | ||
| borderRadius: '0.5rem', | ||
| }} | ||
| /> | ||
| {courses.map((course, idx) => ( | ||
| <Radar | ||
| key={course.id} | ||
| name={course.code} | ||
| dataKey="overall" | ||
| stroke={colors[idx % colors.length]} | ||
| fill={colors[idx % colors.length]} | ||
| fillOpacity={0.3} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Recharts RadarChart multiple series dataKey format
💡 Result:
In Recharts, a RadarChart “multiple series” is done by rendering multiple <Radar /> components, each with its own dataKey. Your data should be an array of objects where each object is one axis/category, and each series is a separate property on that object. (recharts.github.io)
Typical (recommended) shape: one field per series
const data = [
{ subject: "Math", mike: 120, lily: 110 },
{ subject: "English", mike: 98, lily: 130 },
];
<RadarChart data={data}>
<PolarAngleAxis dataKey="subject" />
<Radar name="Mike" dataKey="mike" />
<Radar name="Lily" dataKey="lily" />
</RadarChart>This matches the common Recharts examples where each Radar uses a different dataKey. (ui.tailus.io)
If your values are in an array (e.g. measurements: [5, 12])
Recharts dataKey can be a function, so you can pick an index explicitly. (recharts.github.io)
const data = [
{ label: "Foo", measurements: [5, 12] },
{ label: "Bar", measurements: [8, 10] },
];
<RadarChart data={data}>
<PolarAngleAxis dataKey="label" />
<Radar name="Series A" dataKey={(d) => d.measurements[0]} />
<Radar name="Series B" dataKey={(d) => d.measurements[1]} />
</RadarChart>If you tell me your exact data structure (one item), I can show the precise dataKeys to use.
Citations:
- 1: https://recharts.github.io/en-US/api/RadarChart/?utm_source=openai
- 2: https://ui.tailus.io/react/visualizations/radar-chart/?utm_source=openai
- 3: https://recharts.github.io/en-US/api/RadarChart/?utm_source=openai
🌐 Web query:
Recharts Radar component how to display multiple courses metrics comparison
💡 Result:
To compare multiple courses on a single Recharts RadarChart, structure your data as one row per metric, with one numeric field per course, then render one <Radar /> per course (each uses a different dataKey). This is the standard “multiple radars / multiple series overlay” pattern. [2][3][4]
1) Shape your data for Recharts
const data = [
{ metric: "Assignments", courseA: 82, courseB: 75, courseC: 90 },
{ metric: "Quizzes", courseA: 70, courseB: 88, courseC: 77 },
{ metric: "Exams", courseA: 91, courseB: 80, courseC: 85 },
{ metric: "Projects", courseA: 78, courseB: 92, courseC: 81 },
];2) Render multiple course polygons
import {
RadarChart, Radar, PolarGrid, PolarAngleAxis, PolarRadiusAxis,
Tooltip, Legend, ResponsiveContainer
} from "recharts";
export function CoursesRadar() {
return (
<ResponsiveContainer width="100%" height={360}>
<RadarChart data={data} outerRadius="80%">
<PolarGrid />
<PolarAngleAxis dataKey="metric" />
<PolarRadiusAxis domain={[0, 100]} />
<Radar name="Course A" dataKey="courseA" stroke="#8884d8" fill="#8884d8" fillOpacity={0.25} />
<Radar name="Course B" dataKey="courseB" stroke="#82ca9d" fill="#82ca9d" fillOpacity={0.20} />
<Radar name="Course C" dataKey="courseC" stroke="#ffc658" fill="#ffc658" fillOpacity={0.18} />
<Tooltip />
<Legend />
</RadarChart>
</ResponsiveContainer>
);
}Notes:
- Use different
strokecolors and lowerfillOpacityso overlaps remain readable (common in multi-series examples). [2][3] outerRadiusis a common sizing control onRadarChart. [1]
Sources: Recharts RadarChart API (props like outerRadius) [1], multi-radar overlay examples/patterns [2][3][4].
Radar chart renders duplicate series instead of per‑course comparison.
Each <Radar> uses the same dataKey="overall", so all series read identical data. Recharts RadarChart requires data shaped with one row per metric and one numeric field per course, then each series uses a different dataKey (one per course code). Reshape radarData by metric and update PolarAngleAxis and Radar components accordingly:
🔧 Suggested fix (align data shape with Radar series)
- const radarData = courses.map((course, idx) => ({
- course: course.code,
- overall: course.overall_rating,
- difficulty: course.difficulty_rating,
- workload: course.workload_rating,
- reviews: Math.min(course.review_count / 10, 5), // Normalize reviews to 0-5 scale
- fill: colors[idx % colors.length],
- }));
+ const radarData = [
+ {
+ metric: 'Overall',
+ ...courses.reduce((acc, course) => {
+ acc[course.code] = course.overall_rating;
+ return acc;
+ }, {} as Record<string, number>),
+ },
+ {
+ metric: 'Difficulty',
+ ...courses.reduce((acc, course) => {
+ acc[course.code] = course.difficulty_rating;
+ return acc;
+ }, {} as Record<string, number>),
+ },
+ {
+ metric: 'Workload',
+ ...courses.reduce((acc, course) => {
+ acc[course.code] = course.workload_rating;
+ return acc;
+ }, {} as Record<string, number>),
+ },
+ {
+ metric: 'Reviews',
+ ...courses.reduce((acc, course) => {
+ acc[course.code] = Math.min(course.review_count / 10, 5);
+ return acc;
+ }, {} as Record<string, number>),
+ },
+ ];
...
- <PolarAngleAxis
- dataKey="course"
+ <PolarAngleAxis
+ dataKey="metric"
tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 12 }}
/>
...
- {courses.map((course, idx) => (
- <Radar
- key={course.id}
- name={course.code}
- dataKey="overall"
- stroke={colors[idx % colors.length]}
- fill={colors[idx % colors.length]}
- fillOpacity={0.3}
- />
- ))}
+ {courses.map((course, idx) => (
+ <Radar
+ key={course.id}
+ name={course.code}
+ dataKey={course.code}
+ stroke={colors[idx % colors.length]}
+ fill={colors[idx % colors.length]}
+ fillOpacity={0.3}
+ />
+ ))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Data for Radar Chart | |
| const radarData = courses.map((course, idx) => ({ | |
| course: course.code, | |
| overall: course.overall_rating, | |
| difficulty: course.difficulty_rating, | |
| workload: course.workload_rating, | |
| reviews: Math.min(course.review_count / 10, 5), // Normalize reviews to 0-5 scale | |
| fill: colors[idx % colors.length], | |
| })); | |
| // Data for Credits Comparison | |
| const creditsData = courses.map((course, idx) => ({ | |
| name: course.code, | |
| credits: course.credits, | |
| fill: colors[idx % colors.length], | |
| })); | |
| return ( | |
| <div className="space-y-6"> | |
| {/* Ratings Comparison Bar Chart */} | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardHeader> | |
| <CardTitle className="flex items-center gap-2"> | |
| 📊 Ratings Comparison | |
| </CardTitle> | |
| <p className="text-sm text-muted-foreground"> | |
| Compare overall rating, difficulty, and workload across courses | |
| </p> | |
| </CardHeader> | |
| <CardContent> | |
| <ResponsiveContainer width="100%" height={300}> | |
| <BarChart data={ratingsData}> | |
| <CartesianGrid strokeDasharray="3 3" className="stroke-muted" /> | |
| <XAxis | |
| dataKey="metric" | |
| className="text-xs" | |
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | |
| /> | |
| <YAxis | |
| domain={[0, 5]} | |
| className="text-xs" | |
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | |
| /> | |
| <Tooltip | |
| contentStyle={{ | |
| backgroundColor: 'hsl(var(--card))', | |
| border: '1px solid hsl(var(--border))', | |
| borderRadius: '0.5rem', | |
| }} | |
| /> | |
| <Legend /> | |
| {courses.map((course, idx) => ( | |
| <Bar | |
| key={course.id} | |
| dataKey={course.code} | |
| fill={colors[idx % colors.length]} | |
| radius={[4, 4, 0, 0]} | |
| /> | |
| ))} | |
| </BarChart> | |
| </ResponsiveContainer> | |
| </CardContent> | |
| </Card> | |
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> | |
| {/* Radar Chart */} | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardHeader> | |
| <CardTitle className="flex items-center gap-2"> | |
| 🎯 Multi-Metric Radar | |
| </CardTitle> | |
| <p className="text-sm text-muted-foreground"> | |
| Visual overview of all metrics | |
| </p> | |
| </CardHeader> | |
| <CardContent> | |
| <ResponsiveContainer width="100%" height={300}> | |
| <RadarChart data={radarData}> | |
| <PolarGrid className="stroke-muted" /> | |
| <PolarAngleAxis | |
| dataKey="course" | |
| tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 12 }} | |
| /> | |
| <PolarRadiusAxis | |
| angle={90} | |
| domain={[0, 5]} | |
| tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 10 }} | |
| /> | |
| <Tooltip | |
| contentStyle={{ | |
| backgroundColor: 'hsl(var(--card))', | |
| border: '1px solid hsl(var(--border))', | |
| borderRadius: '0.5rem', | |
| }} | |
| /> | |
| {courses.map((course, idx) => ( | |
| <Radar | |
| key={course.id} | |
| name={course.code} | |
| dataKey="overall" | |
| stroke={colors[idx % colors.length]} | |
| fill={colors[idx % colors.length]} | |
| fillOpacity={0.3} | |
| /> | |
| ))} | |
| // Data for Radar Chart | |
| const radarData = [ | |
| { | |
| metric: 'Overall', | |
| ...courses.reduce((acc, course) => { | |
| acc[course.code] = course.overall_rating; | |
| return acc; | |
| }, {} as Record<string, number>), | |
| }, | |
| { | |
| metric: 'Difficulty', | |
| ...courses.reduce((acc, course) => { | |
| acc[course.code] = course.difficulty_rating; | |
| return acc; | |
| }, {} as Record<string, number>), | |
| }, | |
| { | |
| metric: 'Workload', | |
| ...courses.reduce((acc, course) => { | |
| acc[course.code] = course.workload_rating; | |
| return acc; | |
| }, {} as Record<string, number>), | |
| }, | |
| { | |
| metric: 'Reviews', | |
| ...courses.reduce((acc, course) => { | |
| acc[course.code] = Math.min(course.review_count / 10, 5); | |
| return acc; | |
| }, {} as Record<string, number>), | |
| }, | |
| ]; | |
| // Data for Credits Comparison | |
| const creditsData = courses.map((course, idx) => ({ | |
| name: course.code, | |
| credits: course.credits, | |
| fill: colors[idx % colors.length], | |
| })); | |
| return ( | |
| <div className="space-y-6"> | |
| {/* Ratings Comparison Bar Chart */} | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardHeader> | |
| <CardTitle className="flex items-center gap-2"> | |
| 📊 Ratings Comparison | |
| </CardTitle> | |
| <p className="text-sm text-muted-foreground"> | |
| Compare overall rating, difficulty, and workload across courses | |
| </p> | |
| </CardHeader> | |
| <CardContent> | |
| <ResponsiveContainer width="100%" height={300}> | |
| <BarChart data={ratingsData}> | |
| <CartesianGrid strokeDasharray="3 3" className="stroke-muted" /> | |
| <XAxis | |
| dataKey="metric" | |
| className="text-xs" | |
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | |
| /> | |
| <YAxis | |
| domain={[0, 5]} | |
| className="text-xs" | |
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | |
| /> | |
| <Tooltip | |
| contentStyle={{ | |
| backgroundColor: 'hsl(var(--card))', | |
| border: '1px solid hsl(var(--border))', | |
| borderRadius: '0.5rem', | |
| }} | |
| /> | |
| <Legend /> | |
| {courses.map((course, idx) => ( | |
| <Bar | |
| key={course.id} | |
| dataKey={course.code} | |
| fill={colors[idx % colors.length]} | |
| radius={[4, 4, 0, 0]} | |
| /> | |
| ))} | |
| </BarChart> | |
| </ResponsiveContainer> | |
| </CardContent> | |
| </Card> | |
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> | |
| {/* Radar Chart */} | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardHeader> | |
| <CardTitle className="flex items-center gap-2"> | |
| 🎯 Multi-Metric Radar | |
| </CardTitle> | |
| <p className="text-sm text-muted-foreground"> | |
| Visual overview of all metrics | |
| </p> | |
| </CardHeader> | |
| <CardContent> | |
| <ResponsiveContainer width="100%" height={300}> | |
| <RadarChart data={radarData}> | |
| <PolarGrid className="stroke-muted" /> | |
| <PolarAngleAxis | |
| dataKey="metric" | |
| tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 12 }} | |
| /> | |
| <PolarRadiusAxis | |
| angle={90} | |
| domain={[0, 5]} | |
| tick={{ fill: 'hsl(var(--muted-foreground))', fontSize: 10 }} | |
| /> | |
| <Tooltip | |
| contentStyle={{ | |
| backgroundColor: 'hsl(var(--card))', | |
| border: '1px solid hsl(var(--border))', | |
| borderRadius: '0.5rem', | |
| }} | |
| /> | |
| {courses.map((course, idx) => ( | |
| <Radar | |
| key={course.id} | |
| name={course.code} | |
| dataKey={course.code} | |
| stroke={colors[idx % colors.length]} | |
| fill={colors[idx % colors.length]} | |
| fillOpacity={0.3} | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
In `@src/components/courses/compare/ComparisonCharts.tsx` around lines 69 - 173,
The RadarChart is rendering duplicate series because each <Radar> uses the same
dataKey="overall" and radarData is shaped per course; reshape radarData to be
one object per metric (e.g., { metric: 'Overall', [courseCodeA]: val,
[courseCodeB]: val, ... } for overall, difficulty, workload, reviews), update
PolarAngleAxis to use dataKey="metric", and change each <Radar> in the
courses.map loop to use dataKey={course.code} (and keep stroke/fill using
colors[idx % colors.length]) so each series reads its own course field instead
of the shared "overall".
Comment on lines
+69
to
+74
| <button | ||
| onClick={() => handleRemoveCourse(course.id)} | ||
| className="ml-1 hover:text-destructive transition-colors" | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label to the remove button.
Icon‑only buttons need an aria‑label (and type="button" to avoid accidental form submits).
Proposed fix
- <button
- onClick={() => handleRemoveCourse(course.id)}
- className="ml-1 hover:text-destructive transition-colors"
- >
+ <button
+ type="button"
+ aria-label={`Remove ${course.code} from comparison`}
+ onClick={() => handleRemoveCourse(course.id)}
+ className="ml-1 hover:text-destructive transition-colors"
+ >
<X className="h-3 w-3" />
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| onClick={() => handleRemoveCourse(course.id)} | |
| className="ml-1 hover:text-destructive transition-colors" | |
| > | |
| <X className="h-3 w-3" /> | |
| </button> | |
| <button | |
| type="button" | |
| aria-label={`Remove ${course.code} from comparison`} | |
| onClick={() => handleRemoveCourse(course.id)} | |
| className="ml-1 hover:text-destructive transition-colors" | |
| > | |
| <X className="h-3 w-3" /> | |
| </button> |
🤖 Prompt for AI Agents
In `@src/components/courses/compare/CourseSelector.tsx` around lines 69 - 74, The
remove-course button in CourseSelector is an icon-only control and needs
accessibility and form-safety fixes: update the button element used with
onClick={() => handleRemoveCourse(course.id)} to include type="button" and an
appropriate aria-label (e.g., aria-label={`Remove ${course.name}` or a generic
"Remove course") so screen readers can announce its purpose; locate the button
in CourseSelector.tsx where handleRemoveCourse is invoked and add those
attributes.
Comment on lines
+24
to
+147
| export default function ReviewHighlights({ courses }: ReviewHighlightsProps) { | ||
| const [reviewsData, setReviewsData] = useState<Record<string, Review[]>>({}); | ||
| const [loading, setLoading] = useState(true); | ||
|
|
||
| useEffect(() => { | ||
| const fetchReviews = async () => { | ||
| if (courses.length === 0) { | ||
| setLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| setLoading(true); | ||
| const reviewsByUUID: Record<string, Review[]> = {}; | ||
|
|
||
| // Fetch UUIDs for all course codes | ||
| const { data: courseUUIDs, error: uuidError } = await supabase | ||
| .from('courses') | ||
| .select('id, code') | ||
| .in('code', courses.map((c) => c.code.toUpperCase())); | ||
|
|
||
| if (uuidError) { | ||
| console.error('Error fetching course UUIDs:', uuidError); | ||
| setLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| // Create a map of course code to UUID | ||
| const codeToUUID: Record<string, string> = {}; | ||
| courseUUIDs?.forEach((c) => { | ||
| codeToUUID[c.code] = c.id; | ||
| }); | ||
|
|
||
| // Fetch reviews for each course UUID | ||
| for (const course of courses) { | ||
| const uuid = codeToUUID[course.code.toUpperCase()]; | ||
| if (!uuid) continue; | ||
|
|
||
| const { data, error } = await supabase | ||
| .from('reviews') | ||
| .select('id, comment, rating_value, difficulty_rating, workload_rating, created_at') | ||
| .eq('target_id', uuid) | ||
| .eq('target_type', 'course') | ||
| .not('comment', 'is', null) | ||
| .order('created_at', { ascending: false }) | ||
| .limit(10); | ||
|
|
||
| if (!error && data) { | ||
| reviewsByUUID[course.id] = data; | ||
| } | ||
| } | ||
|
|
||
| setReviewsData(reviewsByUUID); | ||
| setLoading(false); | ||
| }; | ||
|
|
||
| fetchReviews(); | ||
| }, [courses]); | ||
|
|
||
| const analyzeReviews = (reviews: Review[]) => { | ||
| if (!reviews || reviews.length === 0) { | ||
| return { pros: [], cons: [], topReviews: [] }; | ||
| } | ||
|
|
||
| const positiveReviews = reviews.filter((r) => r.rating_value >= 4); | ||
| const negativeReviews = reviews.filter((r) => r.rating_value <= 2); | ||
|
|
||
| // Get top positive comments | ||
| const pros = positiveReviews | ||
| .filter((r) => r.comment && r.comment.trim().length > 20) | ||
| .slice(0, 3) | ||
| .map((r) => r.comment); | ||
|
|
||
| // Get top negative comments | ||
| const cons = negativeReviews | ||
| .filter((r) => r.comment && r.comment.trim().length > 20) | ||
| .slice(0, 3) | ||
| .map((r) => r.comment); | ||
|
|
||
| // Get most recent reviews | ||
| const topReviews = reviews | ||
| .filter((r) => r.comment && r.comment.trim().length > 20) | ||
| .slice(0, 3); | ||
|
|
||
| return { pros, cons, topReviews }; | ||
| }; | ||
|
|
||
| if (courses.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| if (loading) { | ||
| return ( | ||
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | ||
| <CardContent className="flex items-center justify-center py-12"> | ||
| <div className="text-center space-y-2"> | ||
| <Loader2 className="h-8 w-8 animate-spin mx-auto text-primary" /> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Loading review highlights... | ||
| </p> | ||
| </div> | ||
| </CardContent> | ||
| </Card> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | ||
| <CardHeader> | ||
| <CardTitle className="flex items-center gap-2"> | ||
| 💬 Review Highlights | ||
| </CardTitle> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Key insights from student reviews for each course | ||
| </p> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <Tabs defaultValue={courses[0]?.id} className="w-full"> | ||
| <TabsList className="grid w-full" style={{ gridTemplateColumns: `repeat(${courses.length}, 1fr)` }}> | ||
| {courses.map((course) => ( | ||
| <TabsTrigger key={course.id} value={course.id} className="font-mono text-xs"> | ||
| {course.code} | ||
| </TabsTrigger> | ||
| ))} | ||
| </TabsList> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full ReviewHighlights.tsx file
cat -n src/components/courses/compare/ReviewHighlights.tsxRepository: OpenLake/RateMyCourse
Length of output: 11423
🏁 Script executed:
# Also check what Tabs component is being used (likely from shadcn/ui)
fd -e tsx -e ts | xargs rg -l "export.*Tabs" | head -5Repository: OpenLake/RateMyCourse
Length of output: 91
🌐 Web query:
Radix UI Tabs defaultValue behavior when props change
💡 Result:
In Radix UI Tabs, defaultValue is for uncontrolled usage: it’s only used to pick the initially active tab on the first render. After the component mounts, changing the defaultValue prop won’t update the selected tab. (radix-ui.com)
If you need the active tab to change when your props/state change, make the tabs controlled:
- Use
value={...}(derived from your props/state) - Update it via
onValueChange(radix-ui.com)
Workarounds if you must stay uncontrolled:
- Force a remount when the “default” should change (e.g., set a changing
keyonTabs.Root).
Citations:
- 1: https://www.radix-ui.com/primitives/docs/components/tabs?utm_source=openai
- 2: https://www.radix-ui.com/primitives/docs/components/tabs?utm_source=openai
Tabs can become stale when the course list changes.
When courses changes, the defaultValue prop doesn't update the active tab because Radix UI only reads it on initial render. If the previously selected course is removed from the list, the tab will point to a non-existent id and show empty content. Use controlled Tabs with state and reset when the list changes.
✅ Suggested fix (controlled Tabs)
- const [reviewsData, setReviewsData] = useState<Record<string, Review[]>>({});
- const [loading, setLoading] = useState(true);
+ const [reviewsData, setReviewsData] = useState<Record<string, Review[]>>({});
+ const [loading, setLoading] = useState(true);
+ const [activeTab, setActiveTab] = useState<string | undefined>(courses[0]?.id);
+ useEffect(() => {
+ if (courses.length === 0) return;
+ setActiveTab((prev) =>
+ prev && courses.some((c) => c.id === prev) ? prev : courses[0].id
+ );
+ }, [courses]);
...
- <Tabs defaultValue={courses[0]?.id} className="w-full">
+ <Tabs value={activeTab} onValueChange={setActiveTab} className="w-full">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export default function ReviewHighlights({ courses }: ReviewHighlightsProps) { | |
| const [reviewsData, setReviewsData] = useState<Record<string, Review[]>>({}); | |
| const [loading, setLoading] = useState(true); | |
| useEffect(() => { | |
| const fetchReviews = async () => { | |
| if (courses.length === 0) { | |
| setLoading(false); | |
| return; | |
| } | |
| setLoading(true); | |
| const reviewsByUUID: Record<string, Review[]> = {}; | |
| // Fetch UUIDs for all course codes | |
| const { data: courseUUIDs, error: uuidError } = await supabase | |
| .from('courses') | |
| .select('id, code') | |
| .in('code', courses.map((c) => c.code.toUpperCase())); | |
| if (uuidError) { | |
| console.error('Error fetching course UUIDs:', uuidError); | |
| setLoading(false); | |
| return; | |
| } | |
| // Create a map of course code to UUID | |
| const codeToUUID: Record<string, string> = {}; | |
| courseUUIDs?.forEach((c) => { | |
| codeToUUID[c.code] = c.id; | |
| }); | |
| // Fetch reviews for each course UUID | |
| for (const course of courses) { | |
| const uuid = codeToUUID[course.code.toUpperCase()]; | |
| if (!uuid) continue; | |
| const { data, error } = await supabase | |
| .from('reviews') | |
| .select('id, comment, rating_value, difficulty_rating, workload_rating, created_at') | |
| .eq('target_id', uuid) | |
| .eq('target_type', 'course') | |
| .not('comment', 'is', null) | |
| .order('created_at', { ascending: false }) | |
| .limit(10); | |
| if (!error && data) { | |
| reviewsByUUID[course.id] = data; | |
| } | |
| } | |
| setReviewsData(reviewsByUUID); | |
| setLoading(false); | |
| }; | |
| fetchReviews(); | |
| }, [courses]); | |
| const analyzeReviews = (reviews: Review[]) => { | |
| if (!reviews || reviews.length === 0) { | |
| return { pros: [], cons: [], topReviews: [] }; | |
| } | |
| const positiveReviews = reviews.filter((r) => r.rating_value >= 4); | |
| const negativeReviews = reviews.filter((r) => r.rating_value <= 2); | |
| // Get top positive comments | |
| const pros = positiveReviews | |
| .filter((r) => r.comment && r.comment.trim().length > 20) | |
| .slice(0, 3) | |
| .map((r) => r.comment); | |
| // Get top negative comments | |
| const cons = negativeReviews | |
| .filter((r) => r.comment && r.comment.trim().length > 20) | |
| .slice(0, 3) | |
| .map((r) => r.comment); | |
| // Get most recent reviews | |
| const topReviews = reviews | |
| .filter((r) => r.comment && r.comment.trim().length > 20) | |
| .slice(0, 3); | |
| return { pros, cons, topReviews }; | |
| }; | |
| if (courses.length === 0) { | |
| return null; | |
| } | |
| if (loading) { | |
| return ( | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardContent className="flex items-center justify-center py-12"> | |
| <div className="text-center space-y-2"> | |
| <Loader2 className="h-8 w-8 animate-spin mx-auto text-primary" /> | |
| <p className="text-sm text-muted-foreground"> | |
| Loading review highlights... | |
| </p> | |
| </div> | |
| </CardContent> | |
| </Card> | |
| ); | |
| } | |
| return ( | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardHeader> | |
| <CardTitle className="flex items-center gap-2"> | |
| 💬 Review Highlights | |
| </CardTitle> | |
| <p className="text-sm text-muted-foreground"> | |
| Key insights from student reviews for each course | |
| </p> | |
| </CardHeader> | |
| <CardContent> | |
| <Tabs defaultValue={courses[0]?.id} className="w-full"> | |
| <TabsList className="grid w-full" style={{ gridTemplateColumns: `repeat(${courses.length}, 1fr)` }}> | |
| {courses.map((course) => ( | |
| <TabsTrigger key={course.id} value={course.id} className="font-mono text-xs"> | |
| {course.code} | |
| </TabsTrigger> | |
| ))} | |
| </TabsList> | |
| export default function ReviewHighlights({ courses }: ReviewHighlightsProps) { | |
| const [reviewsData, setReviewsData] = useState<Record<string, Review[]>>({}); | |
| const [loading, setLoading] = useState(true); | |
| const [activeTab, setActiveTab] = useState<string | undefined>(courses[0]?.id); | |
| useEffect(() => { | |
| if (courses.length === 0) return; | |
| setActiveTab((prev) => | |
| prev && courses.some((c) => c.id === prev) ? prev : courses[0].id | |
| ); | |
| }, [courses]); | |
| useEffect(() => { | |
| const fetchReviews = async () => { | |
| if (courses.length === 0) { | |
| setLoading(false); | |
| return; | |
| } | |
| setLoading(true); | |
| const reviewsByUUID: Record<string, Review[]> = {}; | |
| // Fetch UUIDs for all course codes | |
| const { data: courseUUIDs, error: uuidError } = await supabase | |
| .from('courses') | |
| .select('id, code') | |
| .in('code', courses.map((c) => c.code.toUpperCase())); | |
| if (uuidError) { | |
| console.error('Error fetching course UUIDs:', uuidError); | |
| setLoading(false); | |
| return; | |
| } | |
| // Create a map of course code to UUID | |
| const codeToUUID: Record<string, string> = {}; | |
| courseUUIDs?.forEach((c) => { | |
| codeToUUID[c.code] = c.id; | |
| }); | |
| // Fetch reviews for each course UUID | |
| for (const course of courses) { | |
| const uuid = codeToUUID[course.code.toUpperCase()]; | |
| if (!uuid) continue; | |
| const { data, error } = await supabase | |
| .from('reviews') | |
| .select('id, comment, rating_value, difficulty_rating, workload_rating, created_at') | |
| .eq('target_id', uuid) | |
| .eq('target_type', 'course') | |
| .not('comment', 'is', null) | |
| .order('created_at', { ascending: false }) | |
| .limit(10); | |
| if (!error && data) { | |
| reviewsByUUID[course.id] = data; | |
| } | |
| } | |
| setReviewsData(reviewsByUUID); | |
| setLoading(false); | |
| }; | |
| fetchReviews(); | |
| }, [courses]); | |
| const analyzeReviews = (reviews: Review[]) => { | |
| if (!reviews || reviews.length === 0) { | |
| return { pros: [], cons: [], topReviews: [] }; | |
| } | |
| const positiveReviews = reviews.filter((r) => r.rating_value >= 4); | |
| const negativeReviews = reviews.filter((r) => r.rating_value <= 2); | |
| // Get top positive comments | |
| const pros = positiveReviews | |
| .filter((r) => r.comment && r.comment.trim().length > 20) | |
| .slice(0, 3) | |
| .map((r) => r.comment); | |
| // Get top negative comments | |
| const cons = negativeReviews | |
| .filter((r) => r.comment && r.comment.trim().length > 20) | |
| .slice(0, 3) | |
| .map((r) => r.comment); | |
| // Get most recent reviews | |
| const topReviews = reviews | |
| .filter((r) => r.comment && r.comment.trim().length > 20) | |
| .slice(0, 3); | |
| return { pros, cons, topReviews }; | |
| }; | |
| if (courses.length === 0) { | |
| return null; | |
| } | |
| if (loading) { | |
| return ( | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardContent className="flex items-center justify-center py-12"> | |
| <div className="text-center space-y-2"> | |
| <Loader2 className="h-8 w-8 animate-spin mx-auto text-primary" /> | |
| <p className="text-sm text-muted-foreground"> | |
| Loading review highlights... | |
| </p> | |
| </div> | |
| </CardContent> | |
| </Card> | |
| ); | |
| } | |
| return ( | |
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | |
| <CardHeader> | |
| <CardTitle className="flex items-center gap-2"> | |
| 💬 Review Highlights | |
| </CardTitle> | |
| <p className="text-sm text-muted-foreground"> | |
| Key insights from student reviews for each course | |
| </p> | |
| </CardHeader> | |
| <CardContent> | |
| <Tabs value={activeTab} onValueChange={setActiveTab} className="w-full"> | |
| <TabsList className="grid w-full" style={{ gridTemplateColumns: `repeat(${courses.length}, 1fr)` }}> | |
| {courses.map((course) => ( | |
| <TabsTrigger key={course.id} value={course.id} className="font-mono text-xs"> | |
| {course.code} | |
| </TabsTrigger> | |
| ))} | |
| </TabsList> |
🤖 Prompt for AI Agents
In `@src/components/courses/compare/ReviewHighlights.tsx` around lines 24 - 147,
The Tabs use defaultValue so the active tab doesn't update when courses changes;
convert to a controlled Tabs by adding state (e.g., const [activeTab,
setActiveTab] = useState<string | null>(courses[0]?.id || null)) and pass
value={activeTab} and onValueChange={setActiveTab} to the Tabs component, update
TabsTrigger to use course.id as before, and add a useEffect watching courses to
reset activeTab to courses[0]?.id (or null) when the list changes so removed
course ids don't leave the Tabs pointing at a stale/non-existent id.
Comment on lines
+25
to
+29
| const getComparisonList = (): string[] => { | ||
| if (typeof window === 'undefined') return []; | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| return stored ? JSON.parse(stored) : []; | ||
| }; |
There was a problem hiding this comment.
Add error handling for JSON.parse to prevent runtime exceptions.
If localStorage contains malformed JSON (e.g., from manual tampering or storage corruption), JSON.parse will throw. Consider wrapping in a try-catch with a fallback.
🛠️ Suggested fix
const getComparisonList = (): string[] => {
if (typeof window === 'undefined') return [];
const stored = localStorage.getItem(STORAGE_KEY);
- return stored ? JSON.parse(stored) : [];
+ if (!stored) return [];
+ try {
+ const parsed = JSON.parse(stored);
+ return Array.isArray(parsed) ? parsed : [];
+ } catch {
+ return [];
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const getComparisonList = (): string[] => { | |
| if (typeof window === 'undefined') return []; | |
| const stored = localStorage.getItem(STORAGE_KEY); | |
| return stored ? JSON.parse(stored) : []; | |
| }; | |
| const getComparisonList = (): string[] => { | |
| if (typeof window === 'undefined') return []; | |
| const stored = localStorage.getItem(STORAGE_KEY); | |
| if (!stored) return []; | |
| try { | |
| const parsed = JSON.parse(stored); | |
| return Array.isArray(parsed) ? parsed : []; | |
| } catch { | |
| return []; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@src/components/courses/CompareButton.tsx` around lines 25 - 29, The
getComparisonList function should guard against malformed JSON from localStorage
by wrapping the JSON.parse call in a try-catch: after retrieving stored =
localStorage.getItem(STORAGE_KEY), attempt to parse it inside try and on success
return the parsed array; on catch return an empty array (optionally remove the
bad key with localStorage.removeItem(STORAGE_KEY) or log the error) so that
JSON.parse errors cannot throw at runtime.
Comment on lines
+174
to
+176
| <Badge variant="outline" className="text-xs"> | ||
| ⭐ {course.overall_rating.toFixed(1)} | ||
| </Badge> |
There was a problem hiding this comment.
Add defensive check for overall_rating to prevent potential runtime error.
If course.overall_rating is undefined or null, calling .toFixed(1) will throw. Consider adding a fallback.
🛠️ Suggested fix
<Badge variant="outline" className="text-xs">
- ⭐ {course.overall_rating.toFixed(1)}
+ ⭐ {(course.overall_rating ?? 0).toFixed(1)}
</Badge>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Badge variant="outline" className="text-xs"> | |
| ⭐ {course.overall_rating.toFixed(1)} | |
| </Badge> | |
| <Badge variant="outline" className="text-xs"> | |
| ⭐ {(course.overall_rating ?? 0).toFixed(1)} | |
| </Badge> |
🤖 Prompt for AI Agents
In `@src/components/courses/CompareButton.tsx` around lines 174 - 176, The Badge
rendering calls course.overall_rating.toFixed(1) which will throw if
overall_rating is null/undefined; update the rendering around the Badge (the
block with Badge and course.overall_rating) to guard against nullish values by
using a safe fallback such as const displayRating = (typeof
course.overall_rating === 'number') ? course.overall_rating.toFixed(1) : '–' (or
'N/A' / '0.0') and then render "⭐ {displayRating}", or conditionally render the
Badge only when course.overall_rating is a number; reference the Badge component
and course.overall_rating to locate where to apply the change.
Comment on lines
+11
to
+15
| const getComparisonList = (): string[] => { | ||
| if (typeof window === 'undefined') return []; | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| return stored ? JSON.parse(stored) : []; | ||
| }; |
There was a problem hiding this comment.
Guard against malformed localStorage data.
JSON.parse can throw (e.g., corrupted or legacy values), which would crash the component.
Proposed fix
const getComparisonList = (): string[] => {
if (typeof window === 'undefined') return [];
const stored = localStorage.getItem(STORAGE_KEY);
- return stored ? JSON.parse(stored) : [];
+ if (!stored) return [];
+ try {
+ const parsed = JSON.parse(stored);
+ return Array.isArray(parsed) ? parsed : [];
+ } catch {
+ return [];
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const getComparisonList = (): string[] => { | |
| if (typeof window === 'undefined') return []; | |
| const stored = localStorage.getItem(STORAGE_KEY); | |
| return stored ? JSON.parse(stored) : []; | |
| }; | |
| const getComparisonList = (): string[] => { | |
| if (typeof window === 'undefined') return []; | |
| const stored = localStorage.getItem(STORAGE_KEY); | |
| if (!stored) return []; | |
| try { | |
| const parsed = JSON.parse(stored); | |
| return Array.isArray(parsed) ? parsed : []; | |
| } catch { | |
| return []; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@src/components/courses/CompareCoursesButton.tsx` around lines 11 - 15,
getComparisonList currently calls JSON.parse on localStorage data which can
throw on malformed values; wrap the parse in a try/catch in getComparisonList
(and keep the existing typeof window guard) so parsing failures return an empty
array instead of crashing, and after parsing validate the result is an array of
strings (otherwise return []); reference getComparisonList and STORAGE_KEY when
making this change.
Comment on lines
+16
to
+20
| const getComparisonList = (): string[] => { | ||
| if (typeof window === 'undefined') return []; | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| return stored ? JSON.parse(stored) : []; | ||
| }; |
There was a problem hiding this comment.
Harden comparison list parsing.
A malformed localStorage value can throw and break the compare UI state.
Proposed fix
const getComparisonList = (): string[] => {
if (typeof window === 'undefined') return [];
const stored = localStorage.getItem(STORAGE_KEY);
- return stored ? JSON.parse(stored) : [];
+ if (!stored) return [];
+ try {
+ const parsed = JSON.parse(stored);
+ return Array.isArray(parsed) ? parsed : [];
+ } catch {
+ return [];
+ }
};🤖 Prompt for AI Agents
In `@src/components/courses/course_page/AddToComparison.tsx` around lines 16 - 20,
The getComparisonList function can throw when localStorage contains malformed
JSON; wrap the localStorage access/JSON.parse in a try/catch and return an empty
array on any error, and additionally validate the parsed value is an array of
strings before returning it. Specifically update getComparisonList to: check
typeof window, read localStorage.getItem(STORAGE_KEY) inside try, JSON.parse
safely, ensure the result is an Array and every item is a string (otherwise
fallback), and swallow/log the error then return [].
There was a problem hiding this comment.
Pull request overview
Adds a new course comparison experience so students can select (up to 4) courses and view side-by-side metrics, charts, and review highlights (Fixes #49).
Changes:
- Added
/courses/comparepage with selector-driven comparison table, charts, and review highlights. - Added “compare” entry points from the navbar, course listing cards, and course detail sidebar (with localStorage persistence).
- Added documentation describing the feature and user flows.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/layout/Navbar.tsx | Adds navbar entry point to the comparison page. |
| src/components/courses/course_page/AddToComparison.tsx | Adds course-detail sidebar card to add/remove and navigate to comparison. |
| src/components/courses/compare/ReviewHighlights.tsx | Adds per-course review fetching and highlighting UI (tabs). |
| src/components/courses/compare/CourseSelector.tsx | Adds course search/selection UI for choosing courses to compare. |
| src/components/courses/compare/ComparisonTable.tsx | Adds side-by-side comparison table for key metrics. |
| src/components/courses/compare/ComparisonCharts.tsx | Adds charts for ratings/radar/credits comparisons. |
| src/components/courses/ItemCard.tsx | Adds compare button on course cards in the listing UI. |
| src/components/courses/CompareCoursesButton.tsx | Adds a sidebar button to open the comparison flow based on stored selections. |
| src/components/courses/CompareButton.tsx | Adds add/remove compare control + floating comparison drawer button. |
| src/app/courses/page.tsx | Integrates compare sidebar button and floating compare drawer on courses page. |
| src/app/courses/compare/page.tsx | Implements the main compare page and selection-driven rendering. |
| src/app/courses/[courseId]/page.tsx | Integrates “Add to Comparison” card into course detail page. |
| docs/COURSE_COMPARISON_FEATURE.md | Documents feature behavior, user flows, and implementation notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 6. Automatically redirected to comparison page with selected courses | ||
|
|
||
| ### Method 3: Direct URL | ||
| Navigate to `/courses/compare?courses=courseId1,courseId2,courseId3` |
There was a problem hiding this comment.
The compare page currently preselects by matching courses query values against Course.id (which useCourses populates from the JSON id, e.g. “mal401”), not by display codes like “MAL401”. The direct-URL examples here should be updated to use the actual IDs accepted by /courses/compare?courses=... (or the page should be updated to accept course codes consistently).
Suggested change
| Navigate to `/courses/compare?courses=courseId1,courseId2,courseId3` | |
| Navigate to `/courses/compare?courses=<courseId1>,<courseId2>,<courseId3>` using the underlying course IDs (the JSON `id` / `Course.id`, e.g. `mal401`), not the display codes like `MAL401`. | |
| For example: `/courses/compare?courses=mal401,cs101,math200` |
Comment on lines
+88
to
+92
| #### 5. `/src/components/courses/compare/ReviewHighlights.tsx` | ||
| Review analysis component: | ||
| - Fetches reviews from Supabase | ||
| - Analyzes sentiment based on ratings | ||
| - Categorizes into pros/cons |
There was a problem hiding this comment.
The PR description mentions “automated sentiment analysis”, but the implementation (and this doc) classify reviews purely by rating thresholds (e.g., ≥4 / ≤2). Please either adjust the description/docs to reflect rating-based categorization, or implement actual sentiment analysis as described.
Comment on lines
+87
to
+89
| const positiveReviews = reviews.filter((r) => r.rating_value >= 4); | ||
| const negativeReviews = reviews.filter((r) => r.rating_value <= 2); | ||
|
|
There was a problem hiding this comment.
This “analysis” is rating-threshold based (≥4 / ≤2), not sentiment analysis. Please either rename/reword to avoid implying NLP sentiment scoring, or implement actual sentiment analysis if that’s a requirement in the PR description.
Comment on lines
186
to
+191
| <CardFooter className="px-4 py-3 bg-background/40 backdrop-blur-sm border-t border-border/40 flex justify-between items-center group-hover:bg-primary/5 transition-all duration-300"> | ||
| <div className="text-xs font-bold font-mono tracking-wide text-primary"> | ||
| View Details | ||
| </div> | ||
| <div className="p-1.5 rounded-md bg-primary/10 group-hover:bg-primary/20 group-hover:translate-x-1 transition-all duration-300"> | ||
| <ChevronRight className="h-3.5 w-3.5 text-primary" /> | ||
| </div> | ||
| {type === 'course' ? ( | ||
| <> | ||
| <CompareButton course={item as Course} /> | ||
| <div className="flex items-center gap-2"> |
There was a problem hiding this comment.
CompareButton renders a <button> inside the <Link> wrapper for the whole card, which creates nested interactive elements (<a> containing <button>). This is invalid HTML and can break keyboard/screen-reader interaction. Consider restructuring so the card isn’t a single link (e.g., make only the “View Details” area a link), or render the compare control outside the link wrapper.
Comment on lines
+17
to
+19
| const searchParams = useSearchParams(); | ||
| const { courses, isLoading } = useCourses(); | ||
|
|
There was a problem hiding this comment.
useCourses() is used here for preselecting from URL params, but CourseSelector also calls useCourses() internally. Because useCourses performs a full course load + per-course dynamic fetches, the compare page will refetch everything twice. Consider passing courses down into CourseSelector instead of having it call useCourses again (or add caching to useCourses).
| <span className="max-w-[200px] truncate">{course.title}</span> | ||
| <button | ||
| onClick={() => handleRemoveCourse(course.id)} | ||
| className="ml-1 hover:text-destructive transition-colors" |
There was a problem hiding this comment.
The remove-course control is an icon-only <button> with no accessible name. Please add an aria-label (e.g., “Remove from comparison”) so screen readers can announce what the control does.
Suggested change
| className="ml-1 hover:text-destructive transition-colors" | |
| className="ml-1 hover:text-destructive transition-colors" | |
| aria-label={`Remove ${course.code} from comparison`} |
Comment on lines
+20
to
+22
| import { Check, ChevronsUpDown, Plus, X } from 'lucide-react'; | ||
| import { cn } from '@/lib/utils'; | ||
| import { Badge } from '@/components/ui/badge'; |
There was a problem hiding this comment.
Unused imports in this file (useEffect, Check, and cn) are likely to fail next lint’s unused-vars/unused-imports rules. Please remove them (or use them).
| @@ -0,0 +1,152 @@ | |||
| 'use client'; | |||
|
|
|||
| import { useState, useEffect } from 'react'; | |||
There was a problem hiding this comment.
useEffect is imported but not used in this component. Please remove it to avoid unused-import lint failures.
Suggested change
| import { useState, useEffect } from 'react'; | |
| import { useState } from 'react'; |
Comment on lines
+40
to
+46
| const [comparisonCount, setComparisonCount] = useState(0); | ||
|
|
||
| useEffect(() => { | ||
| const updateState = () => { | ||
| const list = getComparisonList(); | ||
| setIsInComparison(list.includes(course.id)); | ||
| setComparisonCount(list.length); |
There was a problem hiding this comment.
Unused variable comparisonCount.
Suggested change
| const [comparisonCount, setComparisonCount] = useState(0); | |
| useEffect(() => { | |
| const updateState = () => { | |
| const list = getComparisonList(); | |
| setIsInComparison(list.includes(course.id)); | |
| setComparisonCount(list.length); | |
| useEffect(() => { | |
| const updateState = () => { | |
| const list = getComparisonList(); | |
| setIsInComparison(list.includes(course.id)); |
| return null; | ||
| } | ||
|
|
||
| const getDepartmentColor = (department: string) => { |
There was a problem hiding this comment.
Unused variable getDepartmentColor.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/courses/compare/page.tsx (1)
21-28:⚠️ Potential issue | 🟡 MinorMake query preselection resilient to casing and identifier format.
At Line 23–27, preselection only does case-sensitive
c.idmatching, so links using course codes (or different casing/whitespace) can fail silently.🔧 Proposed fix
useEffect(() => { if (!isLoading && courses.length > 0 && searchParams) { - const courseIds = searchParams.get('courses')?.split(',') || []; - if (courseIds.length > 0) { - const preselectedCourses = courses.filter((c) => - courseIds.includes(c.id) - ); + const rawIds = searchParams.get('courses')?.split(',') || []; + const normalizedIds = rawIds + .map((id) => id.trim().toLowerCase()) + .filter(Boolean); + + if (normalizedIds.length > 0) { + const preselectedCourses = courses.filter( + (c) => + normalizedIds.includes(c.id.toLowerCase()) || + normalizedIds.includes(c.code.toLowerCase()) + ); setSelectedCourses(preselectedCourses.slice(0, 4)); } } }, [searchParams, courses, isLoading]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/courses/compare/page.tsx` around lines 21 - 28, The preselection logic in the useEffect is doing case-sensitive matching on c.id and can miss matches with different casing/whitespace or when a course code is supplied; normalize both the incoming query values and course identifiers before comparing. Trim and lower-case each value from searchParams.get('courses') after splitting, and compare against normalized forms of course properties (e.g., c.id and c.code converted with .trim().toLowerCase()) when building preselectedCourses, then call setSelectedCourses(preselectedCourses.slice(0, 4)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/courses/compare/ComparisonCharts.tsx`:
- Around line 38-56: The metric objects in ComparisonCharts.tsx currently use
course.code as the series key (in the three reduce blocks that build
overall_rating, difficulty_rating, workload_rating), which can collide for
cross‑listed/duplicate codes; change the key to a guaranteed-unique identifier
(e.g., course.id or a composite like `${course.code}-${course.id}`) when
populating the reduce accumulators and ensure the same unique key is used where
the series are rendered (the code that iterates/matches series around the render
block at lines ~111-117) so series no longer collide.
- Around line 77-173: The PR is missing the multi-metric radar chart and credits
comparison visuals; update the ComparisonCharts component to render a RadarChart
(inside ResponsiveContainer) using the existing ratingsData and courses mapping:
add Recharts components RadarChart, Radar, PolarGrid, PolarAngleAxis,
PolarRadiusAxis and map course.code -> Radar dataKey with colors[idx %
colors.length], and also render a separate credits comparison chart (e.g., a
small BarChart or ColumnBar) using a creditsData array derived from courses
(course.code and course.credits) with XAxis dataKey="code" and YAxis domain
derived from credits, reusing ResponsiveContainer and colors for bars; ensure
imports for RadarChart/Radar/Polar* and any creditsData preparation are added
and that keys/ids use course.id/course.code consistently to avoid rendering
collisions.
---
Duplicate comments:
In `@src/app/courses/compare/page.tsx`:
- Around line 21-28: The preselection logic in the useEffect is doing
case-sensitive matching on c.id and can miss matches with different
casing/whitespace or when a course code is supplied; normalize both the incoming
query values and course identifiers before comparing. Trim and lower-case each
value from searchParams.get('courses') after splitting, and compare against
normalized forms of course properties (e.g., c.id and c.code converted with
.trim().toLowerCase()) when building preselectedCourses, then call
setSelectedCourses(preselectedCourses.slice(0, 4)).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3556629b-82a0-47ad-934f-59bbe7e33bbe
📒 Files selected for processing (2)
src/app/courses/compare/page.tsxsrc/components/courses/compare/ComparisonCharts.tsx
Comment on lines
+77
to
+173
| return ( | ||
| <div className="space-y-6"> | ||
| {/* Ratings Comparison Bar Chart */} | ||
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | ||
| <CardHeader> | ||
| <CardTitle className="flex items-center gap-2"> | ||
| 📊 Ratings Comparison | ||
| </CardTitle> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Compare overall rating, difficulty, and workload across courses | ||
| </p> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <ResponsiveContainer width="100%" height={300}> | ||
| <BarChart data={ratingsData}> | ||
| <CartesianGrid strokeDasharray="3 3" className="stroke-muted" /> | ||
| <XAxis | ||
| dataKey="metric" | ||
| className="text-xs" | ||
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | ||
| /> | ||
| <YAxis | ||
| domain={[0, 5]} | ||
| className="text-xs" | ||
| tick={{ fill: 'hsl(var(--muted-foreground))' }} | ||
| /> | ||
| <Tooltip | ||
| contentStyle={{ | ||
| backgroundColor: 'hsl(var(--card))', | ||
| border: '1px solid hsl(var(--border))', | ||
| borderRadius: '0.5rem', | ||
| }} | ||
| /> | ||
| <Legend /> | ||
| {courses.map((course, idx) => ( | ||
| <Bar | ||
| key={course.id} | ||
| dataKey={course.code} | ||
| fill={colors[idx % colors.length]} | ||
| radius={[4, 4, 0, 0]} | ||
| /> | ||
| ))} | ||
| </BarChart> | ||
| </ResponsiveContainer> | ||
| </CardContent> | ||
| </Card> | ||
|
|
||
| <Card className="border-border/60 bg-card/40 backdrop-blur-sm"> | ||
| <CardHeader> | ||
| <CardTitle className="flex items-center gap-2"> | ||
| ✨ Quick Takeaways | ||
| </CardTitle> | ||
| <p className="text-sm text-muted-foreground"> | ||
| A fast summary of which course leads by goal | ||
| </p> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <div className="grid grid-cols-1 sm:grid-cols-2 xl:grid-cols-4 gap-4"> | ||
| <div className="rounded-lg border border-border/60 bg-background/60 p-4 space-y-1"> | ||
| <div className="flex items-center gap-2 text-sm text-muted-foreground"> | ||
| <Trophy className="h-4 w-4" /> | ||
| Best Overall | ||
| </div> | ||
| <p className="font-mono font-bold text-primary">{bestOverall.code}</p> | ||
| <p className="text-sm">{bestOverall.overall_rating.toFixed(1)}/5 rating</p> | ||
| </div> | ||
|
|
||
| <div className="rounded-lg border border-border/60 bg-background/60 p-4 space-y-1"> | ||
| <div className="flex items-center gap-2 text-sm text-muted-foreground"> | ||
| <Feather className="h-4 w-4" /> | ||
| Lightest Workload | ||
| </div> | ||
| <p className="font-mono font-bold text-primary">{lightestWorkload.code}</p> | ||
| <p className="text-sm">{lightestWorkload.workload_rating.toFixed(1)}/5 workload</p> | ||
| </div> | ||
|
|
||
| <div className="rounded-lg border border-border/60 bg-background/60 p-4 space-y-1"> | ||
| <div className="flex items-center gap-2 text-sm text-muted-foreground"> | ||
| <MessageSquare className="h-4 w-4" /> | ||
| Most Reviewed | ||
| </div> | ||
| <p className="font-mono font-bold text-primary">{mostReviewed.code}</p> | ||
| <p className="text-sm">{mostReviewed.review_count} student reviews</p> | ||
| </div> | ||
|
|
||
| <div className="rounded-lg border border-border/60 bg-background/60 p-4 space-y-1"> | ||
| <div className="flex items-center gap-2 text-sm text-muted-foreground"> | ||
| <Target className="h-4 w-4" /> | ||
| Best Rating-to-Effort | ||
| </div> | ||
| <p className="font-mono font-bold text-primary">{bestBalance.code}</p> | ||
| <p className="text-sm">High rating with lower effort profile</p> | ||
| </div> | ||
| </div> | ||
| </CardContent> | ||
| </Card> | ||
| </div> |
There was a problem hiding this comment.
Missing required chart types: radar and credits comparison are not rendered.
This section currently renders only the ratings bar chart plus quick takeaways. The PR objective includes a multi-metric radar chart and a credits comparison visualization; neither appears here.
🧩 Minimal direction for completion
return (
<div className="space-y-6">
{/* Ratings Comparison Bar Chart */}
<Card>...</Card>
+ {/* Multi-metric Radar Chart */}
+ <Card>
+ <CardHeader>...</CardHeader>
+ <CardContent>
+ {/* RadarChart with overall/difficulty/workload/review_count */}
+ </CardContent>
+ </Card>
+
+ {/* Credits Comparison */}
+ <Card>
+ <CardHeader>...</CardHeader>
+ <CardContent>
+ {/* BarChart (or equivalent) comparing course credits */}
+ </CardContent>
+ </Card>
+
<Card>{/* Quick Takeaways */}</Card>
</div>
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/courses/compare/ComparisonCharts.tsx` around lines 77 - 173,
The PR is missing the multi-metric radar chart and credits comparison visuals;
update the ComparisonCharts component to render a RadarChart (inside
ResponsiveContainer) using the existing ratingsData and courses mapping: add
Recharts components RadarChart, Radar, PolarGrid, PolarAngleAxis,
PolarRadiusAxis and map course.code -> Radar dataKey with colors[idx %
colors.length], and also render a separate credits comparison chart (e.g., a
small BarChart or ColumnBar) using a creditsData array derived from courses
(course.code and course.credits) with XAxis dataKey="code" and YAxis domain
derived from credits, reusing ResponsiveContainer and colors for bars; ensure
imports for RadarChart/Radar/Polar* and any creditsData preparation are added
and that keys/ids use course.id/course.code consistently to avoid rendering
collisions.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/courses/compare/ComparisonCharts.tsx (1)
79-176:⚠️ Potential issue | 🟠 MajorFeature scope gap: radar and credits comparison visuals are not rendered.
From Line 79 onward, this component renders the ratings bar chart and quick takeaways only. The PR objectives include a multi-metric radar chart and a credits comparison visualization, so this is still incomplete.
Suggested completion sketch
import { BarChart, Bar, + RadarChart, + Radar, + PolarGrid, + PolarAngleAxis, + PolarRadiusAxis, XAxis, YAxis, CartesianGrid, Tooltip, Legend, ResponsiveContainer, } from 'recharts'; +const creditsData = courses.map((course) => ({ + code: course.code, + credits: course.credits, +})); + +const radarData = [ + { metric: 'Overall', ...Object.fromEntries(courses.map((c) => [getSeriesKey(c), c.overall_rating])) }, + { metric: 'Difficulty', ...Object.fromEntries(courses.map((c) => [getSeriesKey(c), c.difficulty_rating])) }, + { metric: 'Workload', ...Object.fromEntries(courses.map((c) => [getSeriesKey(c), c.workload_rating])) }, +]; return ( <div className="space-y-6"> {/* Ratings Comparison Bar Chart */} <Card>...</Card> + {/* Multi-metric Radar Chart */} + <Card> + <CardHeader>...</CardHeader> + <CardContent> + <ResponsiveContainer width="100%" height={320}> + <RadarChart data={radarData}> + <PolarGrid /> + <PolarAngleAxis dataKey="metric" /> + <PolarRadiusAxis domain={[0, 5]} /> + {courses.map((course, idx) => ( + <Radar + key={course.id} + name={course.code} + dataKey={getSeriesKey(course)} + stroke={colors[idx % colors.length]} + fill={colors[idx % colors.length]} + fillOpacity={0.2} + /> + ))} + <Legend /> + <Tooltip /> + </RadarChart> + </ResponsiveContainer> + </CardContent> + </Card> + + {/* Credits Comparison */} + <Card> + <CardHeader>...</CardHeader> + <CardContent> + <ResponsiveContainer width="100%" height={260}> + <BarChart data={creditsData}> + <CartesianGrid strokeDasharray="3 3" /> + <XAxis dataKey="code" /> + <YAxis /> + <Tooltip /> + <Bar dataKey="credits" /> + </BarChart> + </ResponsiveContainer> + </CardContent> + </Card> + <Card>{/* Quick Takeaways */}</Card> </div> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/courses/compare/ComparisonCharts.tsx` around lines 79 - 176, The component currently renders only the ratings BarChart and quick takeaways; add the missing multi-metric radar chart and credits comparison visualization to meet the PR scope. Inside ComparisonCharts.tsx, below the existing Ratings Comparison Card (after the ResponsiveContainer/BarChart block) insert a Card that renders a RadarChart using the same courses/colors and a data array (e.g., radarData computed from courses by metric: overall_rating, difficulty_rating, workload_rating) with Radar, PolarGrid, PolarAngleAxis (dataKey "metric"), and Radars keyed by getSeriesKey(course); then add another Card that renders a credits comparison (e.g., a small BarChart or PieChart) built from creditsData derived from courses (course.credits) using XAxis/YAxis/Tooltip and Bars (use colors[idx % colors.length] and keys from getSeriesKey). Ensure you compute radarData and creditsData earlier in the component (add helper functions or mappings) and import any Recharts components you use (RadarChart, Radar, PolarGrid, PolarAngleAxis, PolarRadiusAxis, PieChart/BarChart) so the new visuals render correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/courses/compare/ComparisonCharts.tsx`:
- Around line 79-176: The component currently renders only the ratings BarChart
and quick takeaways; add the missing multi-metric radar chart and credits
comparison visualization to meet the PR scope. Inside ComparisonCharts.tsx,
below the existing Ratings Comparison Card (after the
ResponsiveContainer/BarChart block) insert a Card that renders a RadarChart
using the same courses/colors and a data array (e.g., radarData computed from
courses by metric: overall_rating, difficulty_rating, workload_rating) with
Radar, PolarGrid, PolarAngleAxis (dataKey "metric"), and Radars keyed by
getSeriesKey(course); then add another Card that renders a credits comparison
(e.g., a small BarChart or PieChart) built from creditsData derived from courses
(course.credits) using XAxis/YAxis/Tooltip and Bars (use colors[idx %
colors.length] and keys from getSeriesKey). Ensure you compute radarData and
creditsData earlier in the component (add helper functions or mappings) and
import any Recharts components you use (RadarChart, Radar, PolarGrid,
PolarAngleAxis, PolarRadiusAxis, PieChart/BarChart) so the new visuals render
correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f283e40-69c4-41f1-bcf1-7eb283969c51
📒 Files selected for processing (1)
src/components/courses/compare/ComparisonCharts.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a comprehensive course comparison tool that enables students to make informed decisions by directly evaluating multiple courses side by side.
Features Implemented:
Ratings Comparison Bar Chart
Multi-Metric Radar Chart
Credits Comparison
Fixes #49
Summary by CodeRabbit
New Features
Documentation