Results page : UI and graphs update #48
Conversation
- Added icons to question and explanation titles in Match Following, MCQ, and True/False question components for better visual representation. - Updated the MCQ question component to allow clickable options for selecting correct answers, enhancing user experience. - Improved the Tiptap editor to accept a height prop for better customization. - Refactored select components for cleaner code and improved styling. - Created a new test page for editing questions with sample data to facilitate testing.
…anagement - Created types for student overall results, recent test results, course results, test summaries, detailed test results, question results, and results filters. - Implemented ResultsAPI class with methods to fetch student overview, course results, test summaries, detailed test results, and recent results. - Added mock data for development and testing purposes for both student and teacher results.
feat: implement StudentResultsTable component for detailed student results feat: define types for teacher-specific results in types.ts refactor: remove deprecated TestSummaries component and related types refactor: delete unused types from results/types.ts fix: update results-api to use new types and mock data structure feat: create utility functions for consistent score color display chore: add stub for useRecentTestResults to resolve import errors
- Removed `question-stats-table` component and its export from `index.ts`. - Added `marks-frequency-bar-chart`, `question-wise-bar-chart`, and `student-performance-pie-chart` components for improved data visualization. - Updated mock data in `results-api.ts` to reflect more realistic test results and added new courses and tests. - Increased the number of mock students and questions for better testing scenarios.
…nts; update mock data for testing
…e colors; update performance distribution comments
- Fix SSR compatibility in student-performance-pie-chart by adding window check for getComputedStyle - Fix rounding error in QuestionWiseBarChart calculation to ensure correct + wrong = attemptedCount - Rename QuestionStat to QuestionStatChart in question-wise-bar-chart to avoid type conflicts - Add useThemeColors hook for SSR-safe theme detection in chart components - Switch from sonner to useToast hook for toast messages (partial implementation)
WalkthroughThis update introduces several new chart components for both student and teacher results pages, enhancing data visualization with performance trends, score distributions, and question-wise analytics. The mock API data is expanded for realism, and layouts are refactored to accommodate the new components and provide more detailed statistical summaries. Changes
Sequence Diagram(s)sequenceDiagram
participant Student as StudentResultsPage
participant TrendsChart as StudentPerformanceTrendsChart
participant API as ResultsAPI
Student->>API: Fetch courseResults
Student->>TrendsChart: Pass courseResults as courses prop
TrendsChart->>TrendsChart: Render line chart of performance trends
sequenceDiagram
participant Teacher as TeacherResultsPage
participant API as TeacherResultsAPI
participant PieChart as StudentPerformancePieChart
participant BarChart as MarksFrequencyBarChart
participant QBarChart as QuestionWiseBarChart
Teacher->>API: Fetch test statistics
Teacher->>PieChart: Pass scores
Teacher->>BarChart: Pass scores
Teacher->>QBarChart: Pass questionStats
PieChart->>PieChart: Render performance pie chart
BarChart->>BarChart: Render marks frequency bar chart
QBarChart->>QBarChart: Render question-wise bar chart
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances both teacher and student results pages by introducing interactive chart components for visualizing performance data and updating the UI layout.
- Added new chart components (pie, bar, line) to display distributions, question stats, and performance trends.
- Updated teacher and student result pages to integrate charts, including a layout toggle on the teacher page.
- Removed deprecated exports and cleaned up detailed result view by removing redundant charts.
Reviewed Changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/results/teacher/student-performance-pie-chart.tsx | New pie chart for student performance distribution |
| src/components/results/teacher/question-wise-bar-chart.tsx | Bar chart for question-wise correct/wrong stats |
| src/components/results/teacher/marks-frequency-bar-chart.tsx | Line chart showing marks frequency distribution |
| src/components/results/teacher/index.ts | Removed obsolete export of question-stats-table |
| src/components/results/student/performance-trends-chart.tsx | Student-side line chart for performance trends |
| src/components/results/student/performance-distribution-chart.tsx | Student-side bar chart for distribution with grouping selector |
| src/components/results/common/question-bar-chart.tsx | Common bar chart component for question results |
| src/components/results/common/marks-pie-chart.tsx | Common pie chart for marks breakdown |
| src/components/results/common/detailed-test-result.tsx | Refactored MCQ rendering; removed embedded charts |
| src/app/(test)/teacher-results/page.tsx | Integrated new teacher charts and layout toggle |
| src/app/(test)/student-results/page.tsx | Added performance trends chart to student page |
Comments suppressed due to low confidence (2)
src/components/results/teacher/marks-frequency-bar-chart.tsx:38
- [nitpick] This component renders a line chart (using
Line) but is namedMarksFrequencyBarChart. Consider renaming it toMarksFrequencyLineChartfor clarity.
export const MarksFrequencyBarChart: React.FC<MarksFrequencyBarChartProps> = ({
src/components/results/teacher/student-performance-pie-chart.tsx:1
- This new chart component lacks associated unit or snapshot tests. Consider adding tests to verify that the chart renders correctly for different score distributions.
import React from "react";
| scores={testStatistics.studentResults.map( | ||
| (s) => s.percentage, | ||
| )} | ||
| totalMarks={testStatistics.totalMarks} |
There was a problem hiding this comment.
The MarksFrequencyBarChart component does not accept a totalMarks prop in its interface. Either remove this prop from the JSX or update the component signature to accept and use it.
| totalMarks={testStatistics.totalMarks} |
| const chart = ( | ||
| <Bar | ||
| data={{ | ||
| labels: visibleStats.map((q, i) => `Q${i + 1}`), |
There was a problem hiding this comment.
You already have a question property in visibleStats. To ensure the labels reflect the actual question identifiers, consider using visibleStats.map(q => q.question) instead of hard-coding Q${i + 1}.
| labels: visibleStats.map((q, i) => `Q${i + 1}`), | |
| labels: visibleStats.map((q) => q.question), |
| Rows3, | ||
| } from "lucide-react"; | ||
| import { Separator } from "@/components/ui/separator"; | ||
| import { toast } from "sonner"; |
There was a problem hiding this comment.
Per project conventions, use the useToast hook from src/hooks/use-toast.ts instead of importing toast directly from sonner.
| import { toast } from "sonner"; | |
| import { useToast } from "@/hooks/use-toast"; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/app/(test)/teacher-results/page.tsx (1)
500-556: Extract repeated score calculations to avoid duplicationThe calculation
(percentage / 100) * totalMarksis repeated multiple times. Consider extracting it to a utility function.Add a utility function:
const calculateAbsoluteScore = (percentage: number, totalMarks: number): string => { return ((percentage / 100) * totalMarks).toFixed(1); };Then use it throughout:
-{testStatistics.averageScore.toFixed(1)}% ( -{( - (testStatistics.averageScore / 100) * - testStatistics.totalMarks -).toFixed(1)} -/{testStatistics.totalMarks}) +{testStatistics.averageScore.toFixed(1)}% ( +{calculateAbsoluteScore(testStatistics.averageScore, testStatistics.totalMarks)} +/{testStatistics.totalMarks})
🧹 Nitpick comments (14)
src/components/results/common/marks-pie-chart.tsx (1)
13-23: Consider adding validation for empty data sets.The component correctly processes question results but doesn't handle the edge case where the questions array might be empty, which could result in a pie chart with all zero values.
Consider adding a guard clause to handle empty data:
export const MarksPieChart: React.FC<MarksPieChartProps> = ({ questions }) => { + if (!questions.length) { + return ( + <div className="w-full h-64 flex items-center justify-center text-muted-foreground"> + No question data available + </div> + ); + } + const correctMarks = questions .filter((q) => q.isCorrect === true) .reduce((sum, q) => sum + (q.marks || 0), 0);src/components/results/common/question-bar-chart.tsx (1)
23-26: Consider visualizing unanswered questions.The component only shows correct and wrong answers but doesn't visualize unanswered questions (where
isCorrect === undefined). This might provide incomplete information to users.Consider adding a third dataset for unanswered questions:
export const QuestionBarChart: React.FC<QuestionBarChartProps> = ({ questions, }) => { const labels = questions.map((q, i) => `Q${i + 1}`); const correct = questions.map((q) => (q.isCorrect === true ? 1 : 0)); const wrong = questions.map((q) => (q.isCorrect === false ? 1 : 0)); + const unanswered = questions.map((q) => (q.isCorrect === undefined ? 1 : 0));src/components/results/teacher/marks-frequency-bar-chart.tsx (2)
23-23: Component name doesn't match implementation.The component is named
MarksFrequencyBarChartbut actually renders a Line chart. This could cause confusion for developers using this component.Consider renaming to better reflect the implementation:
-export interface MarksFrequencyBarChartProps { +export interface MarksFrequencyLineChartProps {-export const MarksFrequencyBarChart: React.FC<MarksFrequencyBarChartProps> = ({ +export const MarksFrequencyLineChart: React.FC<MarksFrequencyLineChartProps> = ({
63-70: Reusable SSR-safe utility function.The
getCSSVariablefunction is well-implemented and could potentially be extracted to a shared utilities file for reuse across other chart components.Consider extracting this utility to a shared location:
// utils/chart-utils.ts export const getCSSVariable = (variable: string, fallback: string) => { if (typeof window === "undefined") return fallback; return ( getComputedStyle(document.documentElement).getPropertyValue(variable) || fallback ); };src/components/results/student/performance-trends-chart.tsx (3)
77-77: Use a more robust approach for color opacityUsing string concatenation with hex colors for opacity is fragile and may not work correctly with all color formats.
Consider using a utility function or CSS rgba values:
-backgroundColor: palette[idx % palette.length] + "33", // 20% opacity fill +backgroundColor: `${palette[idx % palette.length]}33`, // 20% opacity fillOr better yet, use a color manipulation library or convert to rgba:
// Example utility function const hexToRgba = (hex: string, opacity: number) => { const r = parseInt(hex.slice(1, 3), 16); const g = parseInt(hex.slice(3, 5), 16); const b = parseInt(hex.slice(5, 7), 16); return `rgba(${r}, ${g}, ${b}, ${opacity})`; };
91-96: Extract dark mode detection as a utility functionThe
isDarkModefunction doesn't depend on component props or state and could be reused elsewhere.Consider extracting this to a shared utility file:
// In @/lib/utils/theme.ts export const isDarkMode = (): boolean => { if (typeof window === "undefined" || typeof document === "undefined") return false; return document.documentElement.classList.contains("dark"); };Then import and use it in the component.
113-113: Extract complex className logic for better maintainabilityThe className string is complex and hard to read. Consider using a utility like
clsxor extracting the logic.import { clsx } from 'clsx'; const buttonClassName = clsx( "flex items-center gap-2 px-3 py-1 rounded-full text-xs font-semibold shadow border-2 transition-colors duration-150 focus:outline-none", { "text-white border-transparent": isActive, "text-gray-400 border-dashed border-gray-300 opacity-60": !isActive, } );src/components/results/student/performance-distribution-chart.tsx (2)
60-60: Simplify the empty array checkThe condition can be simplified since empty arrays are already falsy in the first check.
-if (!results || results.length === 0) return []; +if (!results?.length) return [];
70-78: Clarify the special handling for 100% scoresThe logic for handling perfect scores could be more explicit with a comment or clearer structure.
results.forEach((result) => { const scorePercentage = result.percentage; + // Place perfect scores (100%) in the last bin if (scorePercentage === 100) { bins[bins.length - 1].count++; } else { const binIndex = Math.floor(scorePercentage / groupSize); if (binIndex >= 0 && binIndex < bins.length) { bins[binIndex].count++; } } });src/app/(test)/teacher-results/page.tsx (2)
630-639: Extract complex question stats mapping logicThe mapping logic for question statistics is complex and could be extracted for better readability and testability.
Extract to a utility function:
const mapQuestionStatsToChartData = (questionStats: QuestionStat[]) => { return questionStats.map((q, i) => { const correctCount = Math.round( (q.correctPercentage / 100) * q.attemptedCount ); return { question: `Q${i + 1}`, correct: correctCount, wrong: q.attemptedCount - correctCount, }; }); }; // Usage: questionStats={mapQuestionStatsToChartData(testStatistics.questionStats)}
575-591: Enhance accessibility for layout toggle buttonsThe toggle buttons could benefit from better accessibility attributes.
Add role and aria-label attributes:
<div className="inline-flex rounded-md shadow-sm bg-muted border border-border"> <button type="button" - aria-label="Vertical layout" + aria-label="Switch to vertical layout" + aria-pressed={chartsStacked} + role="radio" className={`px-2 py-1 flex items-center justify-center rounded-l-md focus:outline-none transition-colors duration-150 ${chartsStacked ? "bg-primary text-primary-foreground" : "bg-transparent text-muted-foreground hover:bg-accent"} border-r border-border`} onClick={() => setChartsStacked(true)} >src/components/results/teacher/question-wise-bar-chart.tsx (3)
27-42: Consider enhancing the theme colors hook for better chart integration.The hook is well-implemented and SSR-safe, but it only retrieves the foreground color. Consider expanding it to include other theme colors that could enhance the chart's visual integration.
const useThemeColors = () => { - const [colors, setColors] = useState({ foreground: "#222" }); + const [colors, setColors] = useState({ + foreground: "#222", + background: "#fff", + muted: "#666", + accent: "#0066cc" + }); useEffect(() => { const updateColors = () => { const style = getComputedStyle(document.documentElement); setColors({ foreground: style.getPropertyValue("--foreground") || "#222", + background: style.getPropertyValue("--background") || "#fff", + muted: style.getPropertyValue("--muted") || "#666", + accent: style.getPropertyValue("--accent") || "#0066cc", }); }; updateColors(); }, []); return colors; };
86-100: Remove redundant Chart.js configuration.The
stacked: falseconfiguration is redundant since bars are not stacked by default in Chart.js.scales: { x: { - stacked: false, ticks: { color: themeColors.foreground, }, }, y: { beginAtZero: true, - stacked: false, ticks: { color: themeColors.foreground, }, }, },
102-102: Consider responsive height instead of fixed height.The fixed height of 220px might not work well across different screen sizes and could cause layout issues.
- height={220} + height="220px"Or better yet, use CSS classes for responsive height:
- height={220} + className="h-[220px] sm:h-[280px] lg:h-[320px]"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
src/app/(test)/student-results/page.tsx(2 hunks)src/app/(test)/teacher-results/page.tsx(7 hunks)src/components/results/common/detailed-test-result.tsx(3 hunks)src/components/results/common/marks-pie-chart.tsx(1 hunks)src/components/results/common/question-bar-chart.tsx(1 hunks)src/components/results/student/performance-distribution-chart.tsx(1 hunks)src/components/results/student/performance-trends-chart.tsx(1 hunks)src/components/results/teacher/index.ts(0 hunks)src/components/results/teacher/marks-frequency-bar-chart.tsx(1 hunks)src/components/results/teacher/question-wise-bar-chart.tsx(1 hunks)src/components/results/teacher/student-performance-pie-chart.tsx(1 hunks)src/lib/results-api.ts(16 hunks)
💤 Files with no reviewable changes (1)
- src/components/results/teacher/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/components/results/teacher/marks-frequency-bar-chart.tsx (2)
src/components/ui/tooltip.tsx (1)
Tooltip(61-61)src/components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)
src/components/results/student/performance-trends-chart.tsx (1)
src/components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)
src/components/results/teacher/question-wise-bar-chart.tsx (2)
src/components/ui/tooltip.tsx (1)
Tooltip(61-61)src/components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)
src/lib/results-api.ts (1)
src/components/results/teacher/types.ts (2)
StudentTestResult(39-54)QuestionStat(76-86)
🔇 Additional comments (14)
src/components/results/common/marks-pie-chart.tsx (1)
24-46: LGTM! Clean chart implementation with good responsive design.The Chart.js configuration is well-structured with appropriate responsive settings and tooltip functionality. The color scheme provides good visual distinction between categories.
src/components/results/common/question-bar-chart.tsx (1)
47-66: Excellent chart configuration with proper stacking and grid styling.The Chart.js options are well-configured with appropriate scales, legend positioning, and visual styling. The stacked configuration correctly represents the question-wise results.
src/lib/results-api.ts (3)
699-891: Well-structured expansion with consistent data patterns.The addition of two new courses ("Computer Networks" and "Software Engineering") follows the existing data structure patterns correctly. The test data includes realistic score distributions and appropriate difficulty progressions.
1441-1504: Good expansion of mock data for better chart visualization.Increasing the student count from 15 to 30 and questions from 10 to 20 provides more realistic data for testing the new chart components. The addition of the
totalStudentsfield addresses a missing data point.
169-172: Verify average score calculation for Data Structures course.The updated course shows
averageScore: 81.1andhighestScore: 98, but I need to verify this matches the actual test scores in the expanded data.Let me verify the average score calculation:
#!/bin/bash # Description: Calculate the actual average score from the test results to verify accuracy # Extract test scores for course-1 and calculate average echo "Calculating average score for Data Structures course..." echo "Test scores: 78, 85, 72, 95, 88, 81, 98, 67" python3 -c " scores = [78, 85, 72, 95, 88, 81, 98, 67] average = sum(scores) / len(scores) print(f'Calculated average: {average:.1f}') print(f'Declared average: 81.1') print(f'Match: {abs(average - 81.1) < 0.1}') "src/components/results/teacher/student-performance-pie-chart.tsx (3)
12-25: Clean and well-documented performance categorization logic.The performance thresholds are clearly defined and the distribution function is pure and easily testable. The categorization ranges are sensible for academic performance evaluation.
58-64: Excellent SSR-safe CSS variable handling.The implementation properly handles server-side rendering by checking for
windowavailability before accessinggetComputedStyle. This prevents SSR hydration issues.
71-81: Good use of Card components for consistent UI structure.The component properly uses the Card UI components to maintain consistent styling and layout across the application.
src/components/results/teacher/marks-frequency-bar-chart.tsx (2)
28-36: Solid binning logic with proper boundary handling.The binning function correctly handles edge cases by using
Math.minto prevent index out-of-bounds when scores equal the maximum value (100).
109-118: Well-structured chart layout with proper responsive design.The component uses the Card UI pattern consistently and includes appropriate height configuration for the chart container.
src/app/(test)/student-results/page.tsx (1)
15-15: Chart integration looks good!The
StudentPerformanceTrendsChartis properly imported and integrated into the student results page with appropriate data passing.Also applies to: 174-178
src/components/results/common/detailed-test-result.tsx (1)
139-177: Good refactoring for improved readabilityThe explicit
isCorrectandisSelectedboolean variables make the conditional logic much clearer and easier to understand.src/components/results/teacher/question-wise-bar-chart.tsx (2)
1-13: LGTM! Proper Chart.js setup.The imports and Chart.js registration are correctly configured for a bar chart implementation.
15-25: Well-defined interfaces with clear naming strategy.The interfaces appropriately define the component's data structure and props. The naming strategy to avoid conflicts is a good practice.
| const sortedTests = [...course.testResults].sort((a, b) => | ||
| a.completedAt.localeCompare(b.completedAt), | ||
| ); |
There was a problem hiding this comment.
Add null/undefined handling for date sorting
The sorting logic doesn't handle cases where completedAt might be null or undefined.
Add defensive checks:
-const sortedTests = [...course.testResults].sort((a, b) =>
- a.completedAt.localeCompare(b.completedAt),
-);
+const sortedTests = [...course.testResults].sort((a, b) => {
+ if (!a.completedAt || !b.completedAt) return 0;
+ return a.completedAt.localeCompare(b.completedAt);
+});📝 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.
| const sortedTests = [...course.testResults].sort((a, b) => | |
| a.completedAt.localeCompare(b.completedAt), | |
| ); | |
| const sortedTests = [...course.testResults].sort((a, b) => { | |
| if (!a.completedAt || !b.completedAt) return 0; | |
| return a.completedAt.localeCompare(b.completedAt); | |
| }); |
🤖 Prompt for AI Agents
In src/components/results/student/performance-trends-chart.tsx around lines 66
to 68, the sorting logic for testResults does not handle cases where completedAt
might be null or undefined. Update the sort comparator to first check if either
a.completedAt or b.completedAt is null or undefined, and handle those cases by
placing null/undefined dates either at the start or end consistently. Then, if
both dates are valid, proceed with localeCompare as before.
| // Only show first 15 questions, rest scrollable in horizontal mode; for vertical, scroll if >30 | ||
| const scrollThreshold = horizontalMode ? 15 : 30; | ||
| const isScrollable = questionStats.length > scrollThreshold; | ||
| const visibleStats = | ||
| isScrollable && horizontalMode ? questionStats.slice(0, 15) : questionStats; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify the scrolling implementation.
The scrolling logic is incomplete. The code mentions "rest scrollable" but doesn't implement actual scrolling functionality. The vertical mode doesn't limit questions despite the comment suggesting it should scroll if >30 questions.
Consider either:
- Implementing actual horizontal scrolling for the chart container
- Simplifying the logic to just limit the number of displayed questions
- Adding a "Show More" button to load additional questions
- // Only show first 15 questions, rest scrollable in horizontal mode; for vertical, scroll if >30
- const scrollThreshold = horizontalMode ? 15 : 30;
- const isScrollable = questionStats.length > scrollThreshold;
- const visibleStats =
- isScrollable && horizontalMode ? questionStats.slice(0, 15) : questionStats;
+ // Limit questions to prevent overcrowding
+ const maxQuestions = horizontalMode ? 15 : 30;
+ const visibleStats = questionStats.slice(0, maxQuestions);📝 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.
| // Only show first 15 questions, rest scrollable in horizontal mode; for vertical, scroll if >30 | |
| const scrollThreshold = horizontalMode ? 15 : 30; | |
| const isScrollable = questionStats.length > scrollThreshold; | |
| const visibleStats = | |
| isScrollable && horizontalMode ? questionStats.slice(0, 15) : questionStats; | |
| // Limit questions to prevent overcrowding | |
| const maxQuestions = horizontalMode ? 15 : 30; | |
| const visibleStats = questionStats.slice(0, maxQuestions); |
🤖 Prompt for AI Agents
In src/components/results/teacher/question-wise-bar-chart.tsx around lines 49 to
53, the current code sets thresholds for scrolling but does not implement actual
scrolling or question limiting for vertical mode as the comment suggests. To fix
this, either add CSS or container logic to enable horizontal scrolling when
horizontalMode is true and question count exceeds 15, or simplify by limiting
the number of displayed questions in both modes. Alternatively, implement a
"Show More" button to load additional questions dynamically. Ensure the UI
behavior matches the intended scrolling or question limiting described in the
comments.
| export const QuestionWiseBarChart: React.FC<QuestionWiseBarChartProps> = ({ | ||
| questionStats, | ||
| horizontalMode = false, | ||
| }) => { | ||
| const themeColors = useThemeColors(); | ||
| // Only show first 15 questions, rest scrollable in horizontal mode; for vertical, scroll if >30 | ||
| const scrollThreshold = horizontalMode ? 15 : 30; | ||
| const isScrollable = questionStats.length > scrollThreshold; | ||
| const visibleStats = | ||
| isScrollable && horizontalMode ? questionStats.slice(0, 15) : questionStats; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for edge cases.
The component doesn't handle edge cases like empty data or malformed question stats, which could cause runtime errors.
export const QuestionWiseBarChart: React.FC<QuestionWiseBarChartProps> = ({
questionStats,
horizontalMode = false,
}) => {
+ // Handle empty data
+ if (!questionStats || questionStats.length === 0) {
+ return (
+ <Card className="w-full h-full">
+ <CardHeader>
+ <CardTitle>Question-wise Correct/Wrong</CardTitle>
+ </CardHeader>
+ <CardContent>
+ <div className="flex items-center justify-center h-[220px] text-muted-foreground">
+ No question data available
+ </div>
+ </CardContent>
+ </Card>
+ );
+ }
+
const themeColors = useThemeColors();🤖 Prompt for AI Agents
In src/components/results/teacher/question-wise-bar-chart.tsx around lines 44 to
53, the component lacks error handling for cases when questionStats is empty or
malformed. Add checks to verify questionStats is a valid array with expected
structure before processing. If invalid or empty, render a fallback UI or return
null to prevent runtime errors. This ensures the component handles edge cases
gracefully.
| labels: visibleStats.map((q, i) => `Q${i + 1}`), | ||
| datasets: [ | ||
| { | ||
| label: "Correct", | ||
| data: visibleStats.map((q) => q.correct), | ||
| backgroundColor: "#22c55e", | ||
| borderRadius: 4, | ||
| }, | ||
| { | ||
| label: "Wrong", | ||
| data: visibleStats.map((q) => q.wrong), | ||
| backgroundColor: "#ef4444", | ||
| borderRadius: 4, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address color inconsistency and consider question labeling.
Two concerns with the chart data configuration:
-
Color inconsistency: Hard-coded colors (#22c55e, #ef4444) are used for bars while theme colors are used elsewhere. This breaks theme consistency.
-
Question labeling: Using
Q${i + 1}instead of actual question text may reduce readability for users who want to identify specific questions.
data={{
labels: visibleStats.map((q, i) => `Q${i + 1}`),
datasets: [
{
label: "Correct",
data: visibleStats.map((q) => q.correct),
- backgroundColor: "#22c55e",
+ backgroundColor: "hsl(var(--success))",
borderRadius: 4,
},
{
label: "Wrong",
data: visibleStats.map((q) => q.wrong),
- backgroundColor: "#ef4444",
+ backgroundColor: "hsl(var(--destructive))",
borderRadius: 4,
},
],
}}Consider adding a tooltip to show the actual question text when hovering over bars.
📝 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.
| labels: visibleStats.map((q, i) => `Q${i + 1}`), | |
| datasets: [ | |
| { | |
| label: "Correct", | |
| data: visibleStats.map((q) => q.correct), | |
| backgroundColor: "#22c55e", | |
| borderRadius: 4, | |
| }, | |
| { | |
| label: "Wrong", | |
| data: visibleStats.map((q) => q.wrong), | |
| backgroundColor: "#ef4444", | |
| borderRadius: 4, | |
| }, | |
| ], | |
| labels: visibleStats.map((q, i) => `Q${i + 1}`), | |
| datasets: [ | |
| { | |
| label: "Correct", | |
| data: visibleStats.map((q) => q.correct), | |
| backgroundColor: "hsl(var(--success))", | |
| borderRadius: 4, | |
| }, | |
| { | |
| label: "Wrong", | |
| data: visibleStats.map((q) => q.wrong), | |
| backgroundColor: "hsl(var(--destructive))", | |
| borderRadius: 4, | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In src/components/results/teacher/question-wise-bar-chart.tsx around lines 57 to
71, replace the hard-coded bar colors (#22c55e and #ef4444) with colors derived
from the current theme to maintain visual consistency. Also, update the labels
array to use the actual question text from visibleStats instead of generic "Q1",
"Q2", etc., to improve readability. Additionally, implement a tooltip feature on
the chart bars that displays the full question text when hovered over, enhancing
user understanding.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/results/common/detailed-test-result.tsx (1)
139-203: Fix the parsing error and incomplete refactoring in MCQ rendering logic.The code contains both the new refactored version (lines 139-166) and remnants of the old version (lines 167-203), causing a parsing error. The JSX structure is malformed with duplicate mapping logic.
Apply this diff to remove the old code and fix the parsing error:
{question.options?.map((option, index) => { const isCorrect = option.isCorrect; const isSelected = question.studentAnswer === option.id; return ( <div key={option.id} className={`p-2 rounded border ${ isCorrect ? "bg-emerald-100 border-emerald-300 dark:bg-green-900/20 dark:border-green-800" : isSelected ? "bg-red-100 border-red-300 dark:bg-red-900/20 dark:border-red-800" : "bg-slate-100 border-slate-300 dark:bg-gray-900/20 dark:border-gray-800" }`} > <div className="flex items-center gap-2"> <span className="font-medium"> {String.fromCharCode(65 + index)}. </span> <span>{option.text}</span> {isCorrect && ( <Badge variant="secondary" className="ml-auto bg-emerald-100 border border-emerald-300 text-emerald-800" > Correct </Badge> )} {isSelected && !isCorrect && ( - {question.options?.map((option, index) => ( - <div - key={option.id} - className={`p-2 rounded border ${ - option.isCorrect - ? "bg-emerald-100 border-emerald-300 dark:bg-green-900/20 dark:border-green-800" - : question.studentAnswer === option.id - ? "bg-red-100 border-red-300 dark:bg-red-900/20 dark:border-red-800" - : "bg-slate-100 border-slate-300 dark:bg-gray-900/20 dark:border-gray-800" - }`} - > - <div className="flex items-center gap-2"> - <span className="font-medium"> - {String.fromCharCode(65 + index)}. - </span> - <span>{option.text}</span> - {option.isCorrect && ( - <Badge - variant="secondary" - className="ml-auto bg-emerald-100 border border-emerald-300 text-emerald-800" - > - Correct - </Badge> - )} - {question.studentAnswer === option.id && - !option.isCorrect && ( - <Badge - variant="secondary" - className="ml-auto bg-red-100 border border-red-300 text-red-800" - > - Your Answer - </Badge> - )} + <Badge + variant="secondary" + className="ml-auto bg-red-100 border border-red-300 text-red-800" + > + Your Answer + </Badge> + )} </div> - </div> - ); - })} + </div> + ); + })}
♻️ Duplicate comments (2)
src/app/(test)/teacher-results/page.tsx (2)
34-34: Replace toast import with project's useToast hook.Per project conventions, use the
useToasthook fromsrc/hooks/use-toast.tsinstead of importingtoastdirectly fromsonner.-import { toast } from "sonner"; +import { useToast } from "@/hooks/use-toast";Then update the toast usage in the component to use the hook pattern.
621-621: Verify MarksFrequencyBarChart component accepts totalMarks prop.The
MarksFrequencyBarChartcomponent is receiving atotalMarksprop that may not be defined in its interface.#!/bin/bash # Check the MarksFrequencyBarChart component interface ast-grep --pattern 'interface $_ { $$$ }' src/components/results/teacher/marks-frequency-bar-chart.tsx # Also check the component props definition ast-grep --pattern 'const MarksFrequencyBarChart: React.FC<$_> = ($_) => { $$$ }' src/components/results/teacher/marks-frequency-bar-chart.tsx
🧹 Nitpick comments (1)
src/components/results/common/detailed-test-result.tsx (1)
401-401: Consider removing the comment or implementing the mentioned change.The comment mentions removing charts for students, but no actual code changes accompany this comment. If charts were indeed removed, consider removing this comment as it may cause confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/(test)/teacher-results/page.tsx(7 hunks)src/components/results/common/detailed-test-result.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,jsx,ts,tsx}`: Use shadcn/ui components for UI elements. Use Tailwind CSS for styling. Ensure all UI components are responsive and work well on both desktop and mobile dev...
**/*.{js,jsx,ts,tsx}: Use shadcn/ui components for UI elements.
Use Tailwind CSS for styling.
Ensure all UI components are responsive and work well on both desktop and mobile devices.
Ensure all components work well with both light and dark mode.
Where possible, use useQuery and useMutation from @tanstack/react-query for API calls.
Use Server Components in Next.js where possible.
Strictly use TypeScript for type safety.
src/app/(test)/teacher-results/page.tsxsrc/components/results/common/detailed-test-result.tsx
🧠 Learnings (1)
src/app/(test)/teacher-results/page.tsx (1)
Learnt from: CR
PR: evalify/evalify-frontend#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-24T12:27:53.281Z
Learning: For toast messages, always use the useToast hook from src/hooks/use-toast.ts instead of native alert or error messages to provide a better user experience.
🪛 Biome (1.9.4)
src/components/results/common/detailed-test-result.tsx
[error] 203-203: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 203-203: Unexpected token. Did you mean {'}'} or }?
(parse)
🔇 Additional comments (6)
src/components/results/common/detailed-test-result.tsx (1)
336-339: LGTM! Improved code formatting.The multi-line formatting of the
getScoreTextColorfunction call improves readability.src/app/(test)/teacher-results/page.tsx (5)
30-31: LGTM! Well-organized chart component imports.The new imports for layout icons and chart components are properly organized and follow the project's import conventions.
Also applies to: 35-39
63-63: LGTM! Clear and descriptive state variable.The
chartsStackedstate variable clearly indicates its purpose for the layout toggle functionality.
500-505: LGTM! Enhanced score displays with absolute marks.The addition of absolute marks alongside percentages provides more context and improves the user experience. The calculations are correct and the formatting is consistent.
Also applies to: 516-519, 530-540, 553-558
565-645: LGTM! Well-implemented performance graphs section.The new performance graphs section with layout toggle functionality is well-structured:
- Clear section heading and layout controls
- Responsive grid/flex layout based on toggle state
- Proper chart component integration
- Good use of conditional styling for different layouts
684-707: LGTM! Improved student detail view organization.Moving the charts to the top of the student detail view improves visibility and user experience. The grid layout for the charts and clear section headings enhance the overall presentation.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores