enhance(apps/analytics): allow switching between courses and activity on analytics dashboards #4406
Conversation
|
Warning Rate limit exceeded@sjschlapbach has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new Changes
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: 2
🧹 Nitpick comments (3)
apps/frontend-manage/src/components/analytics/overview/AnalyticsNavigation.tsx (3)
1-19: Consider improving type safety and backward compatibilityThe interface change makes
slugrequired, which could break existing usages. Consider:
- Making
slugoptional with a default value- Adding TypeScript types for GraphQL operations
- import { GetUserCoursesDocument } from '@klicker-uzh/graphql/dist/ops' + import { GetUserCoursesDocument, GetUserCoursesQuery } from '@klicker-uzh/graphql/dist/ops' interface AnalyticsNavigationProps { hrefLeft: string labelLeft: React.ReactNode hrefRight: string labelRight: React.ReactNode - slug: string + slug?: string }
33-35: Enhance loading and error state handlingConsider improving the user experience during loading and error states:
- Add a more contextual loading state
- Include error state UI
- Consider skeleton loading for better UX
+ if (error) { + return ( + <div className="text-red-600"> + {t('shared.errors.failedToLoadCourses')} + </div> + ) + } if (loading) { - return <Loader /> + return ( + <div className="mb-6 grid w-full grid-cols-2 md:grid-cols-3"> + <div className="animate-pulse h-8 bg-gray-200 rounded" /> + <div className="hidden md:block animate-pulse h-8 bg-gray-200 rounded" /> + <div className="animate-pulse h-8 bg-gray-200 rounded" /> + </div> + ) }
39-45: Enhance navigation links accessibility and interaction feedbackThe navigation links could benefit from improved accessibility and user interaction:
<Link href={hrefLeft} - className="flex flex-row items-center justify-start gap-2" + className="flex flex-row items-center justify-start gap-2 hover:text-primary-600 focus:outline-none focus:ring-2 focus:ring-primary-500 rounded-md transition-colors" + aria-label={typeof labelLeft === 'string' ? labelLeft : 'Previous section'} >Also applies to: 63-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/frontend-manage/src/components/analytics/activity/ActivityAnalyticsNavigation.tsx(1 hunks)apps/frontend-manage/src/components/analytics/overview/AnalyticsNavigation.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceAnalyticsNavigation.tsx(1 hunks)apps/frontend-manage/src/components/analytics/quiz/QuizSelectionNavigation.tsx(1 hunks)
🔇 Additional comments (3)
apps/frontend-manage/src/components/analytics/performance/PerformanceAnalyticsNavigation.tsx (1)
12-12: LGTM! Verify navigation path consistency.
The addition of slug="performance" is clean and aligns with the PR objective.
Let's verify the navigation paths are consistent across the analytics components:
✅ Verification successful
Navigation paths are consistent across analytics components
The verification confirms that navigation paths follow a consistent pattern /analytics/${courseId}/(performance|activity|quizzes) across all analytics components. The circular navigation flow is properly maintained:
- Performance → Activity → Quizzes → Performance
- Each component correctly links to its adjacent sections
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent analytics navigation paths
# Expected pattern: /analytics/${courseId}/(performance|activity|quizzes)
rg -g '*.tsx' -g '*.ts' '/analytics/\$\{courseId\}/(performance|activity|quizzes)'
Length of output: 1170
apps/frontend-manage/src/components/analytics/activity/ActivityAnalyticsNavigation.tsx (1)
12-12: LGTM! Implementation is consistent.
The addition of slug="activity" follows the same pattern as other analytics navigation components.
apps/frontend-manage/src/components/analytics/quiz/QuizSelectionNavigation.tsx (1)
12-12: Consider consistent naming patterns.
While the implementation is correct, there are two potential consistency improvements to consider:
- Component naming: Other components follow the pattern
{Section}AnalyticsNavigation(e.g.,ActivityAnalyticsNavigation), while this usesQuizSelectionNavigation - Slug value: Uses plural "quizzes" while URL patterns typically use singular form
Consider renaming to maintain consistency:
-function QuizSelectionNavigation({ courseId }: { courseId: string }) {
+function QuizAnalyticsNavigation({ courseId }: { courseId: string }) {
return (
<AnalyticsNavigation
hrefLeft={`/analytics/${courseId}/performance`}
labelLeft={<PerformanceDashboardLabel />}
hrefRight={`/analytics/${courseId}/activity`}
labelRight={<ActivityDashboardLabel />}
- slug="quizzes"
+ slug="quiz"
/>
)
}
-export default QuizSelectionNavigation
+export default QuizAnalyticsNavigationLet's verify the component naming patterns across the codebase:
apps/frontend-manage/src/components/analytics/overview/AnalyticsNavigation.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/overview/AnalyticsNavigation.tsx
Show resolved
Hide resolved
…e same course on quiz dashboard
|
Summary by CodeRabbit
New Features
AnalyticsNavigationcomponents across various sections (Activity, Overview, Performance, Quiz) with newslugprops for improved functionality.Bug Fixes