enhance(apps/analytics): add illustrations for asynchronous activity progress on performance dashboard #4396
Conversation
sjschlapbach
changed the title
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several new React components related to analytics in the frontend application, including Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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
🧹 Outside diff range and nitpick comments (17)
packages/graphql/src/ops.ts (1)
2178-2182: Remove extra blank line.There's an unnecessary blank line between the type definitions.
export type QueryGetCoursePerformanceAnalyticsArgs = { courseId: Scalars['String']['input']; }; - export type QueryGetCourseRunningLiveQuizzesArgs = {apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx (4)
27-27: Simplify loading state condition.Since the
useQueryhook now handles the absence ofcourseId, the check for!courseIdin the loading condition is redundant.Apply this diff to streamline the condition:
-if (loading || !courseId) { +if (loading) {
37-37: Simplify null and undefined checks forcourse.The condition can be simplified for better readability.
Apply this diff:
-if (course === null || typeof course === 'undefined' || error) { +if (!course || error) {
51-53: Use appropriate heading levels for accessibility.Ensure that heading levels follow a logical order to improve accessibility and SEO.
Consider verifying if
<H1>is the correct heading level in this context.
55-58: Ensure proper pluralization in localization strings.When displaying
totalParticipants, make sure the localization handles both singular and plural forms correctly.Verify that
manage.analytics.totalParticipantssupports pluralization.packages/graphql/src/services/analytics.ts (1)
142-142: Avoid relying ontotalCourseParticipantsfrom a single activity.Accessing
totalCourseParticipantsfromactivityProgresses[0]may not be reliable if data varies.Consider retrieving
totalParticipantsdirectly from thecourseentity:-return { - name: course.name, - totalParticipants: course.activityProgresses[0]!.totalCourseParticipants, - activityProgresses, -} +return { + name: course.name, + totalParticipants: course.participations.length, + activityProgresses, +}apps/frontend-manage/src/components/analytics/performance/PerformanceAnalyticsNavigation.tsx (1)
5-14: Consider adding a navigation link to the performance dashboard.The current navigation omits a link back to the performance dashboard, which may affect user experience.
Include a link to the performance page to enhance navigation:
return ( <AnalyticsNavigation + hrefCenter={`/analytics/${courseId}/performance`} + labelCenter={<PerformanceDashboardLabel />} hrefLeft={`/analytics/${courseId}/activity`} labelLeft={<ActivityDashboardLabel />} hrefRight={`/analytics/${courseId}/quizzes`} labelRight={<QuizDashboardLabel />} /> )apps/frontend-manage/src/components/analytics/AnalyticsLoadingView.tsx (1)
17-20: Enhance accessibility for loading stateWhile the implementation is clean, consider adding ARIA attributes to improve accessibility:
- <div className="flex h-full w-full flex-row items-center justify-center gap-4 text-lg"> + <div + className="flex h-full w-full flex-row items-center justify-center gap-4 text-lg" + role="status" + aria-live="polite" + > {t('manage.analytics.analyticsLoadingWait')} - <Loader basic /> + <Loader basic aria-hidden="true" /> </div>apps/frontend-manage/src/components/analytics/AnalyticsErrorView.tsx (2)
16-18: Consider removing redundant title displayThe title is currently shown both in the Layout's displayName and as an H1. This might be redundant and could create visual noise.
return ( <Layout displayName={title}> {navigation} - <H1>{title}</H1>
6-12: Consider adding error details propThe error view could be more informative by accepting optional error details to display more specific information about what went wrong.
function AnalyticsErrorView({ title, navigation, + errorDetails, }: { title: string navigation: React.ReactNode + errorDetails?: string }) { const t = useTranslations() return ( <Layout displayName={title}> {navigation} <UserNotification message={t('manage.analytics.analyticsLoadingFailed')} + description={errorDetails} type="error" className={{ root: 'mx-auto my-auto w-max max-w-full text-base' }} />Also applies to: 19-23
apps/frontend-manage/src/components/analytics/performance/ActivityProgressPlot.tsx (2)
22-27: Move color scheme to theme constantsThe color scheme is currently hardcoded with Tailwind classes in comments. Consider moving these to a theme constants file for better maintainability.
- // required for tailwind styles to be included: text-[#4ade80] text-[#15803d] text-[#064e3b] - const chartColors = { - started: '#4ade80', - completed: '#15803d', - repeated: '#064e3b', - } + const chartColors = ACTIVITY_PROGRESS_COLORSCreate a new file
constants/theme.ts:export const ACTIVITY_PROGRESS_COLORS = { started: '#4ade80', completed: '#15803d', repeated: '#064e3b', } as const
33-52: Improve legend positioning and responsivenessThe current legend positioning with absolute positioning might cause issues on different screen sizes. Consider using a more flexible layout.
- <div className="relative"> + <div className="flex flex-wrap items-center justify-between gap-4"> <H2>{t('manage.analytics.asynchronousActivityProgress')}</H2> <Legend payload={[ { value: t('manage.analytics.started'), color: chartColors.started, type: 'rect', }, { value: t('manage.analytics.completed'), color: chartColors.completed, type: 'rect', }, { value: t('manage.analytics.repeated'), color: chartColors.repeated, type: 'rect', }, ]} - wrapperStyle={{ bottom: 0, right: 0 }} + wrapperStyle={{ display: 'flex', gap: '1rem' }} />apps/frontend-manage/src/components/analytics/performance/StackedProgress.tsx (2)
50-58: Consider adding accessibility features to the chartThe chart lacks accessibility features for screen readers. Consider adding ARIA labels and descriptions.
<ResponsiveContainer width="100%" height={35}> - <BarChart data={data} layout="vertical"> + <BarChart + data={data} + layout="vertical" + role="img" + aria-label={`Progress chart for ${progress.activityName}`} + >
50-51: Optimize ResponsiveContainer performanceResponsiveContainer can cause performance issues when many instances are rendered. Consider using a fixed size container for lists with many items.
Consider implementing virtualization if this component is used in a list with many items.
apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx (1)
29-31: Consider memoizing the navigation componentThe navigation component is recreated on every render. Consider memoizing it with useMemo since it only depends on courseId.
- const navigation = ( - <ActivityAnalyticsNavigation courseId={courseId as string} /> - ) + const navigation = useMemo( + () => <ActivityAnalyticsNavigation courseId={courseId as string} />, + [courseId] + )packages/types/src/index.ts (1)
11-15: Consider adding JSDoc comments to document the ActivityType enum.Adding documentation would help clarify the purpose and use case of each activity type.
+/** + * Represents different types of learning activities available in the system + */ export enum ActivityType { + /** Real-time quiz sessions with immediate feedback */ LIVE_QUIZ = 'LIVE_QUIZ', + /** Self-paced practice quizzes for revision */ PRACTICE_QUIZ = 'PRACTICE_QUIZ', + /** Bite-sized learning modules */ MICRO_LEARNING = 'MICRO_LEARNING', + /** Collaborative learning activities for groups */ GROUP_ACTIVITY = 'GROUP_ACTIVITY', }packages/graphql/src/public/schema.graphql (1)
33-39: LGTM! Consider adding documentation.The
ActivityProgresstype is well-structured with clear field names and appropriate nullability. Consider adding documentation comments to describe the semantic meaning of each count field (e.g., when is an activity considered "started" vs "completed").type ActivityProgress { + """ + Name of the activity being tracked + """ activityName: String! + """ + Type of the activity (e.g., GROUP_ACTIVITY, LIVE_QUIZ, etc.) + """ activityType: ActivityType! + """ + Number of participants who started but haven't completed the activity + """ startedCount: Int! + """ + Number of participants who repeated the activity after completion + """ repeatedCount: Int + """ + Number of participants who completed the activity + """ completedCount: Int! }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/frontend-manage/src/components/analytics/AnalyticsErrorView.tsx(1 hunks)apps/frontend-manage/src/components/analytics/AnalyticsLoadingView.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/ActivityProgressPlot.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceAnalyticsNavigation.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/StackedProgress.tsx(1 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx(3 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx(1 hunks)packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql(1 hunks)packages/graphql/src/ops.schema.json(4 hunks)packages/graphql/src/ops.ts(6 hunks)packages/graphql/src/public/client.json(1 hunks)packages/graphql/src/public/schema.graphql(3 hunks)packages/graphql/src/public/server.json(1 hunks)packages/graphql/src/schema/analytics.ts(2 hunks)packages/graphql/src/schema/query.ts(2 hunks)packages/graphql/src/services/analytics.ts(2 hunks)packages/graphql/src/services/stacks.ts(0 hunks)packages/i18n/messages/de.ts(2 hunks)packages/i18n/messages/en.ts(3 hunks)packages/types/src/index.ts(1 hunks)
🔥 Files not summarized due to errors (1)
- packages/graphql/src/ops.ts: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (1)
- packages/graphql/src/services/stacks.ts
🔇 Additional comments (22)
packages/graphql/src/ops.ts (4)
55-62: LGTM! Well-structured type definitions for activity tracking.
The ActivityProgress type and ActivityType enum are well-defined with clear field names and proper typing. The optional repeatedCount field is correctly marked with Maybe<T>.
Also applies to: 70-75
352-357: LGTM! Clear and concise analytics type definition.
The CoursePerformanceAnalytics type effectively combines course-level metrics with detailed activity progress data.
2045-2045: LGTM! Query return type follows established patterns.
The new query field is correctly typed as Maybe<CoursePerformanceAnalytics>, maintaining consistency with other query definitions.
4140-4140: LGTM! Well-structured GraphQL document definition.
The GetCoursePerformanceAnalyticsDocument is properly defined with comprehensive field selections and correct typing.
packages/graphql/src/services/analytics.ts (1)
103-103: Confirm the use of findUnique with composite keys.
Using findUnique with multiple fields requires a composite unique constraint in the Prisma schema.
Run the following script to check for a composite unique constraint on (id, ownerId):
packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql (1)
1-13: LGTM!
The GraphQL query is well-structured and aligns with the schema definitions.
apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx (1)
36-39: LGTM: Improved error and loading states
The replacement of generic components with analytics-specific ones improves user experience and maintains consistency across the analytics section.
Also applies to: 46-49
packages/graphql/src/schema/analytics.ts (2)
164-182: LGTM: Well-structured performance analytics schema
The CoursePerformanceAnalytics type is well-defined with clear relationships and proper typing.
141-144: Good use of regions for code organization
The code is well-organized using regions to separate different analytics domains.
packages/graphql/src/public/client.json (1)
121-121: LGTM!
The new operation hash is properly formatted and follows the existing pattern.
packages/graphql/src/schema/query.ts (2)
16-16: LGTM!
The import is properly added and follows the existing import pattern.
773-782: LGTM!
The new query field is well-structured:
- Uses proper authentication
- Has correct type annotations
- Takes required courseId parameter
- Follows the established pattern of delegating to a service
packages/graphql/src/public/schema.graphql (3)
46-51: LGTM!
The ActivityType enum follows a consistent naming convention and covers all activity types in the system.
307-311: LGTM!
The CoursePerformanceAnalytics type provides a clean structure for aggregating activity progress data at the course level.
1278-1278: LGTM!
The getCoursePerformanceAnalytics query is well-defined with appropriate parameter typing.
packages/i18n/messages/en.ts (1)
1822-1822: LGTM!
The translations are clear, consistent, and cover all new analytics features. The dashboard name change to "Performance and Progress Dashboard" better reflects its expanded functionality.
Also applies to: 1848-1851
packages/graphql/src/public/server.json (1)
121-121: LGTM!
The server configuration correctly includes the new performance analytics query with appropriate typing.
packages/i18n/messages/de.ts (2)
1834-1834: LGTM: Updated performance dashboard label
The label change from "Leistungs-Dashboard" to "Leistungs- und Fortschritts-Dashboard" better reflects the expanded scope of metrics being tracked.
1860-1863: LGTM: Added translations for asynchronous activity progress
The new translations for asynchronous activity progress states are clear and consistent:
- "Fortschritt in asynchronen Aktivitäten" (progress in asynchronous activities)
- "Gestartet" (started)
- "Abgeschlossen" (completed)
- "Wiederholt" (repeated)
packages/graphql/src/ops.schema.json (3)
279-366: LGTM! Well-structured type definition for activity progress tracking.
The ActivityProgress type is well-designed with:
- Strong typing with non-null constraints where appropriate
- Clear separation of counts (started, completed, repeated)
- Good use of enums for activity categorization
3554-3621: LGTM! Well-designed analytics aggregation type.
The CoursePerformanceAnalytics type provides a good structure for course-level analytics:
- Proper nesting of activity progress data
- Essential course metrics (name, total participants)
- All critical fields marked as non-null
19018-19046: LGTM! Query field follows schema conventions.
The getCoursePerformanceAnalytics query is well-defined:
- Consistent with other analytics queries
- Required courseId parameter
- Returns the appropriate analytics type
apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
Outdated
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/ActivityProgressPlot.tsx
Outdated
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/ActivityProgressPlot.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/StackedProgress.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/StackedProgress.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx (1)
Line range hint
71-86: Consider implementing Incremental Static Regeneration (ISR).The current implementation uses blocking fallback. Consider adding a revalidation period to improve performance while keeping data fresh.
export async function getStaticProps({ locale }: GetStaticPropsContext) { return { props: { messages: (await import(`@klicker-uzh/i18n/messages/${locale}`)).default, }, + revalidate: 60, // Revalidate pages every 60 seconds } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/frontend-manage/src/components/analytics/performance/ActivityProgressPlot.tsx(1 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/frontend-manage/src/components/analytics/performance/ActivityProgressPlot.tsx
🔇 Additional comments (4)
apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx (4)
1-10: LGTM! Well-organized imports.
The imports are logically structured and all imported components are utilized in the code.
16-16: Ensure safe handling of courseId.
The type assertion as string without proper validation could lead to runtime errors.
26-44: LGTM! Comprehensive loading and error handling.
The implementation properly handles all states:
- Loading state prevents UI flashing by checking both loading flag and courseId
- Error handling covers all edge cases (null, undefined, and error states)
61-64: Verify ActivityProgressPlot prop types.
Let's ensure the props passed to ActivityProgressPlot match its expected types.
✅ Verification successful
Let me gather more information about the ActivityProgressPlot component definition.
Let me try a different pattern to locate the ActivityProgressPlot component definition.
Based on the results, I can now generate the final response as we have found both the component definition and the type information:
Props match expected types - no changes needed
The props passed to ActivityProgressPlot match its type definition exactly:
activityProgressesis correctly typed asActivityProgress[]where ActivityProgress is a well-defined type from the GraphQL schemaparticipantsis correctly typed asnumber
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ActivityProgressPlot component definition
ast-grep --pattern 'interface $_ {
activityProgresses: $_;
participants: $_;
}'
# Search for any type definitions related to activityProgresses
ast-grep --pattern 'type ActivityProgress = {
$$$
}'
Length of output: 673
Script:
#!/bin/bash
# Search for ActivityProgressPlot component and its props
ast-grep --pattern 'const ActivityProgressPlot = ({ $_ }: { $_ }) => {
$$$
}'
# Also search for any type definitions or interfaces for the component
ast-grep --pattern 'interface ActivityProgressPlotProps {
$$$
}'
Length of output: 150
Script:
#!/bin/bash
# Search for ActivityProgressPlot component using ripgrep for broader context
rg "ActivityProgressPlot" -A 10 -B 5
# Also search for any exports or type definitions
rg "export.*ActivityProgressPlot"
Length of output: 5922
|
Summary by CodeRabbit
Release Notes
New Features
AnalyticsErrorViewandAnalyticsLoadingViewcomponents for improved error and loading state handling in analytics.ActivityProgressPlot,PerformanceAnalyticsNavigation, andStackedProgresscomponents to enhance performance analytics visualization.GetCoursePerformanceAnalyticsfor retrieving detailed course performance analytics.Improvements
PerformanceDashboardandActivityDashboardcomponents to utilize new loading and error components, improving user experience.Schema Changes
ActivityProgressandCoursePerformanceAnalyticsto the GraphQL schema for better tracking of activity metrics.ActivityTypeenum to categorize activities more effectively.