-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
13 navigation refinement from home to solution #18
13 navigation refinement from home to solution #18
Conversation
WalkthroughThis pull request delivers a broad range of updates across the project. It adds new configuration settings for Git, VS Code, and Supabase while introducing several new React components and pages for enhanced navigation, resource display, and exercise handling. Key additions include improved exercise question fetching with a dedicated action, updates to layout components (such as Navbar and BottomNavigation), and the integration of new UI libraries and dependencies. The changes also include formatting improvements and the introduction of new utility functions and custom hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant B as Browser
participant E as ExercisePage Component
participant Q as getExerciseQuestions (Action)
participant DB as Database (Prisma)
B->>E: Request Exercise Page
E->>Q: Call getExerciseQuestions(chapterId, exerciseId)
Q->>DB: Query exercise questions
DB-->>Q: Return questions
Q-->>E: Deliver questions data
E->>B: Render Loading state then TopBar & ExerciseContent with questions
Possibly related issues
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 17
🔭 Outside diff range comments (1)
next.config.ts (1)
11-18
:🧩 Analysis chain
Consider restricting image domains for security.
The current configuration allows images from any HTTPS domain. Consider restricting this to specific trusted domains to prevent potential security issues.
🌐 Web query:
What are the security best practices for Next.js image domains configuration?
💡 Result:
Next.js provides robust security controls for image domain configuration through its
remotePatterns
system innext.config.js
, which replaced the deprecateddomains
approach in modern versions (15+). Here are the key security best practices:1. Use Strict Remote Patterns
Configure explicit allowed sources rather than broad wildcards:
// next.config.js module.exports = { images: { remotePatterns: [ { protocol: 'https', hostname: 'assets.yourdomain.com', port: '', pathname: '/cdn/**', } ], }, }This restricts images to HTTPS from a specific subdomain and path [5][6].
2. Avoid Insecure Wildcards
While technically possible, never use:
hostname: '*' // Security risk [1][4] hostname: '**.example.com' // Still risky [5]Instead use explicit subdomains:
hostname: 'cdn.example.com' [6]3. Layer Security Controls
Feature Security Benefit Protocol restriction Blocks HTTP images Port specification Prevents alternate ports Path restrictions Limits exposure surface Search params Validates query strings 4. Deprecation Handling
Migrate from old
domains
toremotePatterns
:// Before (insecure) domains: ['example.com'] // After (secure) remotePatterns: [ { protocol: 'https', hostname: 'images.example.com', pathname: '/uploads/**' } ]This prevents blanket access to all subpaths [6][9].
5. Additional Protections
- Set
dangerouslyAllowSVG: false
unless absolutely required [5]- Use
contentSecurityPolicy
headers for extra XSS protection [9]- Keep Next.js updated (v15+ recommended) [1][6]
For dynamic image sources requiring flexibility, consider:
unoptimized: true // Disables optimization but maintains same-origin security [1][3]However this removes automatic format/size optimization benefits.
These practices help prevent image-based attack vectors while maintaining Next.js' performance advantages. Always prefer explicit allow-listing over broad permissions [2][7].
Citations:
- 1: https://dev.to/joodi/allow-all-domains-for-images-in-nextjs-hpj
- 2: https://app.studyraid.com/en/read/1903/31025/security-best-practices-in-nextjs
- 3: https://sentry.io/answers/how-to-allow-domains-for-images-in-next-js/
- 4: https://dev.to/bmstefanski/how-to-use-wildcard-domains-with-nextjs-image-component-5dga
- 5: https://nextjs.org/docs/pages/api-reference/components/image
- 6: https://www.timsanteford.com/posts/migrating-next-js-image-domains-to-remote-patterns/
- 7: https://nextjs.org/docs/pages/building-your-application/optimizing/images
- 8: Image is not from a permitted domain vercel/next.js#25597
- 9: https://nextjs.org/docs/pages/building-your-application/configuring/content-security-policy
Security: Restrict Remote Image Domains
- The current configuration in
next.config.ts
uses a wildcard (hostname: "**"
) that allows images from any HTTPS domain.- According to Next.js best practices, explicit allow-listing is recommended instead of using broad wildcards. For example, restrict access to trusted domains by specifying explicit hostnames.
- This overly permissive setting poses a security risk as it potentially allows untrusted sources to serve images.
🧹 Nitpick comments (26)
app/globals.css (1)
9-11
: CSS Addition for Safe Area Insets
The new.safe-area-inset-bottom
class correctly uses theenv(safe-area-inset-bottom)
property to accommodate device safe areas. Consider adding a fallback value (e.g.,env(safe-area-inset-bottom, 0px)
) to ensure graceful degradation on browsers that do not support the environment variable syntax.components/resources/chapter-accordion.tsx (1)
23-30
: Remove or integrate the unused function.
getQuestionCount
is defined but never used. If it's intended for future logic, consider adding a TODO comment. Otherwise, remove it to keep the code clean.-function getQuestionCount( - chapter: ChapterWithContent, - resourceType: ResourceType -): number { - return resourceType === ResourceType.TEXTBOOK - ? chapter.topics.reduce((acc, topic) => acc + topic.questions.length, 0) - : chapter.questions?.length ?? 0; -}🧰 Tools
🪛 ESLint
[error] 23-23: 'getQuestionCount' is defined but never used.
(@typescript-eslint/no-unused-vars)
components/subjects/resource-list.tsx (2)
61-61
: Prefer using Math.SQRT2 for better accuracy.Using
1.414
is approximate. Adopting a built-in constant improves precision and readability:-const a4Height = 176 * 1.414; // width (176px) * aspect ratio +const a4Height = 176 * Math.SQRT2; // Use the built-in constant🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Prefer constants from the standard library.
Unsafe fix: Use Math.SQRT2 instead.
(lint/suspicious/noApproximativeNumericConstant)
129-175
: No need for an async function if no await calls exist.Currently,
ResourceList
is marked asasync
without awaiting any promises. Consider removingasync
to avoid confusion:-export async function ResourceList({ +export function ResourceList({ resources = [], }: Readonly<ResourceListProps>) {app/(public)/(main-navigation)/home/loading.tsx (1)
3-10
: Consider enhancing the loading state with a spinner or skeleton.The current loading state is quite basic. Consider using a loading spinner or skeleton UI to provide a better user experience.
Example implementation using a spinner:
const Loading = () => { - // Home page layout return ( <div className='h-full w-full flex items-center justify-center'> - <p>Loading...</p> + <div className="flex flex-col items-center gap-2"> + <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-primary" /> + <p>Loading resources...</p> + </div> </div> ) }features/exercise/components/top-bar.tsx (1)
6-17
: Add accessibility improvements for navigation.While the back button has an
aria-label
, consider adding a more descriptive label for better screen reader support.Apply this diff to improve accessibility:
- <ActionIcon variant="transparent" color="black" aria-label="Back"> + <ActionIcon variant="transparent" color="black" aria-label="Back to home page">app/(public)/(main-navigation)/more/page.tsx (2)
13-16
: Implement the TODO sections and remove placeholder text.The component contains placeholder text and a TODO comment. Please implement these sections or create issues to track their implementation.
Would you like me to help create issues to track the implementation of these sections?
8-12
: Enhance semantic HTML structure.Consider using more semantic HTML elements for better accessibility:
-<div className="p-4 bg-white rounded-lg shadow"> +<article className="p-4 bg-white rounded-lg shadow"> <h2 className="text-lg font-semibold mb-2">Current Plan</h2> - <p>Account Information w/ selected plan</p> - <p>On current plan : show switch plan or cancel subscription</p> + <div className="space-y-2"> + <p>Account Information w/ selected plan</p> + <p>On current plan : show switch plan or cancel subscription</p> + </div> -</div> +</article>app/(public)/layout.tsx (1)
19-28
: Add error boundary for NextTopLoader.Consider wrapping the NextTopLoader component with an error boundary to gracefully handle any potential errors during page transitions.
+import { ErrorBoundary } from '@/components/error-boundary'; <section className="relative"> - <NextTopLoader /> + <ErrorBoundary fallback={<div />}> + <NextTopLoader /> + </ErrorBoundary> <div className="min-h-screen flex flex-col">app/(public)/(main-navigation)/home/page.tsx (1)
8-12
: Add loading state handling.Consider using React Suspense or a loading state to provide feedback during resource fetching.
+import { Suspense } from "react"; +import { LoadingSpinner } from "@/components/loading-spinner"; -const { resources } = (await getResources({ +const ResourcesContent = async () => { + const { resources } = (await getResources({ status: ResourceStatus.LIVE, grade: grade ? Number(grade) : undefined, limit: 15, - })) || { resources: [], total: 0, pages: 0 }; + })) || { resources: [], total: 0, pages: 0 }; + return <ResourceList resources={resources} />; +}; +return ( + <section> + <Suspense fallback={<LoadingSpinner />}> + <ResourcesContent /> + </Suspense> + </section> +);lib/constants.tsx (2)
12-12
: Consider expanding and organizing SUBJECTS array.The SUBJECTS array is limited and could be better organized. Consider:
- Adding all subjects from the emoji mapping
- Grouping subjects by category
-export const SUBJECTS: string[] = ["Mathematics", "Science", "Accounting", "Life Sciences"]; +export const SUBJECTS: string[] = Object.keys(subjectEmojis);
1-10
: Optimize Grade interface and values.The Grade interface has redundant key/label values, and the grade format is inconsistent.
interface Grade { key: string; - label: string; + value: number; } export const GRADES: Grade[] = [ - { key: "Gr12", label: "Gr12" }, - { key: "Gr11", label: "Gr11" }, - { key: "Gr10", label: "Gr10" }, + { key: "Grade 12", value: 12 }, + { key: "Grade 11", value: 11 }, + { key: "Grade 10", value: 10 }, ];features/resources/utils/resources.utils.ts (3)
4-19
: Consider adding input validation and pagination.The function could benefit from:
- Input validation for
grade
andlimit
parameters- Pagination support for better performance with large datasets
export async function getResourcesData(grade?: number, limit?: number) { + if (grade && (grade < 1 || grade > 12)) { + throw new Error('Grade must be between 1 and 12'); + } + if (limit && limit < 1) { + throw new Error('Limit must be positive'); + } try { const resources = await prisma.resource.findMany({ where: { status: ResourceStatus.LIVE, grade: grade, }, take: limit, + orderBy: { createdAt: 'desc' }, // Add sorting for consistent pagination }); return { resources }; } catch (error) { console.error("Error fetching resources:", error); - return { resources: [] }; // Or throw the error if you want to handle it higher up + throw error; // Let caller handle the error } }
21-30
: Consider using Object.values(ResourceType) for initialization.Instead of manually listing resource types, use
Object.values(ResourceType)
to ensure all types are covered.export function groupResourceByType(resources: Resource[]) { return resources.reduce<Record<ResourceType, Resource[]>>( (acc, resource) => { acc[resource.type] = acc[resource.type] || []; acc[resource.type].push(resource); return acc; }, - { PAST_PAPER: [], TEXTBOOK: [], STUDY_GUIDE: [] } + Object.values(ResourceType).reduce((obj, type) => ({ ...obj, [type]: [] }), {}) ); }
32-43
: Consider adding type safety for subject strings.The function uses string literals for subjects which could lead to inconsistencies.
Consider creating a
Subject
enum or type to ensure consistency:export enum Subject { MATHEMATICS = 'MATHEMATICS', PHYSICS = 'PHYSICS', // ... other subjects } export function groupResourcesBySubject(resources: Resource[]) { return resources.reduce<Record<Subject, Resource[]>>( (acc, resource) => { if (!acc[resource.subject as Subject]) { acc[resource.subject as Subject] = []; } acc[resource.subject as Subject].push(resource); return acc; }, Object.values(Subject).reduce((obj, subject) => ({ ...obj, [subject]: [] }), {}) ); }components/bottom-navigation.tsx (3)
8-13
: Consider extracting navigation items to a configuration file.Moving navigation items to a separate config file would improve maintainability.
-const navItems = [ - { href: "/home", icon: Home, label: "Home" }, - { href: "/search", icon: Search, label: "Premium" }, - { href: "/pricing", icon: Gem, label: "Premium" }, - { href: "/more", icon: Menu, label: "More" }, -];Create a new file
config/navigation.ts
:import { Home, Search, Gem, Menu } from "lucide-react"; export const navItems = [ { href: "/home", icon: Home, label: "Home" }, { href: "/search", icon: Search, label: "Search" }, { href: "/pricing", icon: Gem, label: "Premium" }, { href: "/more", icon: Menu, label: "More" }, ] as const;
19-22
: Consider adding theme-aware background color.The navigation bar uses a fixed white background which might not work well with dark theme.
- className="flex justify-around fixed bottom-0 left-0 right-0 p-2 bg-white shadow-md z-50 safe-area-inset-bottom" + className="flex justify-around fixed bottom-0 left-0 right-0 p-2 bg-background shadow-md z-50 safe-area-inset-bottom"
23-43
: Consider extracting navigation item to a separate component.The navigation item rendering logic could be moved to a separate component for better maintainability.
Create a new component
NavigationItem.tsx
:interface NavigationItemProps { href: string; icon: LucideIcon; label: string; isActive: boolean; } const NavigationItem = ({ href, icon: Icon, label, isActive }: NavigationItemProps) => ( <Link href={href} aria-current={isActive ? "page" : undefined} aria-label={label} className="flex flex-col justify-center items-center gap-2" > <Icon className={isActive ? "text-blue-600" : "text-gray-600"} /> <p className={`text-xs ${isActive ? "text-blue-600" : "text-gray-600"}`}> {label} </p> </Link> );app/(public)/exercises/[chapterId]/[exerciseId]/page.tsx (1)
73-73
: Remove console.log statement.Production code should not contain console.log statements.
- console.log("Questions From Excerse", questions)
__mocks__/resources.ts (1)
4-83
: Consider using a factory function for generating mock data.Using a factory function would make it easier to generate dynamic test data.
interface MockResourceOptions { type?: ResourceTypeEnum; year?: number; subject?: string; } export function createMockResource(options: MockResourceOptions = {}) { const defaults = { type: ResourceTypeEnum.Textbook, year: new Date().getFullYear(), subject: 'Mathematics', }; return { ...defaults, ...options, title: `Mock ${options.type || defaults.type}`, content: `Mock content for ${options.subject || defaults.subject}`, }; } // Usage: export const dummyResources = [ createMockResource({ type: ResourceTypeEnum.Textbook }), createMockResource({ type: ResourceTypeEnum.PastPaper, year: 2023 }), // ... ];app/(public)/resources/[resourceId]/page.tsx (2)
69-70
: Remove unnecessary whitespace.The empty string literal
{" "}
is unnecessary and can be removed.- {" "}
72-78
: Add alt text based on resource title.The current alt text "Book Cover" is generic. Consider using the resource title for better accessibility.
- alt="Book Cover" + alt={`${resource.title} Cover`}components/ui/tabs-variants.tsx (1)
27-46
: Consider extracting long class strings into constants.The class strings in TabsTriggerVariants are quite long and could be more maintainable if extracted into named constants.
+const DEFAULT_TRIGGER_STYLES = "inline-flex items-center justify-center whitespace-nowrap rounded-md px-3 py-1 text-sm font-medium ring-offset-background transition-all focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:bg-background data-[state=active]:text-foreground data-[state=active]:shadow"; + +const UNDERLINE_TRIGGER_STYLES = "bg-background border-b-2 border-background focus:border-primary ring-0 outline-none shadow-none data-[state=active]:shadow-none data-[state=active]:border-b-2 data-[state=active]:border-primary disabled:opacity-100 data-[state=active]:shadow-none rounded-none m-0 pt-1.5 pb-2 hover:bg-background-muted"; + const TabsTriggerVariants = cva( "inline-flex items-center justify-center whitespace-nowrap text-sm font-normal transition-all disabled:pointer-events-none data-[state=active]:text-foreground px-3", { variants: { variant: { - default: - "inline-flex items-center justify-center whitespace-nowrap rounded-md px-3 py-1 text-sm font-medium ring-offset-background transition-all focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:bg-background data-[state=active]:text-foreground data-[state=active]:shadow", - underline: - "bg-background border-b-2 border-background focus:border-primary ring-0 outline-none shadow-none data-[state=active]:shadow-none data-[state=active]:border-b-2 data-[state=active]:border-primary disabled:opacity-100 data-[state=active]:shadow-none rounded-none m-0 pt-1.5 pb-2 hover:bg-background-muted", + default: DEFAULT_TRIGGER_STYLES, + underline: UNDERLINE_TRIGGER_STYLES, },app/(public)/subjects/[subjectName]/page.tsx (1)
31-39
: Add error handling for image loading.Consider adding an error fallback for when the placeholder image fails to load.
<Image src="/placeholder-book.png" alt={resource.title} width={80} height={96} className="object-cover" + onError={(e) => { + e.currentTarget.src = "/fallback-placeholder.png"; + }} />components/admin/resources/questions-display.tsx (2)
37-38
: Use more descriptive variable names.The variable name
test
is not descriptive of its purpose. Consider renaming it to something more meaningful.- const test = question.resource.type; - const displayType = test == "TEXTBOOK" ? "Exercise" : "Question"; + const resourceType = question.resource.type; + const displayType = resourceType === "TEXTBOOK" ? "Exercise" : "Question";
86-102
: Add error handling for image loading.Consider adding error handling for when images fail to load.
<img src={block.imageData.url} alt={block.imageData.caption || "Question image"} className="max-w-full" + onError={(e) => { + e.currentTarget.src = "/fallback-question-image.png"; + e.currentTarget.alt = "Failed to load question image"; + }} />🧰 Tools
🪛 ESLint
[error] 87-87: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (42)
NOTES.md
(1 hunks).gitignore
(1 hunks).vscode/extensions.json
(1 hunks).vscode/settings.json
(1 hunks)__mocks__/resources.ts
(1 hunks)actions/questions.ts
(1 hunks)actions/resources.ts
(5 hunks)app/(public)/(main-navigation)/home/loading.tsx
(1 hunks)app/(public)/(main-navigation)/home/page.tsx
(1 hunks)app/(public)/(main-navigation)/layout.tsx
(1 hunks)app/(public)/(main-navigation)/more/page.tsx
(1 hunks)app/(public)/(main-navigation)/pricing/page.tsx
(1 hunks)app/(public)/exercises/[chapterId]/[exerciseId]/page.tsx
(1 hunks)app/(public)/layout.tsx
(1 hunks)app/(public)/page.tsx
(3 hunks)app/(public)/resources/[resourceId]/layout.tsx
(1 hunks)app/(public)/resources/[resourceId]/page.tsx
(1 hunks)app/(public)/subjects/[subjectName]/page.tsx
(1 hunks)app/globals.css
(1 hunks)app/layout.tsx
(1 hunks)components/admin/resources/add-question-dialog.tsx
(10 hunks)components/admin/resources/questions-display.tsx
(6 hunks)components/admin/solutions/content-editor.tsx
(2 hunks)components/bottom-navigation.tsx
(1 hunks)components/layout/navbar.tsx
(1 hunks)components/pricing/pricing-table.tsx
(2 hunks)components/providers/providers.tsx
(1 hunks)components/resources/chapter-accordion.tsx
(1 hunks)components/subject-resources.tsx
(5 hunks)components/subjects/resource-list.tsx
(2 hunks)components/ui/tabs-variants.tsx
(1 hunks)features/exercise/components/exercise-content.tsx
(1 hunks)features/exercise/components/top-bar.tsx
(1 hunks)features/exercise/store/exerciseStore.ts
(1 hunks)features/resources/hooks/useResources.ts
(1 hunks)features/resources/utils/resources.utils.ts
(1 hunks)lib/constants.tsx
(1 hunks)next.config.ts
(2 hunks)package.json
(5 hunks)supabase/.gitignore
(1 hunks)supabase/config.toml
(1 hunks)tailwind.config.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (6)
- .vscode/extensions.json
- .gitignore
- .vscode/settings.json
- supabase/.gitignore
- components/layout/navbar.tsx
- NOTES.md
🧰 Additional context used
🪛 ESLint
features/resources/hooks/useResources.ts
[error] 1-1: 'Resource' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 2-2: 'getSuggestedSubjects' is defined but never used.
(@typescript-eslint/no-unused-vars)
components/admin/resources/questions-display.tsx
[error] 87-87: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 121-121: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
actions/resources.ts
[error] 14-14: 'ResourcesResponse' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 188-188: 'error' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 275-275: 'error' is defined but never used.
(@typescript-eslint/no-unused-vars)
components/subject-resources.tsx
[error] 12-12: 'Tabs' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 12-12: 'TabsContent' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 12-12: 'TabsList' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 12-12: 'TabsTrigger' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 13-13: 'RESOURCE_TYPES' is defined but never used.
(@typescript-eslint/no-unused-vars)
components/resources/chapter-accordion.tsx
[error] 23-23: 'getQuestionCount' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 Biome (1.9.4)
components/subjects/resource-list.tsx
[error] 61-61: Prefer constants from the standard library.
Unsafe fix: Use Math.SQRT2 instead.
(lint/suspicious/noApproximativeNumericConstant)
🔇 Additional comments (29)
app/(public)/(main-navigation)/pricing/page.tsx (1)
24-25
: LGTM! Layout improvements enhance visual hierarchy.The increased vertical padding and margin create better spacing and visual separation between sections.
components/pricing/pricing-table.tsx (2)
10-10
: LGTM! Import cleanup.The removal of unused PlanDescription import improves code cleanliness.
149-149
: LGTM! Enhanced responsive layout.The grid layout improvements provide better adaptability across different screen sizes:
- Mobile: Single column
- Tablet: Two columns
- Desktop: Three columns
tailwind.config.ts (2)
5-5
: LGTM!The content paths are correctly configured to include all relevant file types across pages, components, and app directories.
57-57
: Verify the removal of tailwindcss-animate plugin.The removal of
tailwindcss-animate
plugin might affect existing animations in the codebase.Let's verify if there are any dependencies on the removed animation plugin:
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for any usage of Tailwind's animation classes rg -i 'animate-' --type ts --type tsx --type jsx --type jsLength of output: 86
I'll run an updated search using explicit file glob patterns (rather than relying on built-in file types) to cover all JavaScript/TypeScript files. This will help verify if any Tailwind animation classes (e.g., those starting with
animate-
) are still in use. Additionally, I'll check if any reference totailwindcss-animate
exists inpackage.json
.Please run the following shell scripts:
#!/bin/bash # Search for any usage of Tailwind animation classes in JS/TS files using explicit globs rg -i 'animate-' -g "*.ts" -g "*.tsx" -g "*.js" -g "*.jsx"#!/bin/bash # Look for any references to tailwindcss-animate in package.json (dependencies or elsewhere) grep -Ri 'tailwindcss-animate' package.jsonOnce these commands complete, we can determine if any parts of the codebase depend on the removed plugin.
next.config.ts (1)
21-24
: Verify Million.js RSC compatibility and performance impact.The configuration enables Million.js with React Server Components (RSC) support. While this can improve performance, we should verify:
- Compatibility with RSC in production
- Impact on build size and runtime performance
✅ Verification successful
🌐 Web query:
Does Million.js support React Server Components (RSC) in production? What are the implications?
💡 Result:
Million.js offers experimental support for React Server Components (RSC) through its
react-server
module ([1][4]), but production usage requires careful consideration of technical implications:Compatibility Status
Aspect Support Level Details Core Integration ✅ Module exists Ships react-server
package for RSC compatibility ([1][4])Framework Requirements ❌ Partial Only works with frameworks implementing RSC (e.g., Next.js) ([6][9]) Compiler Optimization ⚠️ LimitedStatic analysis works best with client components ([5][8]) Key Implications
Execution Context Conflicts
- RSCs prohibit client-side APIs (
useState
,useEffect
) while Million.js optimizations primarily target client-side reconciliation ([9][10])- Solution: Use Million's
block()
only in Client Components ([5][8])Hydration Challenges
// Problematic pattern: 'use server' const ServerCard = block(() => <Card />) // ❌ Can't hydrate server-rendered blocks
- Million's DOM patching requires client hydration capabilities ([8])
Performance Tradeoffs
- 70% faster updates for Client Components ([4][5])
- No speed benefit for pure RSCs (server components don't re-render) ([2][6])
Toolchain Requirements
Requires Next.js 13.4+ or custom RSC implementation ([6][9]), with specific bundler configurations:npx million@latest --experimental-react-serverRecommended Pattern
'use client' import { block } from 'million/react-server' const OptimizedComponent = block( ({ data }) => <InteractivePart {...data} /> // ✅ Client Component wrapper ) // Server Component remains untouched export default function Page() { const data = await fetchData() return <OptimizedComponent data={data} /> }Production use requires thorough testing of:
- Nested component boundaries ([2][6])
- Data serialization between server/client layers ([3][10])
- Compiler interactions with RSC streaming ([7][8])
Current implementation appears stable in Next.js 14 projects according to community reports ([7]), but lacks official documentation for edge cases like Suspense boundaries or progressive hydration.
Citations:
- 1: https://github.com/aidenybai/million
- 2: https://www.joshwcomeau.com/react/server-components/
- 3: https://storybook.js.org/blog/storybook-react-server-components/
- 4: https://www.npmjs.com/package/million/v/2.6.1
- 5: https://www.showwcase.com/article/35537/how-to-make-your-react-app-70percent-faster-with-millionjs
- 6: https://dev.to/kathryngrayson/the-current-state-of-react-server-components-a-guide-for-the-perplexed-31eb
- 7: Support React Server Components facebook/docusaurus#9089
- 8: https://old.million.dev/blog/million-beyond-speed
- 9: https://www.joshwcomeau.com/react/css-in-rsc/
- 10: https://dev.to/xakrume/react-server-components-deep-dive-29af
Million.js RSC Experimental Support Verified
The
rsc: true
setting activates Million.js's experimental support for React Server Components, which is designed to enhance client component performance. Note that:
- The experimental support works best with Next.js (13.4+ or later) and primarily benefits client components.
- Pure server components might not see performance gains, and there can be hydration challenges if client-side optimizations are applied incorrectly.
- Ensure that any component intended for Million optimization is correctly marked (e.g., using
'use client'
) and follows the recommended integration pattern.app/(public)/page.tsx (1)
36-60
: Looks good!The rest of the UI structure and resource retrieval logic appears correct, and the layout changes enhance the user experience.
components/resources/chapter-accordion.tsx (1)
1-203
: Overall implementation approved.The grouping logic and the newly refactored components (TextbookContent, PastPaperContent, etc.) provide clarity and maintainability.
🧰 Tools
🪛 ESLint
[error] 23-23: 'getQuestionCount' is defined but never used.
(@typescript-eslint/no-unused-vars)
components/subjects/resource-list.tsx (1)
2-128
: Tab-based layout and resource grouping are on point!Switching to a tabbed interface efficiently organizes the resources, and the new component structure clearly separates concerns.
🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Prefer constants from the standard library.
Unsafe fix: Use Math.SQRT2 instead.
(lint/suspicious/noApproximativeNumericConstant)
features/resources/hooks/useResources.ts (2)
5-9
: LGTM!The interface is well-defined with appropriate types and optional properties.
11-27
: LGTM!The hook implementation is clean and handles undefined responses gracefully with default values.
features/exercise/store/exerciseStore.ts (3)
3-10
: LGTM!The interface is well-defined with appropriate types and required properties to ensure data consistency.
12-15
: LGTM!The store interface is well-defined with type-safe questions record and setter method.
17-23
: LGTM!The store implementation maintains immutability and correctly handles state updates.
features/exercise/components/top-bar.tsx (1)
19-19
: LGTM!The component is correctly exported as default.
app/layout.tsx (2)
4-4
: LGTM!The changes improve type safety with
Readonly
and correctly integrate Mantine components.Also applies to: 9-12, 14-18
19-28
: LGTM!The layout correctly integrates Mantine components and handles hydration warnings.
features/resources/utils/resources.utils.ts (1)
45-49
: LGTM! Resource type labels are well-defined.The constant provides clear and consistent labels for resource types.
app/(public)/resources/[resourceId]/page.tsx (1)
85-100
: LGTM! Good use of Mantine's Accordion component.The Accordion implementation with custom styling and proper chevron positioning looks good.
components/ui/tabs-variants.tsx (1)
1-10
: LGTM! Well-structured imports and component setup.Good use of "use client" directive and proper organization of imports.
app/(public)/subjects/[subjectName]/page.tsx (1)
104-110
: Uncomment and implement pagination for better UX.The pagination component is currently commented out. Consider implementing it to improve navigation through resources.
Would you like me to help implement the pagination component?
actions/questions.ts (1)
205-243
: LGTM!The
getExerciseQuestions
function is well-implemented with:
- Proper error handling
- Type-safe database queries
- Comprehensive solution data fetching
- Ordered results for consistent display
components/admin/solutions/content-editor.tsx (2)
58-58
: LGTM! Props immutability enforced.Good practice to enforce immutability on props using
Readonly<T>
to prevent accidental mutations.
180-180
: LGTM! Improved initial rendering.Reducing the textarea's min-height to 100px provides a better initial rendering while still allowing for resizing as needed.
components/admin/resources/add-question-dialog.tsx (2)
87-87
: LGTM! Props immutability enforced.Good practice to enforce immutability on props using
Readonly<T>
to prevent accidental mutations.
185-185
: LGTM! Improved UI layout and styling.The changes improve the UI by:
- Increasing dialog width for better content display
- Better placeholder text
- Consistent styling for form labels
Also applies to: 243-244, 314-316, 353-355
package.json (1)
15-19
: Verify compatibility of new dependencies.Please verify:
- Compatibility between Mantine UI and existing UI components
- Impact of Million.js on build performance
- PostCSS version compatibility with Next.js
Run the following script to check for potential conflicts and vulnerabilities:
Also applies to: 46-46, 62-63, 76-78
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for dependency conflicts and vulnerabilities # Check for dependency conflicts npm ls @mantine/core @radix-ui/react-select # Check for known vulnerabilities npm auditLength of output: 129
Dependency and Vulnerability Checks Passed
- The dependency conflict check (
npm ls @mantine/core @radix-ui/react-select
) returned no issues, indicating that the Mantine UI dependency integrates well with the existing components.npm audit
reported zero vulnerabilities, suggesting no known issues with the current dependencies.- While the script does not directly measure Million.js build performance or explicitly check PostCSS compatibility with Next.js, no dependency conflicts or vulnerabilities were noted, which indirectly supports compatibility.
Overall, based on the results from the automated checks, there are no immediate issues with the new dependencies. Further runtime benchmarking and manual review of Next.js/PostCSS integration may still be considered as a precaution.
supabase/config.toml (2)
104-109
: LGTM! Good JWT and refresh token configuration.The configuration includes:
- Reasonable JWT expiry time (1 hour)
- Refresh token rotation enabled
- Limited token reuse interval
183-186
: LGTM! Proper handling of sensitive data.Good practice using environment variables for sensitive values like auth tokens and secrets.
Also applies to: 216-218
@@ -1,9 +1,11 @@ | |||
|
|||
import MillionLint from "@million/lint"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify Million.js dependency installation and compatibility.
Let's ensure the Million.js dependency is properly installed and compatible with the current Next.js version.
🏁 Script executed:
#!/bin/bash
# Check if Million.js is installed and get its version
npm list @million/lint || true
# Check package.json for Next.js version
cat package.json | jq '.dependencies.next, .devDependencies.next'
Length of output: 162
Dependency @million/lint Not Found
The import in your next.config.ts
file for MillionLint
points to the @million/lint
package, but our check indicates that this dependency is not installed. Please ensure you add it to your project’s dependencies (or remove its usage if not intended) to avoid runtime issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (7)
components/admin/resources/questions-display.tsx (3)
66-66
: Consider removing the double type assertion.The double type assertion with
unknown
can be simplified by directly asserting toQuestionContent
.- const content = question.content as unknown as QuestionContent; + const content = question.content as QuestionContent;
121-130
: Consider making image dimensions responsive.The fixed dimensions (500x300) might not be optimal for all images. Consider:
- Using dynamic dimensions based on the actual image size
- Implementing responsive sizing using CSS classes
- width={500} // Adjust as needed - height={300} // Adjust as needed + width={0} + height={0} + sizes="100vw" + className="w-full h-auto" style={{ maxWidth: "100%", height: "auto", }}Also applies to: 160-169
123-123
: Improve image accessibility.The alt text could be more descriptive for better screen reader support.
- alt={block.imageData.caption ?? "Question image"} + alt={block.imageData.caption ?? `Supporting image for ${displayType.toLowerCase()} ${question.exerciseNumber}`}Also applies to: 162-162
app/(public)/(main-navigation)/home/page.tsx (2)
11-15
: Consider adding loading state handling.The async data fetching could benefit from proper loading state management to improve user experience.
Consider using React Suspense or a loading state:
+ const [isLoading, setIsLoading] = useState(true); const { resources } = (await getResources({ status: ResourceStatus.LIVE, grade: grade ? Number(grade) : undefined, limit: 15, })) || { resources: [], total: 0, pages: 0 }; + setIsLoading(false);
17-24
: Enhance error message with more details.The error message could be more informative to help users and support staff diagnose issues.
Consider adding more context to the error message:
<div className="p-4 text-red-600 bg-red-50 rounded-md"> <h2 className="font-semibold">Unable to load resources</h2> - <p>Please try refreshing the page or contact support if the issue persists.</p> + <p> + We encountered an error while loading the resources. Please try: + <ul className="list-disc ml-6 mt-2"> + <li>Refreshing the page</li> + <li>Checking your internet connection</li> + <li>Contacting support if the issue persists</li> + </ul> + </p> </div>actions/resources.ts (2)
7-7
: Remove unused import.The
Resource
type is imported but never used.-import { - CurriculumType, - Resource, - ResourceStatus, - ResourceType, -} from "@prisma/client"; +import { + CurriculumType, + ResourceStatus, + ResourceType, +} from "@prisma/client";🧰 Tools
🪛 ESLint
[error] 7-7: 'Resource' is defined but never used.
(@typescript-eslint/no-unused-vars)
159-164
: Add rate limiting for admin verification.The admin verification should be rate limited to prevent brute force attacks.
Consider implementing rate limiting for admin verification:
+import { rateLimit } from '@/lib/rate-limit'; + export async function addResource(data: ResourceFormValues) { try { + const rateLimitResult = await rateLimit.check(req.ip, 'admin-verify', 5, '1m'); + if (!rateLimitResult.success) { + return { error: 'Too many attempts. Please try again later.' }; + } + const { isAdmin, error } = await verifyAdmin(); if (!isAdmin) { return { error: error || "Unauthorized - Admin access required" }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (11)
actions/resources.ts
(5 hunks)app/(public)/(main-navigation)/home/page.tsx
(1 hunks)app/(public)/(main-navigation)/layout.tsx
(1 hunks)app/(public)/exercises/[chapterId]/[exerciseId]/page.tsx
(1 hunks)app/(public)/page.tsx
(3 hunks)app/(public)/resources/[resourceId]/layout.tsx
(1 hunks)components/admin/resources/questions-display.tsx
(5 hunks)components/providers/providers.tsx
(1 hunks)components/subject-resources.tsx
(5 hunks)features/exercise/components/exercise-content.tsx
(1 hunks)features/resources/hooks/useResources.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- app/(public)/(main-navigation)/layout.tsx
- features/resources/hooks/useResources.ts
- app/(public)/resources/[resourceId]/layout.tsx
- components/subject-resources.tsx
- app/(public)/exercises/[chapterId]/[exerciseId]/page.tsx
- features/exercise/components/exercise-content.tsx
🧰 Additional context used
🪛 ESLint
actions/resources.ts
[error] 7-7: 'Resource' is defined but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (6)
components/providers/providers.tsx (3)
1-7
: LGTM!The imports are well-organized and the interface is properly defined.
9-12
: Configure additional theme settings.The theme configuration could be enhanced with additional Mantine theme options.
Apply this diff to add more theme configuration:
const theme = createTheme({ primaryColor: "blue", defaultRadius: "md", + colorScheme: "light", + fontFamily: "Inter, sans-serif", + headings: { + fontFamily: "Inter, sans-serif", + }, });
15-20
: LGTM!The implementation is well-structured with:
- Proper use of Readonly for props immutability
- Correct nesting of providers
- Proper theme integration
components/admin/resources/questions-display.tsx (2)
31-57
: LGTM! Well-structured interfaces for improved type safety.The introduction of proper interfaces (
QuestionContent
,SubQuestion
,ImageBlock
) provides excellent type safety and documentation of the expected data structure.
68-85
: LGTM! Clean implementation with proper error handling.The status update handler includes appropriate error handling and user feedback through toast notifications. The displayType logic is well-implemented.
app/(public)/(main-navigation)/home/page.tsx (1)
13-13
: Add type safety for grade conversion.The grade conversion to number could fail silently if grade is invalid.
Add type checking or use a type-safe conversion:
- grade: grade ? Number(grade) : undefined, + grade: grade && !isNaN(Number(grade)) ? Number(grade) : undefined,
app/(public)/page.tsx
Outdated
const { data, error } = await (await supabase).auth.getUser(); | ||
|
||
if (!error && data?.user) { | ||
redirect("/home"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for authentication failure.
The authentication check should handle errors gracefully and inform users.
Add error handling for authentication failures:
- const { data, error } = await (await supabase).auth.getUser();
+ try {
+ const { data, error } = await (await supabase).auth.getUser();
+ if (!error && data?.user) {
+ redirect("/home");
+ }
+ } catch (error) {
+ console.error("Authentication error:", error);
+ // Handle authentication error gracefully
+ }
📝 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 { data, error } = await (await supabase).auth.getUser(); | |
if (!error && data?.user) { | |
redirect("/home"); | |
} | |
try { | |
const { data, error } = await (await supabase).auth.getUser(); | |
if (!error && data?.user) { | |
redirect("/home"); | |
} | |
} catch (error) { | |
console.error("Authentication error:", error); | |
// Handle authentication error gracefully | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
components/admin/resources/add-question-dialog.tsx (1)
115-115
: Avoid using type assertionas any
.Type assertions with
any
bypass TypeScript's type checking. Consider defining a proper type for the content field.-content: questionToEdit.content as any +content: questionToEdit.content as QuestionContentAdd this type definition at the top of the file:
type QuestionContent = { mainQuestion: string; blocks: Array<{ type: 'text' | 'image'; content: string; imageData?: { url: string; caption?: string; position: 'above' | 'below' | 'inline'; }; }>; marks?: number | null; subQuestions: Array<{ part: string; text: string; type: SolutionType; marks: number | null; blocks: Array<{ type: 'text' | 'image'; content: string; imageData?: { url: string; caption?: string; position: 'above' | 'below' | 'inline'; }; }>; }>; };🧰 Tools
🪛 ESLint
[error] 115-115: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
♻️ Duplicate comments (1)
components/providers/providers.tsx (1)
9-12
: Use the created theme in MantineProvider.The
theme
variable is created but never used. This is causing a linting error.Apply this diff to use the theme:
- <MantineProvider> + <MantineProvider theme={theme}> <AuthProvider>{children}</AuthProvider> </MantineProvider>🧰 Tools
🪛 ESLint
[error] 9-9: 'theme' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🧹 Nitpick comments (6)
components/subjects/resource-list.tsx (1)
60-61
: Consider using a named constant for the A4 aspect ratio.The A4 aspect ratio calculation uses a magic number. Consider extracting this into a named constant for better maintainability and clarity.
+const A4_ASPECT_RATIO = 1.414; // ISO 216 A4 aspect ratio - const a4Height = 176 * 1.414; // width (176px) * aspect ratio + const a4Height = 176 * A4_ASPECT_RATIO; // width (176px) * aspect ratio🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Prefer constants from the standard library.
Unsafe fix: Use Math.SQRT2 instead.
(lint/suspicious/noApproximativeNumericConstant)
components/admin/resources/add-question-dialog.tsx (4)
144-148
: Simplify the content spread in questionData.The current spread with optional chaining and nested property access can be simplified.
-content: { - ...data.content, - marks: data.content?.marks - -} +content: data.content
185-185
: Consider responsive max-height for the dialog.The fixed
max-h-[90vh]
might not work well on all screen sizes. Consider using responsive classes or media queries.-<DialogContent className="max-w-4xl max-h-[90vh] overflow-y-auto"> +<DialogContent className="max-w-4xl max-h-[80vh] sm:max-h-[90vh] overflow-y-auto">
243-243
: Make placeholder text consistent.The placeholder text "Enter Page 2" is inconsistent with other input placeholders.
-placeholder="Enter Page 2" +placeholder="2"
359-359
: Consider using theme-consistent background color.The
bg-slate-100
might not align with the application's theme system.-<Card key={field.id} className="p-4 bg-slate-100"> +<Card key={field.id} className="p-4 bg-muted">app/(public)/page.tsx (1)
56-116
: Consider adding loading states for better UX.While the error handling is good, the component could benefit from loading states during data fetching to improve user experience.
export default async function Home() { const supabase = createClient(); + const [isLoading, setIsLoading] = useState(true); const userData = await handleGetUserData(supabase); if (userData?.error) { return ( <div className="max-w-[64rem] mx-auto px-4 py-12"> <Navbar /> <div className="text-center my-12"> <p className="text-red-500"> {userData.error} Please try again later. </p> </div> </div> ); } const { subjects } = await handleGetSuggestedSubjects(); const { resources } = await handleGetResources(); + setIsLoading(false); return ( <div className="max-w-[64rem] mx-auto px-4 py-12"> <Navbar /> <div className="text-center my-12"> + {isLoading ? ( + <div className="flex justify-center"> + <LoadingSpinner /> + </div> + ) : ( {/* Rest of your JSX */} + )} </div> </div> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
actions/resources.ts
(5 hunks)app/(public)/page.tsx
(3 hunks)components/admin/resources/add-question-dialog.tsx
(10 hunks)components/layout/navbar.tsx
(3 hunks)components/providers/providers.tsx
(1 hunks)components/subjects/resource-list.tsx
(2 hunks)features/exercise/components/exercise-content.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/layout/navbar.tsx
- features/exercise/components/exercise-content.tsx
- actions/resources.ts
🧰 Additional context used
🪛 Biome (1.9.4)
components/subjects/resource-list.tsx
[error] 61-61: Prefer constants from the standard library.
Unsafe fix: Use Math.SQRT2 instead.
(lint/suspicious/noApproximativeNumericConstant)
🪛 ESLint
app/(public)/page.tsx
[error] 10-10: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
components/providers/providers.tsx
[error] 9-9: 'theme' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (8)
components/providers/providers.tsx (1)
15-20
: LGTM! Good use of Readonly type.The component structure is correct, and using
Readonly
for props is a good practice to ensure immutability.components/subjects/resource-list.tsx (4)
2-27
: LGTM! Well-organized imports and clean interface definitions.The imports are properly organized, and the interfaces are well-defined with clear type definitions.
29-91
: LGTM! Well-structured component decomposition.The components are well-organized with clear separation of concerns:
- ResourceCardComponent handles the overall card layout
- ResourceCoverImage manages image display with appropriate fallbacks
- ResourceTags presents metadata in a clean format
🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Prefer constants from the standard library.
Unsafe fix: Use Math.SQRT2 instead.
(lint/suspicious/noApproximativeNumericConstant)
93-127
: LGTM! Well-implemented horizontal scrolling with proper accessibility.The SubjectResourceList component is well-structured with:
- Smooth horizontal scrolling with snap points
- Clear subject headers with emoji support
- Accessible navigation with "View All" links
129-175
: LGTM! Well-implemented tabbed interface with proper resource organization.The ResourceList component effectively:
- Groups and displays resources by type and subject
- Implements an accessible tabbed interface
- Handles empty states appropriately
components/admin/resources/add-question-dialog.tsx (1)
87-87
: LGTM! Props are now immutable.Making props readonly improves type safety by preventing accidental mutations.
app/(public)/page.tsx (2)
33-41
: LGTM! Good error handling implementation.The function properly handles errors and provides a sensible fallback value.
43-54
: LGTM! Good error handling and configuration.The function properly handles errors, provides a sensible fallback value, and uses appropriate configuration for live resources.
Summary by CodeRabbit
New Features
Loading
component to enhance user experience during page loading.HomePage
component to display resources with improved error handling.More
component for account information with placeholders for future content.BottomNavigation
component for easy access to key sections of the app.ExercisePage
with a dedicated loading state and improved modular structure.ExerciseTopBar
for better navigation within exercise content.useResources
, for efficient resource fetching.Style Updates
PricingPage
layout for better visual hierarchy and spacing.Bug Fixes
Chores