Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces and refactors several course-related React hooks and components to modularize course data fetching, lesson management, decryption, navigation, and UI rendering. It replaces inline logic in the course page with composable hooks and components, updates import paths, and standardizes the markdown editor across multiple forms with a new dark-mode-compatible editor component. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CoursePage
participant useCourseData
participant useLessons
participant useCourseDecryption
participant useCourseNavigation
participant CourseOverview
participant CourseContent
participant CourseQA
User->>CoursePage: Navigates to course page
CoursePage->>useCourseData: Fetch course data and lesson IDs
useCourseData-->>CoursePage: Returns course, lessonIds, paidCourse, loading
CoursePage->>useLessons: Fetch lessons for lessonIds
useLessons-->>CoursePage: Returns lessons, uniqueLessons
CoursePage->>useCourseDecryption: Decrypt active lesson if needed
useCourseDecryption-->>CoursePage: Returns decryption status
CoursePage->>useCourseNavigation: Manage tab/lesson/sidebar state
useCourseNavigation-->>CoursePage: Returns navigation state and handlers
CoursePage->>CourseOverview: Render overview tab (if active)
CoursePage->>CourseContent: Render content tab (if active)
CoursePage->>CourseQA: Render QA/comments tab (if active)
User->>CoursePage: Interacts with tabs/lessons
CoursePage->>useCourseNavigation: Update navigation state
Possibly related PRs
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/pages/_app.js (1)
🔇 Additional comments (1)
✨ 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 (
|
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (16)
src/config/appConfig.js (1)
14-15: Provide context for the new author public key
The raw hex string is valid (64‐char lowercase), but it’s unclear whose key this is and why it’s being added. Please add a short comment above the entry to explain its owner/purpose and verify that any authentication or test suites have been updated to include this key.src/components/content/courses/details/CourseDetails.js (1)
22-23: Inconsistent import path styles.While DesktopCourseDetails now uses an absolute path with the '@/' alias, MobileCourseDetails still uses a relative import. For consistency and maintainability, consider using the same import style for both components.
// Import the desktop and mobile components import DesktopCourseDetails from '@/components/content/courses/details/DesktopCourseDetails'; -import MobileCourseDetails from './MobileCourseDetails'; +import MobileCourseDetails from '@/components/content/courses/details/MobileCourseDetails';src/components/forms/course/embedded/EmbeddedDocumentForm.js (1)
2-17: Import organization includes unused dependenciesThe import structure has been updated, but some imported modules don't appear to be used in the shown code. Specifically,
axios,InputTextarea,Calendar, anduseRouterare imported but not utilized in this component based on the provided code.Consider removing or commenting unused imports to avoid bloat:
-import axios from 'axios'; -import { InputTextarea } from 'primereact/inputtextarea'; -import { Calendar } from 'primereact/calendar'; -import { useRouter } from 'next/router';src/components/content/courses/tabs/CourseQA.js (1)
1-32: Well-structured comments component with hardcoded relay URLsThe CourseQA component is well-organized and provides a clear UI for the comments section. However, the relay URLs are hardcoded in the component, which could make future maintenance challenging.
Consider extracting the relay URLs to a configuration file or environment variable:
- relays="wss://nos.lol/, wss://relay.damus.io/, wss://relay.snort.social/, wss://relay.nostr.band/, wss://relay.primal.net/, wss://nostrue.com/, wss://purplerelay.com/, wss://relay.devs.tools/" + relays={NOSTR_RELAY_URLS}Then define
NOSTR_RELAY_URLSin a constants file that can be imported.src/hooks/courses/useLessons.js (1)
1-61: Well-implemented lessons hook with performance optimization opportunitiesThe
useLessonshook effectively fetches and manages lesson data. It's well-documented and follows React patterns correctly.Consider these performance improvements:
- Check if NDK is already connected before connecting:
try { - await ndk.connect(); + if (!ndk.connected) { + await ndk.connect(); + }
- Fetch authors in parallel for better performance:
- // Process events (no need to check for duplicates here) - for (const event of events) { - const author = await fetchAuthor(event.pubkey); - const parsedLesson = { ...parseEvent(event), author }; - newLessons.push(parsedLesson); - } + // Process events in parallel + const lessonsPromises = Array.from(events).map(async (event) => { + const author = await fetchAuthor(event.pubkey); + return { ...parseEvent(event), author }; + }); + + const newLessons = await Promise.all(lessonsPromises);
- Consider adding error state handling rather than just logging errors:
} catch (error) { console.error('Error fetching events:', error); + // Consider adding error state here + // setError(error); }src/hooks/courses/useCoursePayment.js (1)
37-38: Simplify conditional expression with optional chainingThe condition can be simplified for better readability.
- if (response && response?.preimage) { + if (response?.preimage) {🧰 Tools
🪛 Biome (1.9.4)
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/hooks/courses/useCourseNavigation.js (1)
109-127: Update useEffect dependency array for keyboard navigationThe useEffect for keyboard navigation depends on
tabMapbut doesn't include the helper functions that use it.- }, [activeTab, tabMap]); + }, [activeTab, tabMap, toggleTab, getActiveTabIndex]);src/hooks/courses/useCourseData.js (2)
39-54: Consider optimizing NDK connection managementThe
ndk.connect()is called on every fetch, which could be inefficient if the hook re-runs frequently.Consider moving the NDK connection management to a higher level (app initialization or a dedicated hook) and ensuring it's only connected once:
const fetchCourse = async courseId => { try { - await ndk.connect(); const event = await ndk.fetchEvent({ '#d': [courseId] }); if (!event) return null;
51-52: Enhance error handling with user feedbackErrors are logged to console but not shown to users, which limits visibility of issues.
} catch (error) { console.error('Error fetching course:', error); + showToast('error', 'Error', 'Failed to load course data. Please try again.'); return null; }src/hooks/encryption/useCourseDecryption.js (1)
105-113: Avoid stale closures when updatinglessons
setLessons(updatedLessons)relies on thelessonsarray captured whendecryptCurrentLessonwas created.
If another component mutateslessonsbefore decryption finishes, those changes are lost (last‐write-wins bug).- const updatedLessons = lessons.map(lesson => + const updatedLessons = (prevLessons) => prevLessons.map(lesson => lesson.id === currentLesson.id ? { ...lesson, content: decryptedContent } : lesson ); - setLessons(updatedLessons); + setLessons(updatedLessons);Using the functional form merges the update with any concurrent state changes.
src/components/content/courses/tabs/CourseContent.js (1)
32-48: Nested transition timer is never clearedInside the effect that handles lesson switching, a second
setTimeout(50 ms) is started inside the first (300 ms) but is not stored or cleared on re-render/unmount. While the leak is tiny, it can accumulate during quick navigation and, more importantly, may still callsetIsTransitioning(false)after the component unmounts.Quick fix:
- const timer = setTimeout(() => { + const outerTimer = setTimeout(() => { ... - setTimeout(() => { + const innerTimer = setTimeout(() => { setIsTransitioning(false); }, 50); + // cleanup inner timer, too + return () => clearTimeout(innerTimer); }, 300); - return () => clearTimeout(timer); + return () => clearTimeout(outerTimer);(Or refactor to a single CSS-only transition to remove JS timers entirely.)
src/hooks/courses/useCourseTabs.js (2)
81-104: Label/key mismatch may confuse analytics & QA
tabMapuses the key"qa"whilegetTabItems()returns the label"Comments".
Ensure naming consistency (e.g. use"qa"everywhere or rename to"comments").- items.push({ - label: 'Comments', + items.push({ + label: 'QA',(or vice-versa)
15-16: Possible initial flicker on SSR / very narrow viewports
windowWidthisundefinedon the first render, yielding
undefined <= 968 // false, soisMobileViewstarts asfalse.
This can cause a one-frame flash of the desktop layout before the hook recalculates.
Consider initialising totruewhenwindowWidthisundefined:- const isMobileView = windowWidth <= 968; + const isMobileView = windowWidth === undefined ? true : windowWidth <= 968;src/pages/course/[slug]/index.js (3)
70-86: Robustness for localStorage parsing & SSR edge-casesGood job deferring the read to
useEffect(client-only). Two small hardening tips:
- Guard for cases where
window.localStorageis unavailable (private-mode Safari can throw).- Prefix
JSON.parseinside its owntry/catch(you already do) and fall back to an empty array to keep state invariant.No action strictly required, but worth considering.
88-106: Simplify duplicate-prevention logic insetCompletedCurrent code does a manual
includescheck and then spreads the array. ASetkeeps intent crystal-clear and guarantees uniqueness:- // Avoid duplicates - if (prev.includes(lessonId)) { - return prev; - } - - const newCompletedLessons = [...prev, lessonId]; + const newCompletedLessons = Array.from( + new Set([...prev, lessonId]) + );Functionality stays identical, complexity drops.
185-190: MemoiseisAuthorizedfor readability
isAuthorizedrecomputes on every render even when none of its constituents change. Wrapping it inuseMemoavoids needless work and self-documents the dependency list:- const isAuthorized = - session?.user?.role?.subscribed || - session?.user?.pubkey === course?.pubkey || - !paidCourse || - session?.user?.purchased?.some(purchase => purchase.courseId === course?.d); + const isAuthorized = React.useMemo(() => ( + session?.user?.role?.subscribed || + session?.user?.pubkey === course?.pubkey || + !paidCourse || + session?.user?.purchased?.some(p => p.courseId === course?.d) + ), [session, paidCourse, course]);A micro-optimisation, but it clarifies intent and avoids accidental dependency omissions in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/components/content/courses/details/CourseDetails.js(1 hunks)src/components/content/courses/layout/CourseSidebar.js(1 hunks)src/components/content/courses/tabs/CourseContent.js(1 hunks)src/components/content/courses/tabs/CourseOverview.js(1 hunks)src/components/content/courses/tabs/CourseQA.js(1 hunks)src/components/forms/combined/CombinedResourceForm.js(2 hunks)src/components/forms/combined/EditDraftCombinedResourceForm.js(2 hunks)src/components/forms/combined/EditPublishedCombinedResourceForm.js(3 hunks)src/components/forms/course/embedded/EmbeddedDocumentForm.js(2 hunks)src/components/forms/document/DocumentForm.js(2 hunks)src/components/forms/document/EditDraftDocumentForm.js(2 hunks)src/components/forms/document/EditPublishedDocumentForm.js(2 hunks)src/components/forms/video/EditPublishedVideoForm.js(2 hunks)src/components/markdown/MarkdownEditor.js(1 hunks)src/components/navbar/Navbar.js(1 hunks)src/config/appConfig.js(1 hunks)src/hooks/courses/index.js(1 hunks)src/hooks/courses/useCourseData.js(1 hunks)src/hooks/courses/useCourseNavigation.js(1 hunks)src/hooks/courses/useCoursePayment.js(1 hunks)src/hooks/courses/useCourseTabs.js(1 hunks)src/hooks/courses/useLessons.js(1 hunks)src/hooks/encryption/useCourseDecryption.js(1 hunks)src/pages/course/[slug]/draft/index.js(1 hunks)src/pages/course/[slug]/index.js(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/components/forms/combined/EditPublishedCombinedResourceForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/forms/course/embedded/EmbeddedDocumentForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/forms/document/DocumentForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/forms/combined/EditDraftCombinedResourceForm.js (3)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)src/components/forms/combined/CombinedResourceForm.js (2)
content(24-24)handleContentChange(40-42)src/components/forms/combined/EditPublishedCombinedResourceForm.js (2)
content(32-32)handleContentChange(68-70)
src/components/forms/combined/CombinedResourceForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/hooks/courses/useLessons.js (5)
src/pages/course/[slug]/index.js (2)
useLessons(125-130)fetchAuthor(108-116)src/components/content/courses/details/CourseDetails.js (2)
fetchAuthor(88-99)author(36-36)src/components/content/courses/details/DraftCourseDetails.js (2)
fetchAuthor(35-46)author(23-23)src/pages/course/[slug]/draft/index.js (2)
fetchAuthor(31-43)lessons(15-15)src/hooks/courses/useCourseData.js (1)
lessonIds(15-15)
src/hooks/courses/useCourseTabs.js (1)
src/hooks/useWindowWidth.js (1)
useWindowWidth(3-24)
🪛 Biome (1.9.4)
src/hooks/courses/useCoursePayment.js
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (26)
src/components/navbar/Navbar.js (1)
11-11: Import path updated to follow new directory structure.The path for CourseHeader has been correctly updated to match the new organization of course components into a 'layout' subdirectory, consistent with the refactoring effort.
src/pages/course/[slug]/draft/index.js (1)
5-6: Import paths correctly updated to absolute paths.The import paths for course components now use absolute paths with the '@/' alias, which is better for maintainability in deeply nested directories. This change aligns with the course components reorganization.
src/components/content/courses/layout/CourseSidebar.js (1)
40-42: Improved navigation with client-side transitions.Great improvement! Replacing the direct page reload with the
onLessonSelectcallback enables smoother transitions between lessons without full page refreshes. This enhances user experience and aligns with React's component-based architecture.src/components/forms/combined/EditDraftCombinedResourceForm.js (2)
8-14: Import reorganization with custom MarkdownEditor.The imports have been reorganized for better logical grouping, and the custom
MarkdownEditorcomponent has been imported to replace the direct usage of the third-party markdown editor.
243-243: Enhanced markdown editing with consistent styling.The custom
MarkdownEditorcomponent replaces the previous implementation, providing consistent dark mode styling across the application while maintaining the same functionality.src/components/forms/document/EditDraftDocumentForm.js (2)
11-14: Added required CSS imports and custom MarkdownEditor.The changes add necessary PrimeReact and PrimeIcons CSS imports that were previously missing, and replace the direct markdown editor import with the custom
MarkdownEditorcomponent.
146-146: Standardized markdown editor implementation.The custom
MarkdownEditorcomponent replaces the previous implementation, ensuring consistent styling and behavior across all forms in the application.src/hooks/courses/index.js (1)
1-13: Well-structured barrel file for course-related hooks.This new file centralizes all course-related hooks in a single export, following the best practice of using barrel exports for related functionality. This makes imports cleaner in components that use multiple hooks.
The organization of related hooks into a single entry point improves code maintainability and supports the broader refactoring effort for course functionality seen in this PR.
src/components/forms/combined/CombinedResourceForm.js (2)
8-14: Import reorganization with custom MarkdownEditor.The changes reorganize imports for better logical grouping and introduce the custom
MarkdownEditorcomponent, aligning with the pattern seen in other form components.
200-200: Standardized markdown editor implementation.The implementation now uses the custom
MarkdownEditorcomponent, ensuring consistent styling and behavior across the application while maintaining the same functionality with the original props.src/components/forms/document/DocumentForm.js (2)
12-14: Import organization improvedThe import structure has been improved by consolidating related imports. The order change for
primeiconsand addition of theMarkdownEditorcomponent follows a logical grouping pattern.
148-152: Enhanced markdown editing with custom componentThe replacement of the direct
MDEditorusage with the customMarkdownEditorcomponent improves consistency across the application and centralizes styling logic. The custom component already includes the dark mode styling that was previously applied through a wrapping div.src/components/forms/document/EditPublishedDocumentForm.js (3)
3-7: Better organization of PrimeReact UI importsThe import statements are now better organized with PrimeReact UI components grouped together, improving code readability and maintainability.
10-18: Improved import structure with necessary stylesThe imports now properly include the required CSS styles for PrimeReact components, and the custom
MarkdownEditorcomponent replaces the previous dynamic import. The hook import path has also been standardized.
201-201: Simplified markdown editor implementationReplaced the complex implementation using direct
MDEditorwith a wrapper div for dark mode with the cleanerMarkdownEditorcomponent that encapsulates these styling concerns.src/components/forms/course/embedded/EmbeddedDocumentForm.js (1)
185-185: Enhanced markdown editing with custom componentThe replacement of the direct
MDEditorusage with the customMarkdownEditorcomponent improves consistency across the application and centralizes styling logic. The component properly receives the same props as before.src/components/forms/video/EditPublishedVideoForm.js (3)
3-7: Better organization of PrimeReact UI importsThe import statements are now better organized with PrimeReact UI components grouped together, improving code readability and maintainability.
16-19: Improved import structure with necessary stylesThe imports now properly include the required CSS styles for PrimeReact components and the custom
MarkdownEditorcomponent. This ensures consistent styling across the application.
192-192: Simplified video embed input with consistent editor componentThe video embed input now uses the same
MarkdownEditorcomponent used throughout the application, ensuring a consistent editing experience across different content types. This is a good example of component reuse.src/components/forms/combined/EditPublishedCombinedResourceForm.js (2)
3-7: Import organization improvedThe imports have been nicely reorganized to group related PrimeReact components together and include necessary CSS files. This makes the dependencies clearer and more maintainable.
Also applies to: 10-10, 16-19
222-222: Standardized markdown editor implementationReplaced the direct markdown editor implementation with the new
MarkdownEditorcomponent that provides consistent styling and behavior across the application. This improves maintainability and ensures a consistent user experience.Also applies to: 239-239
src/components/content/courses/tabs/CourseOverview.js (1)
1-58: Well-structured course overview componentThe CourseOverview component is well-organized with clean conditional rendering based on device view and good separation of concerns. The component properly handles both mobile and desktop views with appropriate styling adjustments.
src/hooks/courses/useCoursePayment.js (1)
1-67: LGTM! Well-structured hook implementationThis hook effectively manages course payment processing and authorization with proper memoization and separation of concerns. The implementation includes good error handling, proper use of React hooks, and comprehensive authorization checks.
🧰 Tools
🪛 Biome (1.9.4)
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/markdown/MarkdownEditor.js (1)
1-128: LGTM! Excellent modular component implementationThis MarkdownEditor component is well-implemented with comprehensive dark mode styling, proper prop handling, and effective dynamic imports. The JSDoc documentation is thorough, and the code correctly handles SSR considerations.
src/hooks/courses/useCourseNavigation.js (1)
1-145: LGTM! Well-structured navigation hookThis hook effectively centralizes course navigation logic with clean state management and responsive behavior. Good use of
useMemofor the tab map and proper history manipulation for client-side navigation.src/hooks/courses/useCourseData.js (1)
1-80: LGTM! Well-implemented data fetching hookThis hook effectively handles course data fetching with proper state management, error handling, and flexible identifier parsing. The code is clean, well-structured, and clearly documented.
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
src/pages/course/[slug]/index.js (3)
58-66:⚠️ Potential issueHandle
slugbeing an array to avoid runtime errors
router.query.slugcan be eitherstringorstring[].
If the user navigates directly to/course/foo/bar, Next JS will supply an array.
Calling.includesor passing the value straight intosetNAddresswill then throw or produce unexpected behaviour.- const { slug } = router.query; - if (slug.includes('naddr')) { - setNAddress(slug); + // Normalise slug to a single string + const slugParam = Array.isArray(router.query.slug) + ? router.query.slug[0] + : router.query.slug; + + if (slugParam?.includes('naddr')) { + setNAddress(slugParam); } else {
185-190:⚠️ Potential issueOptional-chain the method call to prevent “undefined is not a function”
If
session.user.purchasedisundefined(guest users, new accounts, etc.),
session?.user?.purchased?.some(...)still attempts to invoke.someon
undefined, causing a hard crash.- session?.user?.purchased?.some(purchase => purchase.courseId === course?.d); + session?.user?.purchased?.some?.( + purchase => purchase.courseId === course?.d + );(Adding
?.before the call safely short-circuits tofalse.)
315-318: 🛠️ Refactor suggestionIncorrect tab name prevents sidebar from showing on mobile
activeTab === 'lessons'never evaluates totruebecause the defined tab keys
are'overview' | 'content' | 'qa'.
As a result, the sidebar is always collapsed on mobile.- (isMobileView && activeTab === 'lessons') ? 'ml-0 w-auto opacity-100' : + (isMobileView && activeTab === 'content') ? 'ml-0 w-auto opacity-100' :
♻️ Duplicate comments (1)
src/hooks/encryption/useCourseDecryption.js (1)
24-32: Samecourse.didentifier issue as in payment hookAuthorization relies on
purchase.courseId === course?.d– verify the key name or reuse a sharedgetCourseId()helper to avoid drift between hooks.
🧹 Nitpick comments (8)
src/components/content/courses/layout/CourseSidebar.js (1)
40-42: Improved lesson navigation with smooth transitions.The implementation now uses the
onLessonSelectcallback instead of forcing a full page reload. This provides a better user experience with smoother transitions between lessons.Consider adding a comment describing what the
onLessonSelectfunction is expected to do for better documentation, though the existing comment about smooth navigation is helpful.src/components/content/courses/tabs/CourseQA.js (1)
13-19: Consider extracting relay URLs to a configuration constant.The hardcoded relay URLs string could be extracted to a configuration constant or file for better maintainability.
+// At the top of the file or in a separate config file +const RELAY_URLS = "wss://nos.lol/, wss://relay.damus.io/, wss://relay.snort.social/, wss://relay.nostr.band/, wss://relay.primal.net/, wss://nostrue.com/, wss://purplerelay.com/, wss://relay.devs.tools/"; // Then in the component - relays="wss://nos.lol/, wss://relay.damus.io/, wss://relay.snort.social/, wss://relay.nostr.band/, wss://relay.primal.net/, wss://nostrue.com/, wss://purplerelay.com/, wss://relay.devs.tools/" + relays={RELAY_URLS}src/components/markdown/MarkdownEditor.js (2)
24-36: Dynamic theme override is fragileOverwriting
mod.default.Markdownat import-time couples us tightly to the library’s internal API; a minor library upgrade could rename/move that field and silently break theming.Consider:
- Leaving the module untouched and applying colours only via CSS variables / global styles, or
- Contributing a formal “theme” option upstream and using the public API when it lands.
Both reduce upgrade risk and avoid mutating module state at runtime.
73-123: Global<style>per instance → unnecessary reflow & duplication
<style jsx global>is injected every timeMarkdownEditoris mounted, repeating ~50 CSS rules. On pages with multiple editors this:
- inflates the DOM,
- forces additional style recalculations,
- complicates debugging (duplicate selectors).
Move the CSS to a single global stylesheet (e.g.
styles/md-editor-dark.css) and import it once in_app.js:- <style jsx global>{`/* …lots of rules… */`}</style> + {/* global dark-mode styles are imported in _app.js */}If you need CSS-in-JS, wrap the component with
oncesemantics (e.g. a module-leveluseEffectwith a flag) so the styles are injected only once.src/hooks/encryption/useCourseDecryption.js (1)
4-4:routerparameter is unused
routeris accepted but never referenced, triggering an ESLintno-unused-varswarning and confusing future readers.-const useCourseDecryption = (session, paidCourse, course, lessons, setLessons, router, activeIndex = 0) => { +const useCourseDecryption = ( + session, + paidCourse, + course, + lessons, + setLessons, + activeIndex = 0 +) => {Drop the param (and update call-sites) or leverage
routerif you still need to infer the active lesson from the URL.src/components/content/courses/tabs/CourseOverview.js (1)
1-58: Clean implementation of the CourseOverview component.This component effectively separates the overview tab content from the main course page, improving code organization. The conditionally rendered elements based on mobile view provide good responsive design.
Some suggestions for further improvement:
- Consider adding PropTypes validation for stronger type checking
- The logic for determining if a course is completed might need refinement if a course can be considered complete only when all lessons are completed, not just when any lessons are completed
+import PropTypes from 'prop-types'; import React from 'react'; import { Tag } from 'primereact/tag'; import CourseDetails from '../details/CourseDetails'; /** * Component to display course overview with details */ const CourseOverview = ({ course, paidCourse, lessons, decryptionPerformed, handlePaymentSuccess, handlePaymentError, isMobileView, completedLessons }) => { // Determine if course is completed const isCompleted = completedLessons.length > 0; return ( <div className={`bg-gray-800 rounded-lg border border-gray-800 shadow-md ${isMobileView ? 'p-4' : 'p-6'}`}> {isMobileView && course && ( <div className="mb-2"> {/* Completed tag above image in mobile view */} {isCompleted && ( <div className="mb-2"> <Tag severity="success" value="Completed" /> </div> )} {/* Course image */} {course.image && ( <div className="w-full h-48 relative rounded-lg overflow-hidden mb-3"> <img src={course.image} alt={course.title} className="w-full h-full object-cover" /> </div> )} </div> )} <CourseDetails processedEvent={course} paidCourse={paidCourse} lessons={lessons} decryptionPerformed={decryptionPerformed} handlePaymentSuccess={handlePaymentSuccess} handlePaymentError={handlePaymentError} isMobileView={isMobileView} showCompletedTag={!isMobileView} /> </div> ); }; + +CourseOverview.propTypes = { + course: PropTypes.object, + paidCourse: PropTypes.bool, + lessons: PropTypes.array, + decryptionPerformed: PropTypes.bool, + handlePaymentSuccess: PropTypes.func, + handlePaymentError: PropTypes.func, + isMobileView: PropTypes.bool, + completedLessons: PropTypes.array +}; + +CourseOverview.defaultProps = { + completedLessons: [] +}; export default CourseOverview;src/pages/course/[slug]/index.js (2)
70-86: Defensive access tolocalStoragefor private-window / quota-exceeded cases
localStorage.getItemcan throw (e.g. Safari private mode or when quota is
exceeded). Wrap the call intry/catchto keep the page from blanking out for
users in those environments.
220-231: Minor: MemoisegetTabItems()andgetActiveTabIndex()Both helpers are recomputed on every render, even though they only depend on
navigation state. Wrapping them inuseMemo(inside the custom hook or here)
will avoid unnecessary re-renders ofMenuTab.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/components/content/courses/details/CourseDetails.js(1 hunks)src/components/content/courses/layout/CourseSidebar.js(1 hunks)src/components/content/courses/tabs/CourseContent.js(1 hunks)src/components/content/courses/tabs/CourseOverview.js(1 hunks)src/components/content/courses/tabs/CourseQA.js(1 hunks)src/components/forms/combined/CombinedResourceForm.js(2 hunks)src/components/forms/combined/EditDraftCombinedResourceForm.js(2 hunks)src/components/forms/combined/EditPublishedCombinedResourceForm.js(3 hunks)src/components/forms/course/embedded/EmbeddedDocumentForm.js(2 hunks)src/components/forms/document/DocumentForm.js(2 hunks)src/components/forms/document/EditDraftDocumentForm.js(2 hunks)src/components/forms/document/EditPublishedDocumentForm.js(2 hunks)src/components/forms/video/EditPublishedVideoForm.js(2 hunks)src/components/markdown/MarkdownEditor.js(1 hunks)src/components/navbar/Navbar.js(1 hunks)src/config/appConfig.js(1 hunks)src/hooks/courses/index.js(1 hunks)src/hooks/courses/useCourseData.js(1 hunks)src/hooks/courses/useCourseNavigation.js(1 hunks)src/hooks/courses/useCoursePayment.js(1 hunks)src/hooks/courses/useCourseTabs.js(1 hunks)src/hooks/courses/useLessons.js(1 hunks)src/hooks/encryption/useCourseDecryption.js(1 hunks)src/pages/course/[slug]/draft/index.js(1 hunks)src/pages/course/[slug]/index.js(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/components/forms/document/EditDraftDocumentForm.js (4)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)src/components/forms/course/embedded/EmbeddedDocumentForm.js (2)
content(26-26)handleContentChange(40-42)src/components/forms/document/DocumentForm.js (2)
content(23-23)handleContentChange(37-39)src/components/forms/document/EditPublishedDocumentForm.js (2)
content(31-31)handleContentChange(42-44)
src/components/forms/combined/EditDraftCombinedResourceForm.js (2)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)src/components/forms/combined/CombinedResourceForm.js (2)
content(24-24)handleContentChange(40-42)
src/components/forms/combined/CombinedResourceForm.js (7)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)src/components/forms/combined/EditDraftCombinedResourceForm.js (2)
content(23-23)handleContentChange(79-81)src/components/forms/course/embedded/EmbeddedDocumentForm.js (2)
content(26-26)handleContentChange(40-42)src/components/forms/document/DocumentForm.js (2)
content(23-23)handleContentChange(37-39)src/components/forms/combined/EditPublishedCombinedResourceForm.js (2)
content(32-32)handleContentChange(68-70)src/components/forms/document/EditPublishedDocumentForm.js (2)
content(31-31)handleContentChange(42-44)src/components/forms/document/EditDraftDocumentForm.js (2)
content(23-23)handleContentChange(37-39)
src/components/forms/document/DocumentForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/forms/document/EditPublishedDocumentForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/hooks/courses/useCoursePayment.js (2)
src/components/profile/UserAccountLinking.js (1)
useSession(24-24)src/hooks/useToast.js (1)
showToast(11-13)
src/hooks/courses/useCourseNavigation.js (10)
src/pages/course/[slug]/index.js (3)
useCourseNavigation(43-53)router(28-28)isMobileView(39-39)src/components/content/courses/details/CourseDetails.js (1)
router(38-38)src/components/content/courses/layout/CourseHeader.js (1)
router(12-12)src/components/content/courses/details/DraftCourseLesson.js (1)
router(14-14)src/components/content/courses/details/DraftCourseDetails.js (1)
router(32-32)src/components/navbar/Navbar.js (1)
router(17-17)src/components/content/courses/lessons/CourseLesson.js (1)
isMobileView(24-24)src/components/content/courses/lessons/DocumentLesson.js (1)
isMobileView(24-24)src/components/content/courses/lessons/CombinedLesson.js (1)
isMobileView(29-29)src/components/content/courses/lessons/VideoLesson.js (1)
isMobileView(24-24)
src/hooks/courses/useCourseTabs.js (1)
src/hooks/useWindowWidth.js (1)
useWindowWidth(3-24)
🪛 Biome (1.9.4)
src/hooks/courses/useCoursePayment.js
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (40)
src/components/navbar/Navbar.js (1)
11-11: Import path updated to reflect component reorganization.The import path for CourseHeader has been updated to use the new directory structure where course components are organized in a more logical way with layout components in their own subdirectory.
src/pages/course/[slug]/draft/index.js (1)
5-6: Import paths updated to match new directory structure.The imports for DraftCourseDetails and DraftCourseLesson now correctly reference the new
/details/subdirectory, which aligns with the broader refactoring of course-related components throughout the codebase.src/components/content/courses/details/CourseDetails.js (1)
22-22: Changed relative import to absolute path.The import has been updated from a relative path to an absolute path using the
@/alias. This makes the import more consistent with the codebase style and more maintainable if component files get moved.src/config/appConfig.js (1)
14-15: Added new author public key to authorized users.A new pubkey has been added to the
authorPubkeysarray, expanding the list of authorized contributors. This change supports the course authorization and access control mechanisms used throughout the application.src/components/forms/combined/EditDraftCombinedResourceForm.js (2)
8-14: Improved import organization and standardized markdown editor usage.The imports have been reorganized in a more logical order, grouping related PrimeReact imports together and adding the new MarkdownEditor component.
243-243: Clean replacement of direct MDEditor with centralized MarkdownEditor component.The custom MarkdownEditor component now properly encapsulates the dark mode styling and configuration, reducing duplication across forms and standardizing the markdown editing experience.
src/hooks/courses/index.js (1)
1-13: Great use of barrel file pattern for course-related hooks.This new index file efficiently consolidates all course-related hooks into a single export point, making imports cleaner and more maintainable throughout the application. This pattern improves code organization and makes it easier to identify and use available hooks.
src/components/forms/video/EditPublishedVideoForm.js (2)
3-19: Improved import organization with better PrimeReact styling support.The imports have been reorganized for better grouping of related components, added required PrimeReact CSS imports, and introduced the standardized MarkdownEditor component.
192-192: Standardized markdown editing with consistent dark mode styling.Replaced the direct MDEditor usage with the centralized MarkdownEditor component, ensuring consistent styling and behavior across all forms.
src/components/forms/document/EditDraftDocumentForm.js (2)
11-14: Added required CSS imports and standardized markdown editor component.Added necessary PrimeReact CSS imports and introduced the centralized MarkdownEditor component, aligning with the application-wide standardization effort.
146-146: Simplified markdown editing with consistent styling.Replaced the direct MDEditor usage with the centralized MarkdownEditor component, ensuring consistent styling and behavior while reducing code duplication.
src/components/forms/document/DocumentForm.js (2)
12-14: Good refactoring: Standardized markdown editor implementation.The change from a dynamic import of MDEditor to a static import of the new MarkdownEditor component improves code consistency and maintainability. This aligns with similar changes across other form components in the codebase.
148-152: Clean implementation of the new MarkdownEditor component.The replacement of the inline MDEditor with the MarkdownEditor component maintains the same functionality while leveraging the consistent dark mode styling that's now centralized in the MarkdownEditor component.
src/components/forms/combined/CombinedResourceForm.js (2)
8-14: Import organization improved for better readability.The imports have been reorganized in a logical manner, with related components grouped together and the new MarkdownEditor component imported from the local path.
200-200: Simplified editor implementation with centralized styling.The replacement of the wrapped MDEditor with the new MarkdownEditor component reduces markup complexity while maintaining the same functionality, leveraging the centralized dark mode styling.
src/components/forms/course/embedded/EmbeddedDocumentForm.js (2)
16-17: Improved import organization for encryption and markdown editing.The addition of the useEncryptContent hook import makes the component's dependencies clearer, and the MarkdownEditor import aligns with the standardization across the codebase.
185-185: Standardized markdown editor implementation.Replacing the MDEditor with MarkdownEditor maintains functionality while benefiting from the centralized styling and configuration, consistent with changes in other form components.
src/components/forms/combined/EditPublishedCombinedResourceForm.js (3)
3-19: Well-organized imports improve code readability.The import statements have been reorganized in a logical manner, grouping related imports together and adding the new MarkdownEditor component. This maintains consistency with other form components.
222-222: Standardized markdown editor for video embed section.Replacing the MDEditor with MarkdownEditor for the video embed section maintains functionality while leveraging the centralized styling system, creating a consistent editor experience.
239-239: Standardized markdown editor for content section.Similarly, the content section now uses the MarkdownEditor component, ensuring consistent styling and behavior across all markdown editing areas of the application.
src/components/forms/document/EditPublishedDocumentForm.js (5)
3-7: Good addition of PrimeReact component imports.The imports are well-organized and correctly structured, providing the necessary UI components for the form.
10-10: Good implementation of the useToast hook.The useToast hook is properly imported and used in the component for displaying notifications to users (line 23).
14-14: Proper encryption hook implementation.The useEncryptContent hook is correctly imported and utilized for encrypting content when it's a paid resource (line 89).
15-18: Good standardization of CSS imports and MarkdownEditor component.The CSS imports and MarkdownEditor component import are correctly arranged to ensure proper styling and functionality.
201-201: Successful implementation of the custom MarkdownEditor component.The code effectively replaces the previous markdown editor implementation with the new centralized MarkdownEditor component. This refactoring improves consistency across the application by standardizing the markdown editing experience with proper dark mode styling.
src/hooks/courses/useLessons.js (5)
1-11: Well-documented hook with proper imports and JSDoc.The hook has clear documentation explaining its purpose, parameters, and return values, making it easy for other developers to understand and use.
12-15: Good state management setup.The hook correctly initializes state for both the raw lessons array and the deduplicated uniqueLessons array.
17-48: Efficient lesson fetching implementation.The lesson fetching logic is well-implemented with:
- Proper dependency array
- Conditional execution based on lessonIds and pubkey availability
- Single filter creation to minimize API calls
- Async/await pattern with try/catch for error handling
- Author data enrichment for each lesson
This approach is more efficient than fetching lessons individually.
50-56: Good deduplication strategy.The deduplication logic using Map is an efficient way to ensure unique lessons based on their IDs. This helps prevent duplicate lessons from being displayed in the UI.
58-61: Clean and concise return value.The hook returns exactly what consuming components need: the full lessons array, deduplicated lessons, and a setter for updating lessons if needed.
src/components/content/courses/tabs/CourseQA.js (2)
1-7: Well-documented component with proper imports.The component has clear documentation and imports only what's necessary.
7-30: Good conditional rendering based on authorization.The component correctly displays different content based on user authorization status, providing appropriate feedback to unauthorized users.
src/hooks/courses/useCourseData.js (6)
1-12: Well-structured imports and documentation.The hook has comprehensive JSDoc documentation explaining its purpose, parameters, and return values.
13-19: Good state management setup.The hook properly initializes all necessary state variables, including course data, lesson IDs, paid status, and loading state.
20-38: Robust slug and course ID handling.The implementation correctly handles both plain slugs and NIP-19 encoded addresses, with proper error handling for invalid formats.
39-54: Well-implemented course fetching logic.The fetchCourse function properly:
- Connects to NDK
- Fetches the course event
- Retrieves author data
- Extracts lesson IDs from tags
- Parses and combines course and author data
- Handles errors appropriately
56-75: Good orchestration of initialization flow.The initializeCourse function correctly manages the loading state and orchestrates the fetching of course ID and course data, with proper error handling at each step.
77-80: Clean hook API.The hook returns a concise object with all the necessary data for consuming components.
src/components/content/courses/tabs/CourseContent.js (1)
1-122:Details
✅ Verification successful
Well-structured component with smooth transitions, but some improvements needed.
This component effectively handles the content tab of the course interface, with good transitions between lessons. The conditional rendering based on lesson type is well-implemented.
Some issues to address:
- Nested setTimeout in transition effect could be simplified
- Missing PropTypes validation
- The default of
nullforcurrentLessoncould lead to unnecessary re-rendersimport React, { useState, useEffect } from 'react'; +import PropTypes from 'prop-types'; import VideoLesson from '@/components/content/courses/lessons/VideoLesson'; import DocumentLesson from '@/components/content/courses/lessons/DocumentLesson'; import CombinedLesson from '@/components/content/courses/lessons/CombinedLesson'; import MarkdownDisplay from '@/components/markdown/MarkdownDisplay'; /** * Component to display course content including lessons */ const CourseContent = ({ lessons, activeIndex, course, paidCourse, decryptedLessonIds, setCompleted }) => { const [lastActiveIndex, setLastActiveIndex] = useState(activeIndex); const [isTransitioning, setIsTransitioning] = useState(false); - const [currentLesson, setCurrentLesson] = useState(null); + const [currentLesson, setCurrentLesson] = useState(() => { + return lessons.length > 0 && activeIndex < lessons.length ? lessons[activeIndex] : null; + }); // Initialize current lesson and handle updates when lessons or activeIndex change useEffect(() => { if (lessons.length > 0 && activeIndex < lessons.length) { setCurrentLesson(lessons[activeIndex]); } else { setCurrentLesson(null); } }, [lessons, activeIndex]); // Handle smooth transitions between lessons useEffect(() => { if (activeIndex !== lastActiveIndex) { // Start transition setIsTransitioning(true); - // After a short delay, update the current lesson - const timer = setTimeout(() => { - setCurrentLesson(lessons[activeIndex] || null); - setLastActiveIndex(activeIndex); - - // End transition with a slight delay to ensure content is ready - setTimeout(() => { - setIsTransitioning(false); - }, 50); - }, 300); // Match this with CSS transition duration + // Set a single timer that completes the entire transition + const transitionDuration = 300; // Match this with CSS transition duration + const timer = setTimeout(() => { + setCurrentLesson(lessons[activeIndex] || null); + setLastActiveIndex(activeIndex); + setIsTransitioning(false); + }, transitionDuration); return () => clearTimeout(timer); } }, [activeIndex, lastActiveIndex, lessons]); const renderLesson = (lesson) => { if (!lesson) return null; // Check if this specific lesson is decrypted const lessonDecrypted = !paidCourse || decryptedLessonIds[lesson.id] || false; if (lesson.topics?.includes('video') && lesson.topics?.includes('document')) { return ( <CombinedLesson key={`combined-${lesson.id}`} lesson={lesson} course={course} decryptionPerformed={lessonDecrypted} isPaid={paidCourse} setCompleted={setCompleted} /> ); } else if (lesson.type === 'video' || lesson.topics?.includes('video')) { return ( <VideoLesson key={`video-${lesson.id}`} lesson={lesson} course={course} decryptionPerformed={lessonDecrypted} isPaid={paidCourse} setCompleted={setCompleted} /> ); } else if (lesson.type === 'document' || lesson.topics?.includes('document')) { return ( <DocumentLesson key={`doc-${lesson.id}`} lesson={lesson} course={course} decryptionPerformed={lessonDecrypted} isPaid={paidCourse} setCompleted={setCompleted} /> ); } return null; }; return ( <> {lessons.length > 0 && currentLesson ? ( <div className="bg-gray-800 rounded-lg shadow-sm overflow-hidden"> <div key={`lesson-container-${currentLesson.id}`} className={`transition-opacity duration-300 ease-in-out ${isTransitioning ? 'opacity-0' : 'opacity-100'}`} > {renderLesson(currentLesson)} </div> </div> ) : ( <div className={`text-center bg-gray-800 rounded-lg p-8 transition-opacity duration-300 ease-in-out ${isTransitioning ? 'opacity-0' : 'opacity-100'}`}> <p>Select a lesson from the sidebar to begin learning.</p> </div> )} {course?.content && ( <div className="mt-8 bg-gray-800 rounded-lg shadow-sm"> <MarkdownDisplay content={course.content} className="p-4 rounded-lg" /> </div> )} </> ); }; + +CourseContent.propTypes = { + lessons: PropTypes.array.isRequired, + activeIndex: PropTypes.number.isRequired, + course: PropTypes.object.isRequired, + paidCourse: PropTypes.bool, + decryptedLessonIds: PropTypes.object, + setCompleted: PropTypes.func +}; + +CourseContent.defaultProps = { + paidCourse: false, + decryptedLessonIds: {}, + setCompleted: () => {} +}; export default CourseContent;Let's check if the transition timing matches CSS classes:
🏁 Script executed:
#!/bin/bash # Check for CSS transitions in the styling files rg "transition-opacity" --include="*.css" --include="*.scss" src/ rg "duration-300" --include="*.css" --include="*.scss" src/Length of output: 1009
All improvements verified – no further changes needed
The transition timing of 300 ms aligns with the Tailwind CSS utility class
duration-300, so the simplified singlesetTimeoutcovers both the opacity transition and lesson update. PropTypes and defaultProps have been correctly added, and initializingcurrentLessonvia a lazy state initializer prevents extra renders.No outstanding issues remain.
src/pages/course/[slug]/index.js (1)
142-154: GuardactiveIndexto avoid out-of-bounds look-upsWhen
uniqueLessonsupdates (e.g. after decryption finishes) its length can
momentarily be smaller than the previousactiveIndex.
uniqueLessons[activeIndex]will then beundefined, causing an attempt to
read.idonundefinedtwo lines later.Add an early return:
- if (paidCourse && uniqueLessons.length > 0) { - const currentLesson = uniqueLessons[activeIndex]; + if ( + paidCourse && + uniqueLessons.length > 0 && + activeIndex < uniqueLessons.length + ) { + const currentLesson = uniqueLessons[activeIndex];
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/components/forms/course/embedded/EmbeddedDocumentForm.js (1)
170-186:⚠️ Potential issueMissing Tooltip component for info icon.
There's an info icon with tooltip attributes (lines 190-197), but the
Tooltipcomponent that was likely removed during refactoring isn't being rendered. This may result in the tooltip not working properly.Add the Tooltip component at the end of the form to ensure tooltips work correctly:
<div className="p-inputgroup flex-1 flex-col mt-4"> <span>Content</span> <MarkdownEditor value={content} onChange={handleContentChange} height={350} /> </div> + <Tooltip target=".pi-info-circle" />
♻️ Duplicate comments (4)
src/components/content/courses/tabs/CourseOverview.js (1)
19-19: Revisit course completion logicThe current implementation considers a course completed if there's at least one completed lesson, which might not accurately reflect full course completion.
Consider implementing a more robust completion check:
- const isCompleted = completedLessons.length > 0; + const isCompleted = lessons && lessons.length > 0 && completedLessons.length === lessons.length;This ensures a course is only marked as completed when all lessons have been completed.
src/hooks/encryption/useCourseDecryption.js (2)
24-32:⚠️ Potential issue
course?.dtypo still blocks access control
The property should becourse?.id; otherwisehasAccessis alwaysfalse, preventing paying users from seeing their content.- session.user.purchased?.some(purchase => purchase.courseId === course?.d) || + session.user.purchased?.some(purchase => purchase.courseId === course?.id) ||
70-98: 🛠️ Refactor suggestionTimers are not cleared → memory leak & setState-after-unmount risk
Both the 10-second
timeoutIdand the 5-second retry timer live beyond the component’s lifecycle.
If the user leaves the page early they will still fire, potentially callingsetLoadingorsetLessonson an unmounted component.+ const decryptTimeoutRef = useRef(null); + const retryTimeoutRef = useRef(null); ... - const timeoutPromise = new Promise((_, reject) => { - timeoutId = setTimeout(() => { + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { ... - }, 10000); + }, 10000); }); + decryptTimeoutRef.current = timeoutId; ... - setTimeout(() => { + retryTimeoutRef.current = setTimeout(() => { processingRef.current = false; decryptCurrentLesson(); - }, 5000); + }, 5000); ... - }, [currentLesson, hasAccess, paidCourse, decryptContent, lessons, setLessons, decryptedLessonIds]); + }, [currentLesson, hasAccess, paidCourse, decryptContent, lessons, setLessons, decryptedLessonIds]); + + // Cleanup all pending timers on unmount + useEffect(() => () => { + clearTimeout(decryptTimeoutRef.current); + clearTimeout(retryTimeoutRef.current); + }, []);src/pages/course/[slug]/index.js (1)
37-153:isDecryptingis derived state – duplicate of earlier feedback
Storing it inuseState+useEffectadds an unnecessary render cycle and risk of stale state. You can compute the boolean withuseMemodirectly from dependencies.(See previous review on older commit; recommendation still applies.)
🧹 Nitpick comments (17)
src/components/markdown/MarkdownEditor.js (1)
73-123: Consider extracting the CSS to a separate stylesheetWhile the global styles effectively ensure consistent dark mode rendering, consider extracting these styles to a separate CSS file to improve maintainability and reduce the JSX complexity. The extensive use of
!importantflags, while necessary here, would be better isolated in a dedicated stylesheet.- <style jsx global>{` - /* Force all text to white in editor */ - .w-md-editor * { - color: white !important; - } - - /* Reset preview text color */ - .w-md-editor-preview * { - color: #c9d1d9 !important; - } - - /* Editor backgrounds */ - .md-editor-dark { - background-color: #0d1117 !important; - } - - .w-md-editor-text-input { - caret-color: white !important; - -webkit-text-fill-color: white !important; - color: white !important; - } - - .w-md-editor-toolbar { - background-color: #161b22 !important; - border-bottom: 1px solid #30363d !important; - } - - /* Preview styling */ - .w-md-editor-preview { - background-color: #0d1117 !important; - font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif; - } - - /* Make code blocks maintain their styling */ - .w-md-editor-preview pre { - background-color: #1e1e1e !important; - color: #d4d4d4 !important; - padding: 1em !important; - border-radius: 5px !important; - } - - .w-md-editor-preview code { - font-family: 'Consolas', 'Monaco', 'Andale Mono', 'Ubuntu Mono', monospace !important; - color: #d4d4d4 !important; - } - - /* Force anything with text-rendering to be white */ - [style*="text-rendering"] { - color: white !important; - } - `}</style>Then import a stylesheet like:
// At the top of the file import '@/styles/markdown-editor-dark.css';src/components/forms/video/EditPublishedVideoForm.js (1)
44-46: Remove debugging console.log statementThere's an unused console.log statement that should be removed from the production code.
- useEffect(() => { - console.log('event', event); - }, [event]);src/components/content/courses/details/CourseDetails.js (1)
22-22: Consider consistent import styleThe import for
DesktopCourseDetailshas been updated to use an absolute path with the@/alias, which is good for maintainability. However, the import forMobileCourseDetailson line 23 still uses a relative path. For consistency, consider updating all imports to use the same style.-import MobileCourseDetails from './MobileCourseDetails'; +import MobileCourseDetails from '@/components/content/courses/details/MobileCourseDetails';src/components/forms/course/embedded/EmbeddedDocumentForm.js (1)
2-17: Some imports may be unused.Several new imports have been added (
axios,InputTextarea,Calendar,useRouter), but they don't appear to be used in the visible code. Consider cleaning up any unused imports to prevent code bloat.-import axios from 'axios'; -import { InputTextarea } from 'primereact/inputtextarea'; -import { Calendar } from 'primereact/calendar'; -import { useRouter } from 'next/router';src/components/content/courses/tabs/CourseQA.js (1)
16-16: Consider extracting relay list to a configuration file.The hardcoded list of relay URLs could be moved to a central configuration file, making it easier to maintain and update across the application.
// In a config file like src/config/nostr.js export const DEFAULT_RELAYS = "wss://nos.lol/, wss://relay.damus.io/, wss://relay.snort.social/, wss://relay.nostr.band/, wss://relay.primal.net/, wss://nostrue.com/, wss://purplerelay.com/, wss://relay.devs.tools/"; // Then in this component: import { DEFAULT_RELAYS } from '@/config/nostr'; // And use it like: relays={DEFAULT_RELAYS}src/hooks/courses/useLessons.js (1)
1-61: Well-structured hook with efficient implementationThis new hook effectively modularizes lesson fetching and management with a clean separation of concerns. The single filter approach to fetch all lessons in one network call is an optimization worth highlighting.
I particularly like:
- The comprehensive error handling in the fetch operation
- Using a Map for efficient lesson deduplication
- Clear JSDoc documentation
Consider adding an
isLoadingstate to indicate when lessons are being fetched:const useLessons = (ndk, fetchAuthor, lessonIds, pubkey) => { const [lessons, setLessons] = useState([]); const [uniqueLessons, setUniqueLessons] = useState([]); + const [isLoading, setIsLoading] = useState(false); // Fetch lessons when IDs or pubkey change useEffect(() => { if (lessonIds.length > 0 && pubkey) { const fetchLessons = async () => { + setIsLoading(true); try { await ndk.connect(); // Rest of the function... setLessons(newLessons); + setIsLoading(false); } catch (error) { console.error('Error fetching events:', error); + setIsLoading(false); } }; fetchLessons(); } }, [lessonIds, ndk, fetchAuthor, pubkey]); // Rest of the hook... - return { lessons, uniqueLessons, setLessons }; + return { lessons, uniqueLessons, setLessons, isLoading }; };This would allow components consuming this hook to show loading states to users.
src/hooks/courses/useCoursePayment.js (1)
37-37: Simplify conditional check with optional chainingThere's a redundant condition check in this line.
- if (response && response?.preimage) { + if (response?.preimage) {The optional chaining operator
?.already handles the case whereresponseis null or undefined, making theresponse &&check unnecessary.🧰 Tools
🪛 Biome (1.9.4)
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/hooks/courses/useCourseNavigation.js (1)
14-23: Consider extracting the tabMap logic for testabilityThis memoized function is well-implemented, but could be extracted for better testability.
+ // Helper function to get tabMap based on view mode + const getTabMap = (isMobileView) => { + const baseTabMap = ['overview', 'content', 'qa']; + if (isMobileView) { + const mobileTabMap = [...baseTabMap]; + mobileTabMap.splice(2, 0, 'lessons'); + return mobileTabMap; + } + return baseTabMap; + }; const useCourseNavigation = (router, isMobileView) => { // ...existing state declarations // Memoized function to get the tab map based on view mode const tabMap = useMemo(() => { - const baseTabMap = ['overview', 'content', 'qa']; - if (isMobileView) { - const mobileTabMap = [...baseTabMap]; - mobileTabMap.splice(2, 0, 'lessons'); - return mobileTabMap; - } - return baseTabMap; + return getTabMap(isMobileView); }, [isMobileView]); // ...rest of the hook } // Export the helper for testing export { getTabMap }; export default useCourseNavigation;This makes the function independently testable and clarifies its responsibility.
src/hooks/courses/useCourseData.js (1)
67-70: Guarantee a boolean inpaidCourse
parsedCourse.price && parsedCourse.price > 0yieldsundefinedwhenpriceis0.
Consumers expecting a boolean may mis-behave.- setPaidCourse(parsedCourse.price && parsedCourse.price > 0); + setPaidCourse(Boolean(parsedCourse?.price) && Number(parsedCourse.price) > 0);src/hooks/encryption/useCourseDecryption.js (1)
55-60: Retry counter increments before passing the thresholdCurrently the retry count is increased before the
>= MAX_RETRIEScheck, which effectively allows onlyMAX_RETRIES – 1real attempts.- // Limit maximum retries - if (retryCountRef.current[currentLesson.id] >= MAX_RETRIES) { - return; - } - - // Increment retry count - retryCountRef.current[currentLesson.id]++; + // Limit maximum retries + if (retryCountRef.current[currentLesson.id] >= MAX_RETRIES) { + return; + } + + // Increment retry count *after* the guard + retryCountRef.current[currentLesson.id] += 1;src/components/content/courses/tabs/CourseContent.js (2)
37-49: Second timeout not cleared & potential state update after unmountOnly the outer
timeris cleared; the nested 50 ms timeout isn’t.
Add a ref to store the inner id and clear both in the cleanup to avoid subtle memory leaks.- const timer = setTimeout(() => { + const timer = setTimeout(() => { ... - setTimeout(() => { + const inner = setTimeout(() => { setIsTransitioning(false); - }, 50); + }, 50); + // store so we can cancel on unmount + innerTimeoutRef.current = inner; }, 300); - return () => clearTimeout(timer); + return () => { + clearTimeout(timer); + clearTimeout(innerTimeoutRef.current); + };
56-57: Boolean coercion can be simplified
const lessonDecrypted = !paidCourse || decryptedLessonIds[lesson.id] || false;
The trailing|| falseis redundant—!paidCoursealready yields a boolean and the second term is eitherundefinedortrue.- const lessonDecrypted = !paidCourse || decryptedLessonIds[lesson.id] || false; + const lessonDecrypted = !paidCourse || Boolean(decryptedLessonIds[lesson.id]);src/hooks/courses/useCourseTabs.js (3)
38-52: Effect re-runs on every navigation becauserouter.queryis a mutable object
router.queryis a new object on each render, so it constantly invalidates thisuseEffect, even if the query params have not changed.
Destructure only the value(s) you care about to stabilise the dependency array:-useEffect(() => { - if (router.isReady) { - const { active } = router.query; +const { isReady, query: { active } = {} } = router; +useEffect(() => { + if (isReady) {
60-71: Validate requested tab names to avoid out-of-bounds errorsIf
indexOrNameis a string not present intabMap,tabNamewill beundefined, breaking downstream logic.
Consider an early return or default:-const tabName = typeof indexOrName === 'number' - ? tabMap[indexOrName] - : indexOrName; +const tabName = + typeof indexOrName === 'number' + ? tabMap[indexOrName] + : indexOrName; +if (!tabMap.includes(tabName)) return; // ignore bad input
80-105: CouplegetTabItemstotabMapinstead of hard-codingIf consumers supply a custom
options.tabMap,getTabItemsstill returns hard-coded items, causing index mismatches.
Generate labels/icons directly fromtabMap(with a lookup table) to keep both arrays in sync.src/pages/course/[slug]/index.js (2)
71-86: Reset corrupted localStorage entriesIf
JSON.parsefails we silently ignore the error but keep the stale/corrupt value in storage. Consider wiping the key so the user can recover automatically:} catch (error) { console.error('Error parsing completed lessons from storage:', error); + localStorage.removeItem(storageKey); + setCompletedLessons([]); }
190-190: Null-safe access tosession.user.purchased
session.user.purchasedmay beundefined; the current optional-chain prevents a crash, but the filter runs on an undefined array, returningundefinednotfalse. Wrap with?.some ?? falseto ensure a boolean.- session?.user?.purchased?.some(purchase => purchase.courseId === course?.d); + session?.user?.purchased?.some(p => p.courseId === course?.d) ?? false;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/components/content/courses/details/CourseDetails.js(1 hunks)src/components/content/courses/layout/CourseSidebar.js(1 hunks)src/components/content/courses/tabs/CourseContent.js(1 hunks)src/components/content/courses/tabs/CourseOverview.js(1 hunks)src/components/content/courses/tabs/CourseQA.js(1 hunks)src/components/forms/combined/CombinedResourceForm.js(2 hunks)src/components/forms/combined/EditDraftCombinedResourceForm.js(2 hunks)src/components/forms/combined/EditPublishedCombinedResourceForm.js(3 hunks)src/components/forms/course/embedded/EmbeddedDocumentForm.js(2 hunks)src/components/forms/document/DocumentForm.js(2 hunks)src/components/forms/document/EditDraftDocumentForm.js(2 hunks)src/components/forms/document/EditPublishedDocumentForm.js(2 hunks)src/components/forms/video/EditPublishedVideoForm.js(2 hunks)src/components/markdown/MarkdownEditor.js(1 hunks)src/components/navbar/Navbar.js(1 hunks)src/config/appConfig.js(1 hunks)src/hooks/courses/index.js(1 hunks)src/hooks/courses/useCourseData.js(1 hunks)src/hooks/courses/useCourseNavigation.js(1 hunks)src/hooks/courses/useCoursePayment.js(1 hunks)src/hooks/courses/useCourseTabs.js(1 hunks)src/hooks/courses/useLessons.js(1 hunks)src/hooks/encryption/useCourseDecryption.js(1 hunks)src/pages/course/[slug]/draft/index.js(1 hunks)src/pages/course/[slug]/index.js(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/components/forms/combined/EditDraftCombinedResourceForm.js (7)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)src/components/forms/combined/CombinedResourceForm.js (2)
content(24-24)handleContentChange(40-42)src/components/forms/combined/EditPublishedCombinedResourceForm.js (2)
content(32-32)handleContentChange(68-70)src/components/forms/course/embedded/EmbeddedDocumentForm.js (2)
content(26-26)handleContentChange(40-42)src/components/forms/document/DocumentForm.js (2)
content(23-23)handleContentChange(37-39)src/components/forms/document/EditPublishedDocumentForm.js (2)
content(31-31)handleContentChange(42-44)src/components/forms/document/EditDraftDocumentForm.js (2)
content(23-23)handleContentChange(37-39)
src/components/forms/combined/CombinedResourceForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/forms/document/DocumentForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/forms/document/EditPublishedDocumentForm.js (1)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)
src/components/content/courses/tabs/CourseOverview.js (3)
src/pages/course/[slug]/index.js (4)
completedLessons(32-32)isMobileView(39-39)handlePaymentSuccess(191-198)handlePaymentError(200-206)src/components/content/courses/lessons/DocumentLesson.js (1)
isMobileView(24-24)src/components/content/courses/details/CourseDetails.js (1)
CourseDetails(25-233)
src/components/forms/video/EditPublishedVideoForm.js (2)
src/components/markdown/MarkdownEditor.js (1)
MarkdownEditor(50-126)src/components/forms/combined/EditPublishedCombinedResourceForm.js (2)
videoEmbed(31-31)handleVideoEmbedChange(64-66)
src/hooks/courses/useLessons.js (5)
src/pages/course/[slug]/index.js (2)
useLessons(125-130)fetchAuthor(108-116)src/components/content/courses/details/CourseDetails.js (2)
fetchAuthor(88-99)author(36-36)src/components/content/courses/details/DraftCourseDetails.js (2)
fetchAuthor(35-46)author(23-23)src/pages/course/[slug]/draft/index.js (2)
fetchAuthor(31-43)lessons(15-15)src/hooks/courses/useCourseData.js (1)
lessonIds(15-15)
src/hooks/courses/useCoursePayment.js (2)
src/components/profile/UserAccountLinking.js (1)
useSession(24-24)src/hooks/useToast.js (1)
showToast(11-13)
src/hooks/courses/useCourseTabs.js (1)
src/hooks/useWindowWidth.js (1)
useWindowWidth(3-24)
🪛 Biome (1.9.4)
src/hooks/courses/useCoursePayment.js
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (35)
src/config/appConfig.js (1)
14-15: LGTM: New author public key addedThe addition of a new public key to the
authorPubkeysarray aligns with the course refactoring initiative, enabling proper authorization for the new course management functionality.src/components/markdown/MarkdownEditor.js (3)
1-37: Well-implemented dynamic import with theme overridingThe dynamic import approach is correctly implemented with SSR disabled for client-side rendering of the markdown editor. The theme overriding is elegantly handled by extending the module's default theme properties.
38-58: Well-documented component with comprehensive propsThe component is thoroughly documented with JSDoc comments, clearly describing all available props with their types and default values. The prop structure offers good flexibility with sensible defaults.
59-72: Clean implementation with proper prop forwardingThe component efficiently wraps the MDEditor with dark mode styling and properly forwards all relevant props, including the spread operator for additional props, enhancing reusability.
src/components/forms/combined/EditDraftCombinedResourceForm.js (2)
8-14: Good import organization and new MarkdownEditor componentThe imports have been well organized, with the custom MarkdownEditor component now properly imported. This is part of the broader course refactoring initiative to standardize the markdown editing experience.
243-243: Cleaner implementation with the new MarkdownEditor componentThe previous implementation likely used a direct import of the markdown editor with a wrapping div. This change simplifies the JSX and leverages the new reusable component, which is a good refactoring practice.
src/components/forms/document/EditDraftDocumentForm.js (2)
11-14: LGTM: Proper CSS imports and MarkdownEditor componentThe addition of the necessary CSS imports and the new MarkdownEditor component aligns with the standardization effort across form components in the codebase.
146-146: Simplified markdown editor implementationThe change to use the new MarkdownEditor component reduces boilerplate code and ensures a consistent styling and behavior across all forms in the application.
src/components/forms/document/DocumentForm.js (2)
12-12: Import standardization to use local MarkdownEditor componentThe code now imports the local
MarkdownEditorcomponent instead of dynamically importing the external MDEditor. This change helps standardize the markdown editing experience across the application.Also applies to: 14-14
148-152: Clean implementation using the central MarkdownEditor componentReplaced the direct usage of MDEditor with the wrapper div to the centralized MarkdownEditor component, which encapsulates the dark mode styling and consistent editor appearance.
src/components/forms/document/EditPublishedDocumentForm.js (3)
3-7: Improved organization of PrimeReact component importsImports are now grouped more logically with all PrimeReact UI components at the top, improving code readability.
10-18: Added explicit CSS imports and organized custom hooks importsAdded explicit PrimeReact CSS imports and reordered hook imports for better organization. This ensures all necessary styles are available and improves code maintainability.
201-201: Simplified markdown editor implementation with MarkdownEditor componentReplaced the complex MDEditor implementation with the centralized MarkdownEditor component, removing the need for wrapper divs with data-color-mode attributes.
src/components/forms/video/EditPublishedVideoForm.js (3)
3-7: Reorganized PrimeReact UI component importsUI component imports are now grouped together at the top of the file for better organization and readability.
15-19: Added explicit stylesheets and custom component importsAdded explicit imports for PrimeReact and PrimeIcons CSS, and the custom MarkdownEditor component, ensuring proper styling and consistent components usage.
192-192: Standardized video embed editing with MarkdownEditor componentReplaced the MDEditor implementation with the centralized MarkdownEditor component, maintaining the same functionality while using a consistent UI component.
src/components/navbar/Navbar.js (1)
11-11: Updated import path following course components restructuringThe import path for CourseHeader was updated to reflect the new directory structure, where course components are now organized into logical subdirectories (layout, tabs, etc.), improving code organization.
src/pages/course/[slug]/draft/index.js (1)
5-6: Import paths updated for better organizationThe import paths for
DraftCourseDetailsandDraftCourseLessonhave been updated to reflect the new directory structure, moving them into a dedicateddetails/subdirectory. This change aligns with the broader refactoring effort to improve modularity in the course components.src/components/content/courses/layout/CourseSidebar.js (1)
39-42: Improved navigation with client-side routingGreat improvement! The navigation logic has been updated to use client-side routing through the
onLessonSelectcallback instead of forcing a full page reload withwindow.location.href. This will provide a smoother user experience when navigating between lessons.src/hooks/courses/index.js (1)
1-13: Well-organized hook exportsThis new index file creates a centralized location for all course-related hooks, following a clean pattern for modular code organization. By re-exporting these hooks from a single file, you've made imports cleaner throughout the application and improved the maintainability of the codebase.
src/components/forms/combined/CombinedResourceForm.js (2)
8-14: Import reorganization improves code structure.The imports have been logically regrouped, and the markdown editor implementation has been standardized by replacing the external
@uiw/react-md-editorwith a local custom component.
200-200: Good refactoring of markdown editor integration.The markdown editor implementation has been successfully replaced with the new standardized
MarkdownEditorcomponent, which encapsulates the dark mode styling and configuration internally, making the code more maintainable.src/components/forms/course/embedded/EmbeddedDocumentForm.js (1)
185-185: Consistent implementation of markdown editor.The markdown editor has been standardized with the custom
MarkdownEditorcomponent, which is a good refactoring that maintains functionality while providing consistent styling across the application.src/components/forms/combined/EditPublishedCombinedResourceForm.js (2)
3-19: Improved import organization.The imports have been properly organized by grouping related PrimeReact components and styles together, which enhances readability and maintainability.
222-222: Consistent markdown editor implementation in both fields.Both the video embed editor and content editor now use the standardized
MarkdownEditorcomponent, ensuring consistent user experience and simplifying the codebase by removing duplicate styling logic.Also applies to: 239-239
src/components/content/courses/tabs/CourseQA.js (1)
1-32: Well-structured new component for course Q&A.This new component properly separates concerns by encapsulating the course comments functionality. The conditional rendering based on authorization status provides clear user feedback. The component is well-documented with JSDoc comments.
src/components/content/courses/tabs/CourseOverview.js (1)
21-56: Good responsive design pattern with conditional renderingThe component effectively handles different layouts for mobile and desktop views, with appropriate conditional rendering for the image and completion tag.
src/hooks/courses/useCoursePayment.js (2)
19-33: Comprehensive authorization logicThe authorization logic thoroughly covers all relevant scenarios:
- Subscription status
- Course creator check
- Course price check
- Individual course purchase verification
58-65: Nice API with complete return valuesThe hook returns all necessary values for components to handle payment flows and authorization, creating a clean separation between data/logic and presentation.
src/hooks/courses/useCourseNavigation.js (2)
44-57: Client-side navigation improvement with history APIThe
handleLessonSelectfunction smartly updates the URL without causing a page reload using the History API, which is great for maintaining state and improving user experience.
109-127: Excellent keyboard navigation implementationThe keyboard navigation with left/right arrow keys to switch tabs is a nice accessibility feature. The circular navigation (wrapping from last to first tab and vice versa) provides a smooth user experience.
src/hooks/courses/useCourseData.js (2)
40-54: Avoid repeatedndk.connect()calls & strengthen tag parsingInside
fetchCoursethe hook reconnects to NDK every time the effect runs.
That can be dozens of times on route changes or when any dependency mutates, adding needless latency.- await ndk.connect(); + // Connect only once – NDK silently ignores subsequent calls + if (!ndk.connected) { + await ndk.connect(); + }Also,
tag[1].split(':')[2]assumes a strict 3-part format. Add a length check to dodgeTypeError: Cannot read property '2' of undefined.- const lessonIds = event.tags.filter(tag => tag[0] === 'a').map(tag => tag[1].split(':')[2]); + const lessonIds = event.tags + .filter(tag => tag[0] === 'a' && typeof tag[1] === 'string') + .map(tag => { + const parts = tag[1].split(':'); + return parts.length === 3 ? parts[2] : null; + }) + .filter(Boolean);[ suggest_essential_refactor ]
74-75: Effect churn caused by unstable deps
router.query,ndk, andshowToastare new object/function references every render, so the effect constantly re-fires.
Consider wrappingshowToastinuseCallbackinside its provider and exposing a memoisedndkinstance (or omitting it from the dependency array if it’s globally stable).src/components/content/courses/tabs/CourseContent.js (1)
24-29: Guard against out-of-rangeactiveIndexIf
activeIndexis larger thanlessons.length - 1,currentLessonis set tonullbutlastActiveIndexkeeps the old value, so subsequent transitions won’t trigger.
Consider clampingactiveIndexor resettinglastActiveIndextogether.src/hooks/courses/useCourseTabs.js (1)
108-125: Avoid multiple global key-down listenersIf the hook is mounted more than once, each instance adds another
keydownlistener.
Track a ref counter or add the listener only once at module scope to prevent duplicate handlers.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/hooks/courses/useCourseNavigation.js (1)
47-59:⚠️ Potential issueReplace direct history manipulation with Next.js Router API.
The function uses direct DOM manipulation with
window.history.replaceState, which bypasses Next.js's router lifecycle. This was mentioned in a previous review but not addressed.Replace with Next.js's built-in shallow routing:
- const newUrl = `/course/${router.query.slug}?active=${index}`; - window.history.replaceState({ url: newUrl, as: newUrl, options: { shallow: true } }, '', newUrl); + router.push( + { + pathname: `/course/${router.query.slug}`, + query: { active: index } + }, + undefined, + { shallow: true } + );src/hooks/encryption/useCourseDecryption.js (1)
25-34:⚠️ Potential issueWrong property name breaks access-control logic.
course?.don line 30 is almost certainly a typo forcourse?.id. Due to this error,hasAccesswill always returnfalsefor users who purchased the course, blocking decryption and showing locked content.- session.user.purchased?.some(purchase => purchase.courseId === course?.d) || + session.user.purchased?.some(purchase => purchase.courseId === course?.id) ||src/pages/course/[slug]/index.js (1)
143-154: 🛠️ Refactor suggestionConsider replacing this
useEffectwithuseMemofor derived stateThis is a classic example of derived state. The
isDecryptingstate depends entirely on other state values and is recalculated whenever any of its dependencies change. As suggested in the past review comments, this could be simplified usinguseMemo.- const [isDecrypting, setIsDecrypting] = useState(false); + const isDecrypting = React.useMemo(() => { + if (!paidCourse || uniqueLessons.length === 0) return false; + const currentLesson = uniqueLessons[activeIndex]; + return currentLesson && !decryptedLessonIds[currentLesson.id]; + }, [activeIndex, uniqueLessons, decryptedLessonIds, paidCourse]); - useEffect(() => { - if (paidCourse && uniqueLessons.length > 0) { - const currentLesson = uniqueLessons[activeIndex]; - if (currentLesson && !decryptedLessonIds[currentLesson.id]) { - setIsDecrypting(true); - } else { - setIsDecrypting(false); - } - } else { - setIsDecrypting(false); - } - }, [activeIndex, uniqueLessons, decryptedLessonIds, paidCourse]);This approach:
- Eliminates an extra render cycle
- Removes the risk of race conditions
- Makes the code more declarative
🧹 Nitpick comments (1)
src/hooks/courses/useCourseTabsState.js (1)
107-125: Consider adding a dependency on document for the keyboard navigation effectThe keyboard navigation effect attaches event listeners to the
document, but doesn't include it in the dependencies array. Whiledocumentis unlikely to change, explicitly including it follows the React hooks exhaustive deps rule.}, [getActiveTabIndex, tabMap, toggleTab]); + }, [getActiveTabIndex, tabMap, toggleTab, typeof document !== 'undefined' ? document : undefined]);Alternatively, you could conditionally run the effect only in browser environments:
useEffect(() => { + if (typeof document === 'undefined') return; const handleKeyDown = (e) => { // ... }; document.addEventListener('keydown', handleKeyDown); return () => { document.removeEventListener('keydown', handleKeyDown); }; - }, [getActiveTabIndex, tabMap, toggleTab]); + }, [getActiveTabIndex, tabMap, toggleTab]);
📜 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 (11)
package.json(1 hunks)src/components/content/courses/tabs/CourseOverview.js(1 hunks)src/components/forms/course/embedded/EmbeddedDocumentForm.js(2 hunks)src/hooks/courses/index.js(1 hunks)src/hooks/courses/useCourseData.js(1 hunks)src/hooks/courses/useCourseNavigation.js(1 hunks)src/hooks/courses/useCoursePayment.js(1 hunks)src/hooks/courses/useCourseTabs.js(1 hunks)src/hooks/courses/useCourseTabsState.js(1 hunks)src/hooks/encryption/useCourseDecryption.js(1 hunks)src/pages/course/[slug]/index.js(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/hooks/courses/useCoursePayment.js
- src/components/forms/course/embedded/EmbeddedDocumentForm.js
- src/components/content/courses/tabs/CourseOverview.js
- src/hooks/courses/useCourseData.js
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/hooks/courses/useCourseTabs.js (2)
src/hooks/useWindowWidth.js (1)
useWindowWidth(3-24)src/hooks/courses/useCourseTabsState.js (3)
useCourseTabsState(13-138)tabMap(30-41)toggleTab(60-71)
src/hooks/courses/useCourseTabsState.js (2)
src/hooks/courses/useCourseTabs.js (3)
useCourseTabsState(20-33)isMobileView(17-17)toggleTab(54-76)src/hooks/courses/useCourseNavigation.js (1)
toggleTab(62-70)
🔇 Additional comments (24)
src/hooks/courses/index.js (1)
1-17: Good centralized module export pattern.This index file neatly consolidates and re-exports all course-related hooks, creating a clean entry point that simplifies imports throughout the application.
src/hooks/courses/useCourseNavigation.js (3)
10-45: Well-structured hook with good organization of navigation logic.This custom hook effectively extracts course navigation logic into a reusable unit, delegating core tab state management to the base
useCourseTabsStatehook. The initialization based on router parameters and mobile view detection is well-implemented with proper dependency arrays.
61-71: Well-implemented tab toggling with responsive behavior.The
toggleTabfunction correctly handles mobile vs. desktop behavior for sidebar visibility and is properly memoized with the right dependencies.
72-86: Clean and complete hook exports.The hook returns a comprehensive set of state variables and functions that cover all necessary navigation functionality.
src/hooks/courses/useCourseTabs.js (4)
6-14: Good use of deprecation notice.Clearly marking this hook as deprecated and suggesting alternatives helps guide developers to the preferred implementation. This is good documentation practice.
15-34: Effective delegation to base hook.The hook now properly delegates core functionality to
useCourseTabsState, addressing previous redundancy issues. The guard againstundefinedwindow width on line 17 is a good defensive coding practice.
35-51: Improved router integration.The effect now handles both
activeandtabquery parameters, providing better URL synchronization with application state.
52-76: Proper URL synchronization.The
toggleTabimplementation now correctly updates the URL query parameters with shallow routing, which was a missing feature identified in a previous review.src/hooks/encryption/useCourseDecryption.js (5)
4-24: Well-structured hook setup with proper state tracking.The hook initializes all necessary state and refs, including proper tracking of decrypted lessons, loading state, and processing flags. The addition of timeout refs (lines 11-12) addresses a previous issue with memory leaks.
35-43: Good lesson change detection and retry count reset.Tracking the last lesson ID and resetting retry counts when lessons change helps prevent excessive retry attempts on difficult-to-decrypt lessons.
44-132: Robust decryption implementation with timeouts and retries.The decryption function includes:
- Proper guards to prevent unnecessary processing
- Retry count tracking with a maximum limit
- Timeout handling for long-running decryptions
- Proper cleanup of timeouts
- Silent error handling to prevent UI disruption
The implementation now properly cleans up timeouts with
decryptTimeoutRef.current = null(line 94), addressing a previous issue.
133-142: Appropriate trigger for decryption.The effect correctly triggers decryption when:
- The active lesson changes
- The user has access
- The lesson isn't already decrypted
143-155: Proper cleanup on unmount.The hook now includes a cleanup effect that properly clears all timeouts when the component unmounts, addressing a previous issue with potential memory leaks.
src/hooks/courses/useCourseTabsState.js (1)
1-138: Well-designed and thoroughly documented hook for tab state managementThis custom hook provides an elegant solution for managing course tab navigation state. I appreciate the comprehensive JSDoc comments, clear structure with separated concerns, and modular approach.
Key strengths:
- Clear separation between pure state management and side effects
- Well-implemented keyboard navigation for accessibility
- Responsive design with mobile/desktop view adaptations
- Smart initialization and auto-updates for sidebar visibility
The code follows best practices by using appropriate React hooks (
useState,useMemo,useCallback,useEffect) for their intended purposes and maintaining clean dependencies arrays.src/pages/course/[slug]/index.js (10)
1-9: Buffer polyfill properly added – fixes the previous issueThe code now includes the
Bufferpolyfill from thebufferpackage, addressing the browser compatibility issue that was correctly identified in previous reviews.
11-27: Good organization of imports with clear grouping and commentsThe imports are now well-organized into logical sections with clear comments:
- React core and hooks
- Custom hooks
- Components
- Configuration
This makes the file more maintainable and improves readability.
38-54: Improved state management through custom course navigation hookThe refactoring has successfully extracted the navigation logic into a custom hook
useCourseNavigation. This reduces the complexity of the component and centralizes navigation state management.
71-87: Improved localStorage persistence with error handlingThe code now properly handles the loading of completed lessons from localStorage with appropriate error handling. The addition of the course ID and user public key to the storage key improves data isolation.
89-107: Well-implemented completion tracking with duplication preventionThe
setCompletedcallback now includes deduplication logic and proper localStorage persistence. The approach correctly avoids unnecessary state updates when a lesson is already marked as completed.
133-141: Now passing activeIndex to useCourseDecryption hookGood update to the
useCourseDecryptionhook call to include theactiveIndexparameter, which helps the hook know which lesson is currently active.
226-234: Clean menu tab implementation with proper handlersThe menu tab implementation now uses the navigation hook's methods properly. The component receives all necessary props from the hook, including tab items, active index, and toggle functions.
236-284: Good separation of concerns with modular tab componentsThe implementation has been refactored to use dedicated components for each tab type:
CourseOverviewfor the overview tabCourseContentfor the content tabCourseQAfor the QA tabThis approach greatly improves code organization and maintainability by moving the tab-specific rendering logic to specialized components. I also appreciate the improved loading state handling with the decryption spinner.
305-312: Consistent sidebar implementation with navigation hookThe sidebar implementation now uses the navigation hook's
handleLessonSelectmethod for selecting lessons, maintaining consistency across the UI.
323-329: Clear mobile sidebar handling with proper tab togglingThe mobile sidebar implementation also correctly uses the navigation hook's methods, including
handleLessonSelectfor selecting lessons andtoggleTabwithgetActiveTabIndexfor returning to the previous tab after closing the sidebar.
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores