Api connection #26
Api connection #26
Conversation
WalkthroughThe pull request introduces several changes across multiple components and files. Key modifications include the addition of new components such as Changes
Suggested labels
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 (
|
❌ Deploy Preview for wecooked failed.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 50
🧹 Outside diff range and nitpick comments (39)
app/client/components/sign-up/SignUp.tsx (1)
Line range hint
93-99: Consider security enhancementsThe form currently disables browser validation with
noValidateand lacks CSRF protection.Consider:
- Enabling browser validation as an additional layer of security
- Implementing CSRF protection using Next.js built-in CSRF protection
- Adding rate limiting for the sign-up endpoint to prevent abuse
<form onSubmit={handleSubmit} className="flex flex-col w-full space-y-4" aria-label="Sign up form" - noValidate >app/unsigned/components/MostPopularCourse.tsx (1)
Line range hint
1-70: Eliminate component duplicationThis component appears to be duplicated in both
app/unsignedandapp/clientdirectories with identical implementation. Consider:
- Moving the shared component to a common directory
- Creating a single source of truth for course data fetching
#!/bin/bash # Find similar component implementations ast-grep --pattern 'export default function MostPopularCourse() { $$$ }'app/client/components/video/search/TopRateCourse.tsx (3)
Line range hint
23-23: Align component name with file nameThe component is named
NewCourseCardbut exists in a file namedTopRateCourse.tsx. Consider renaming for consistency:
- Either rename the file to
NewCourseCard.tsx- Or rename the component to
TopRateCourse
Line range hint
7-70: Extract shared carousel functionalityThis component shares significant code with
MostPopularCourse, including:
- The
useResponsiveCardshook- Slide navigation logic
- Card rendering structure
Consider creating:
- A reusable carousel component
- A shared hook for responsive card handling
- A unified data fetching layer for courses
Example refactor:
// hooks/useResponsiveCards.ts export const useResponsiveCards = () => { // ... existing implementation } // components/common/CourseCarousel.tsx interface CourseCarouselProps { title: string; courses: Course[]; } export const CourseCarousel: React.FC<CourseCarouselProps> = ({ title, courses }) => { // ... shared carousel implementation } // components/TopRateCourse.tsx export default function TopRateCourse() { // Eventually replace with API call const courses = useCourses(); // Custom hook for data fetching return <CourseCarousel title="Top Rated Courses" courses={courses} />; }
1-1: Architectural Recommendations for API IntegrationThe current implementation has several architectural concerns that should be addressed:
API Integration
- Replace mock data with actual API calls
- Implement proper error handling and loading states
- Consider using React Query or SWR for data fetching
Code Organization
- Create a shared components directory for common UI elements
- Implement a proper data fetching layer
- Consider using a state management solution for course data
Type Safety
- Define proper interfaces for course data
- Add proper error boundaries
- Implement loading state types
Example implementation structure:
// types/course.ts interface Course { // ... course type definition } // api/courses.ts export const fetchCourses = async (): Promise<Course[]> => { // API implementation } // hooks/useCourses.ts export const useCourses = () => { const [courses, setCourses] = useState<Course[]>([]); const [loading, setLoading] = useState(true); const [error, setError] = useState<Error | null>(null); // Fetch implementation } // components/common/CourseCarousel.tsx // Shared carousel component // components/MostPopularCourse.tsx & TopRateCourse.tsx // Thin wrapper components using shared functionalityapp/client/components/video/search/NewCourseCard.tsx (1)
3-3: Consider consolidating duplicate componentsThis component exists in both
app/clientandapp/unsigneddirectories with identical code. This duplication increases maintenance overhead and the risk of inconsistencies.Consider:
- Moving the component to a shared location
- Using composition or props to handle any differences between signed-in and unsigned contexts
- Creating a clear separation of concerns between authentication states
Would you like assistance in refactoring this component to follow these architectural principles?
app/client/components/my-learning/GroupPage.tsx (1)
9-9: Update error message to match new terminologyThe title has been changed to "Workshops" but the error message still refers to "group courses". Consider updating the error message for consistency.
- <p className="text-white">No group courses available.</p> + <p className="text-white">No workshops available.</p>app/client/components/my-learning/GroupCardEnrolled.tsx (1)
Line range hint
1-67: Consider implementing API data fetchingGiven this PR's focus on API connection, consider updating this component to:
- Accept a data fetching prop or use a data fetching hook
- Handle loading and error states
- Implement proper API error boundaries
Here's a suggested approach:
interface GroupCardEnrolledProps extends Group { isLoading?: boolean; error?: Error; } export default function GroupCardEnrolled({ isLoading, error, ...groupProps }: GroupCardEnrolledProps) { if (isLoading) return <LoadingState />; if (error) return <ErrorState error={error} />; if (!groupProps.groupStatus) return null; // ... rest of the component }app/client/components/login&forget-password/Login.tsx (2)
69-99: Improve User Feedback on Login FailureWhen the login attempt fails, the user is not informed about the failure. Consider providing user-friendly error messages to enhance the user experience.
Line range hint
160-164: Update Sign-Up Link to Correct RouteThe "Sign up now" link points to
/client/sign-up, but it should be updated to/chef/sign-up-chefto direct users to the correct sign-up page.Apply this diff to fix the link:
- <Link - href="/client/sign-up" + <Link + href="/chef/sign-up-chef" className="cursor-pointer text-transparent bg-clip-text bg-gradient-to-t from-[#FE3511] to-[#F0725C] hover:underline font-medium" > Sign up now </Link>app/client/components/group/payment/GroupPayment.tsx (3)
107-133: Enhance User Feedback on Payment FailureWhen the payment request fails, the user is not notified. Consider displaying an error message to inform the user about the payment failure, improving the overall user experience.
109-113: Remove Debugging Console LogsThe
console.logstatements forIDandisDeliverare useful for debugging but should be removed or commented out in production code to clean up the console output.Apply this diff to remove the console logs:
- console.log('ID:', ID) - console.log('Current isDeliver state:', isDeliver)
221-235: Enhance Accessibility for Calendar IconAdd accessible attributes like
aria-labelto the calendar icon to improve usability for assistive technologies. Also, ensure the icon is keyboard-navigable for better accessibility.Apply this diff to add an
aria-label:<CalendarIcon onClick={toggleCalendar} + aria-label="Toggle calendar" className="h-5 w-5 text-gray-400 absolute right-3 top-3 cursor-pointer" />app/chef/components/sign-up-chef/SignUpChef.tsx (1)
49-65: Provide User Feedback on Sign-Up FailureWhen the sign-up fails, an error is logged to the console, but the user is not informed. Consider displaying an error message to notify the user about the sign-up failure.
app/client/components/video/payment/VideoPayment.tsx (1)
115-132: Enhance User Feedback on Payment FailureWhen the payment process fails, the user is not informed. Implement user-friendly error messages to notify the user about the payment failure.
app/chef/components/course/CourseUpload.tsx (1)
Line range hint
139-170: Consolidate submission logic to avoid duplicationThe
handleSubmitfunction contains two separate submission processes:
- Submitting
courseUploadDatato the backend endpoint usingfetch.- Calling
mockSubmitToAPIwithcourseData.This can lead to confusion and potential inconsistencies.
Consider consolidating the submission logic into a single process that submits the collected form data to the backend. Remove the redundant
mockSubmitToAPIcall if it's no longer needed. Here's how you might refactor the code:// Remove the second submission process - try { - const courseData: CourseSubmission = { - courseName, - courseDetail, - courseCategory, - courseDietary, - coursePrice, - coursePackPrice, - courseIngredients, - videoUrl, - coverImageUrl, - bookmarks: bookmarks.map((b) => ({ - title: b.title, - time: b.time, - timeStop: b.timeStop, - timeCountdown: b.timeCountdown, - })), - } - - const response = await mockSubmitToAPI(courseData) - - if (response.success) { - setSubmitSuccess(true) - resetForm() - } else { - throw new Error(response.message || 'Course submission failed') - } - } catch (error) { - setSubmitError(error instanceof Error ? error.message : 'Unknown error') - } finally { - setIsSubmitting(false) - } + // Move setIsSubmitting(false) to the finally block of the first try-catch + } finally { + setIsSubmitting(false) + }Ensure that all necessary form data is included in
courseUploadDataand properly handled by the backend.app/client/video/payment/[courseId]/page.tsx (1)
4-11: Consider renamingGroupPaymentPagefor clarityThe component is named
GroupPaymentPage, but since it handles payment for a specific course ([courseId]), consider renaming it to something likeCoursePaymentPageorVideoPaymentPagefor better clarity and consistency.app/chef/profile/page.tsx (2)
1-3: Consider using absolute imports for better maintainability.Replace relative imports with absolute imports to make the code more maintainable and resistant to refactoring.
-import Navbar from '../components/Navbar' -import ChefProfile from '../components/profile/ChefProfile' -import Footer from '../components/Footer' +import Navbar from '@/app/chef/components/Navbar' +import ChefProfile from '@/app/chef/components/profile/ChefProfile' +import Footer from '@/app/chef/components/Footer'
5-13: Add TypeScript type annotations for better type safety.The component should be explicitly typed as a React FC (FunctionComponent).
-export default function ChefProfilePage() { +import { FC } from 'react' + +const ChefProfilePage: FC = () => { return ( <> <Navbar /> <ChefProfile /> <Footer /> </> ) -} +} + +export default ChefProfilePageapp/client/profile/edit-profile/page.tsx (1)
3-4: Add newline between imports and component declaration.Maintain consistent code formatting across all profile pages.
import Footer from '../../components/Footer' + export default function ClientEditProfilePage() {app/client/data/client-review.ts (1)
13-13: Consider parameterizing the ingredientPrice valueThe hardcoded value of 10 for
ingredientPriceseems arbitrary. Consider:
- Moving this to a configuration file
- Making it dynamic based on API data (aligning with PR's "Api connection" objective)
app/client/profile/page.tsx (1)
1-4: Consolidate component imports from consistent locationsThe imports mix components from both
clientandcheffolders. This could lead to maintenance issues and unclear boundaries between client and chef features.Consider:
- Moving shared components to a common location
- Creating client-specific versions of components if they differ from chef versions
app/client/components/profile/CompleteCourse.tsx (1)
12-12: Replace hardcoded margins with responsive classesThe
mx-48class might cause issues on smaller screens. Consider using responsive utility classes or container classes.- <div className="flex flex-col justify-start mb-16 mx-48"> + <div className="flex flex-col justify-start mb-16 mx-4 sm:mx-8 md:mx-16 lg:mx-48">app/client/group/learning/[groupId]/page.tsx (1)
13-21: Improve semantic HTML structureThe sections lack proper semantic meaning and ARIA landmarks.
- <section> + <section aria-label="Group Learning" className="py-8"> <GroupLearning /> </section> - <section> + <section aria-label="Learning Curve" className="py-8"> <GroupCurvePage /> </section> - <section> + <section aria-label="Frequently Asked Questions" className="py-8"> <FAQPage /> </section>app/client/components/video/payment/VideoCancelPage.tsx (1)
20-25: Consider adding transaction details and retry optionThe error message could be more helpful by including:
- Transaction reference ID (if available)
- A "Retry Payment" button alongside the Home button
- Support contact information or link
app/chef/components/sign-up-chef/ThankYouPage.tsx (1)
Line range hint
27-31: Update button label to match destinationThe button label "Home" is misleading as it navigates to the course page.
<Link href="/chef/course"> <div className="px-16 py-3 text-white cursor-pointer bg-gradient-to-b from-[#F0725C] to-[#FE3511] rounded-full font-medium transition-all duration-300 transform hover:scale-105 hover:shadow-lg"> - Home + Go to Courses </div> </Link>app/unsigned/components/video/search/TopRateCourse.tsx (1)
Line range hint
40-48: Add loading and error states to the UIThe component should handle various states during API calls:
+ if (isLoading) return <div className="flex justify-center"><LoadingSpinner /></div>; + if (error) return <div className="text-red-500">Failed to load courses: {error.message}</div>; return ( // ... existing JSX );app/chef/components/Footer.tsx (1)
5-10: Consider moving footer elements to a configuration fileThe footer elements array should be moved to a configuration file for better maintainability and reusability.
// config/navigation.ts export const footerElements = { links: ['About', 'Terms & conditions', 'Privacy Policy', 'Contact'], socialMedia: [ { name: 'Facebook', icon: '/svg/fb-ic.svg', url: 'https://facebook.com/wecooked' }, // ... other social media ] }app/client/components/profile/CompleteCourseCard.tsx (1)
15-16: Add hover state feedback and loading state for imagesConsider adding a loading state for images and visual feedback for hover states.
-<div className="relative z-10 p-5 pr-7 rounded-xl bg-white my-4 transition-all duration-300 hover:scale-[1.01] shadow-lg"> +<div className="relative z-10 p-5 pr-7 rounded-xl bg-white my-4 transition-all duration-300 hover:scale-[1.01] hover:shadow-xl shadow-lg"> <div className="flex gap-4"> {/* Course Image */} <div className="relative w-28 h-28 rounded-lg overflow-hidden"> <Image src={course.courseImage} alt={course.courseTitle} fill className="object-cover" + loading="lazy" + placeholder="blur" + blurDataURL="data:image/jpeg;base64,/9j/4AAQSkZJRg..." />app/unsigned/components/video/course-detail/CourseDetailFirstPage.tsx (1)
Line range hint
24-30: Move hardcoded description to course dataThe course description is hardcoded and should be part of the course data.
// types/courses.ts interface Course { + description: string; // ... other properties } // CourseDetailFirstPage.tsx -<p className="text-xl max-w-2xl pt-12"> - Master the art of authentic Italian carbonara using just five - ingredients: spaghetti, eggs, Pecorino Romano, guanciale, and black - pepper. Learn key techniques to create a creamy, flavorful dish - without cream, perfecting the balance of rich sauce and crispy - guanciale. -</p> +<p className="text-xl max-w-2xl pt-12"> + {course.description} +</p>app/client/components/video/course-detail/CourseDetailFirstPage.tsx (2)
12-12: Remove debug console.log statementRemove development console.log statements before merging to production.
- console.log("ID is",ID)
57-60: Add loading state during navigationConsider adding a loading state while navigating to the payment page to improve user experience.
+ const [isNavigating, setIsNavigating] = useState(false) + + const handleNavigation = async () => { + setIsNavigating(true) + try { + await router.push(`/client/video/payment/${ID}`) + } catch (error) { + console.error('Navigation failed:', error) + } finally { + setIsNavigating(false) + } + } <button className="px-8 py-2.5 bg-gradient-to-b from-[#F0725C] to-[#FE3511] text-white rounded-full font-semibold transition-all duration-300 transform hover:scale-105 hover:shadow-lg" - onClick={() => router.push(`/client/video/payment/${ID}`)} + onClick={handleNavigation} + disabled={isNavigating} > - Let's Cook + {isNavigating ? 'Loading...' : 'Let\'s Cook'} </button>app/chef/components/Navbar.tsx (1)
Line range hint
31-37: Add loading state during logoutConsider adding a loading state during logout to prevent multiple clicks and improve user experience.
+ const [isLoggingOut, setIsLoggingOut] = useState(false) - const logout = () => { + const logout = async () => { + if (isLoggingOut) return + setIsLoggingOut(true) authcontextvalue.setIsAuthenticated(false) if (typeof window !== 'undefined') { localStorage.removeItem('isAuthenticated') } - router.push('/') + try { + await router.push('/') + } finally { + setIsLoggingOut(false) + } }app/client/data/full-mock-data.ts (3)
24-24: Standardize image pathsSeveral image paths are inconsistent:
- Most courses use 'ramachandra.png'
- Some use 'sushiplate.png', 'uma-parlour.png', 'roti.png', 'thai-food.png'
- Images don't always match the cuisine type (e.g., Indian courses using 'thai-food.png')
Consider:
- Using cuisine-specific images for each category
- Implementing a consistent naming convention for images
- Ensuring image paths reflect the actual content
Also applies to: 48-48, 60-60, 84-84, 144-144, 180-180, 264-264, 624-624, 744-744, 768-768, 840-840, 852-852, 984-984
26-26: Review empty dietary optionsSome courses have empty dietary arrays (
courseDietary: []), which might confuse users looking for dietary information.Consider:
- Adding a default dietary option like ['No Restrictions']
- Making dietary information mandatory for all courses
- Adding a validation check to ensure dietary information is provided
Also applies to: 262-262, 753-762
18-24: Standardize dietary combinationsSome courses have potentially conflicting dietary options:
- Courses marked both 'Vegan' and 'Vegetarian'
- Courses marked both 'Kosher' and 'Halal'
Consider:
- Implementing a validation system for dietary combinations
- Using mutually exclusive dietary categories
- Creating a hierarchy of dietary restrictions (e.g., Vegan implies Vegetarian)
Also applies to: 596-602
app/unsigned/components/video/search/NewCourseCard.tsx (3)
Line range hint
24-27: Add loading and error states for API readinessThe component currently assumes data is always available synchronously. To prepare for API integration, consider:
- Adding loading states while data is being fetched
- Implementing error handling for failed requests
- Handling empty states when no courses are available
export default function NewCourseCard() { const [startIndex, setStartIndex] = useState<number>(0); + const [isLoading, setIsLoading] = useState<boolean>(true); + const [error, setError] = useState<Error | null>(null); const cardsToShow = useResponsiveCards(); - const maxIndex = Math.ceil(courses.length / cardsToShow) - 1; + const maxIndex = courses?.length ? Math.ceil(courses.length / cardsToShow) - 1 : 0;
Line range hint
7-21: Optimize window resize performanceThe current implementation of
useResponsiveCardscould cause performance issues due to frequent resize event handling. Consider:
- Debouncing the resize event handler
- Using CSS media queries where possible
- Using a more performant way to detect viewport changes (e.g., ResizeObserver)
const useResponsiveCards = () => { const [cardsToShow, setCardsToShow] = useState<number>(3); + const debounce = (fn: Function, ms = 300) => { + let timeoutId: ReturnType<typeof setTimeout>; + return function (...args: any[]) { + clearTimeout(timeoutId); + timeoutId = setTimeout(() => fn.apply(this, args), ms); + }; + }; + useEffect(() => { const updateCards = () => { if (window.innerWidth < 640) setCardsToShow(1); else if (window.innerWidth < 1024) setCardsToShow(2); else setCardsToShow(3); }; - window.addEventListener('resize', updateCards); + const debouncedUpdateCards = debounce(updateCards); + window.addEventListener('resize', debouncedUpdateCards); updateCards(); - return () => window.removeEventListener('resize', updateCards); + return () => window.removeEventListener('resize', debouncedUpdateCards); }, []);
Line range hint
63-79: Prevent layout shifts in navigation controlsThe current implementation of navigation buttons can cause layout shifts when they appear/disappear. Consider:
- Keeping buttons in the DOM but changing their opacity/visibility
- Using a fixed width container for the navigation area
- {startIndex > 0 && ( <button + style={{ opacity: startIndex > 0 ? 1 : 0, pointerEvents: startIndex > 0 ? 'auto' : 'none' }} onClick={prevSlide} className="absolute left-4 top-1/2 -translate-y-1/2 bg-white p-2 rounded-full shadow-lg z-10" > <ChevronLeftIcon className="w-6 h-6" /> </button> - )} - {startIndex + 1 < maxIndex && ( <button + style={{ opacity: startIndex + 1 < maxIndex ? 1 : 0, pointerEvents: startIndex + 1 < maxIndex ? 'auto' : 'none' }} onClick={nextSlide} className="absolute right-4 top-1/2 -translate-y-1/2 bg-white p-2 rounded-full shadow-lg z-10" > <ChevronRightIcon className="w-6 h-6" /> </button> - )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/images/Aien.pngis excluded by!**/*.png
📒 Files selected for processing (49)
.gitignore(1 hunks)app/chef/components/Footer.tsx(1 hunks)app/chef/components/Navbar.tsx(1 hunks)app/chef/components/course/CourseUpload.tsx(1 hunks)app/chef/components/login&forget-password/Login.tsx(3 hunks)app/chef/components/profile/ChefProfile.tsx(1 hunks)app/chef/components/profile/EditChefProfile.tsx(1 hunks)app/chef/components/sign-up-chef/SignUpChef.tsx(6 hunks)app/chef/components/sign-up-chef/ThankYouPage.tsx(1 hunks)app/chef/profile/edit-profile/page.tsx(1 hunks)app/chef/profile/page.tsx(1 hunks)app/client/components/MostPopularCourse.tsx(1 hunks)app/client/components/Navbar.tsx(1 hunks)app/client/components/group/GroupLearning.tsx(2 hunks)app/client/components/group/course-detail/GroupCourseDetailFirstPage.tsx(3 hunks)app/client/components/group/payment/GroupPayment.tsx(4 hunks)app/client/components/login&forget-password/Login.tsx(2 hunks)app/client/components/my-learning/GroupCardEnrolled.tsx(1 hunks)app/client/components/my-learning/GroupPage.tsx(1 hunks)app/client/components/my-learning/ProgressPage.tsx(1 hunks)app/client/components/profile/ClientProfile.tsx(1 hunks)app/client/components/profile/CompleteCourse.tsx(1 hunks)app/client/components/profile/CompleteCourseCard.tsx(1 hunks)app/client/components/profile/EditClientProfile.tsx(1 hunks)app/client/components/sign-up/SignUp.tsx(1 hunks)app/client/components/video-player/VideoControl.tsx(1 hunks)app/client/components/video/course-detail/CourseDetailFirstPage.tsx(3 hunks)app/client/components/video/payment/VideoCancelPage.tsx(1 hunks)app/client/components/video/payment/VideoPayment.tsx(1 hunks)app/client/components/video/payment/VideoThankYouPage.tsx(1 hunks)app/client/components/video/search/NewCourseCard.tsx(1 hunks)app/client/components/video/search/TopRateCourse.tsx(1 hunks)app/client/data/client-review.ts(1 hunks)app/client/data/full-mock-data.ts(84 hunks)app/client/group/learning/[groupId]/page.tsx(1 hunks)app/client/group/learning/page.tsx(0 hunks)app/client/profile/edit-profile/page.tsx(1 hunks)app/client/profile/page.tsx(1 hunks)app/client/types/courses.d.ts(1 hunks)app/client/video/complete/page.tsx(1 hunks)app/client/video/incomplete/page.tsx(1 hunks)app/client/video/payment/[courseId]/page.tsx(1 hunks)app/unsigned/components/MostPopularCourse.tsx(1 hunks)app/unsigned/components/group/UpcomingWorkshopCard.tsx(3 hunks)app/unsigned/components/video/course-detail/CourseDetailFirstPage.tsx(3 hunks)app/unsigned/components/video/search/NewCourseCard.tsx(1 hunks)app/unsigned/components/video/search/TopRateCourse.tsx(1 hunks)app/unsigned/types/group.d.ts(1 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (1)
- app/client/group/learning/page.tsx
✅ Files skipped from review due to trivial changes (3)
- app/client/video/incomplete/page.tsx
- app/client/components/my-learning/ProgressPage.tsx
- app/client/components/Navbar.tsx
🧰 Additional context used
📓 Learnings (5)
app/unsigned/types/group.d.ts (1)
Learnt from: tong788
PR: FWB-Creators/WeCooked-Frontend#21
File: app/client/types/group.d.ts:0-0
Timestamp: 2024-11-26T07:45:38.638Z
Learning: In `app/client/types/group.d.ts`, the `ingredientPrice` property in the `Group` interface is required and should not be optional.
app/client/group/learning/[groupId]/page.tsx (1)
Learnt from: tong788
PR: FWB-Creators/WeCooked-Frontend#17
File: app/client/group/learning/page.tsx:7-7
Timestamp: 2024-11-17T16:05:40.060Z
Learning: In `app/client/group/learning/page.tsx`, the `FAQPage` component is correctly imported from `../../components/FAQPage`.
app/unsigned/components/video/search/NewCourseCard.tsx (1)
Learnt from: sikares
PR: FWB-Creators/WeCooked-Frontend#8
File: app/client/components/Card.tsx:30-34
Timestamp: 2024-11-12T12:21:03.016Z
Learning: In the `Card` component located at `app/client/components/Card.tsx`, the `courses` data is static and does not require loading or error state handling.
app/unsigned/components/video/course-detail/CourseDetailFirstPage.tsx (2)
Learnt from: sikares
PR: FWB-Creators/WeCooked-Frontend#11
File: app/client/video/course-detail/[course_id]/page.tsx:9-21
Timestamp: 2024-11-12T12:21:03.016Z
Learning: In Next.js 13 using the app directory, dynamic route parameters should be accessed using the `useParams` hook from 'next/navigation', not passed through component props.
Learnt from: sikares
PR: FWB-Creators/WeCooked-Frontend#11
File: app/client/video/course-detail/[course_id]/page.tsx:1-8
Timestamp: 2024-11-12T12:21:03.016Z
Learning: In the 'course detail' page component, it's not necessary to use `useEffect` and `useState` when handling dynamic route parameters.
app/client/components/video/course-detail/CourseDetailFirstPage.tsx (2)
Learnt from: sikares
PR: FWB-Creators/WeCooked-Frontend#11
File: app/client/video/course-detail/[course_id]/page.tsx:9-21
Timestamp: 2024-11-12T12:21:03.016Z
Learning: In Next.js 13 using the app directory, dynamic route parameters should be accessed using the `useParams` hook from 'next/navigation', not passed through component props.
Learnt from: sikares
PR: FWB-Creators/WeCooked-Frontend#11
File: app/client/video/course-detail/[course_id]/page.tsx:1-8
Timestamp: 2024-11-12T12:21:03.016Z
Learning: In the 'course detail' page component, it's not necessary to use `useEffect` and `useState` when handling dynamic route parameters.
🔇 Additional comments (17)
app/client/components/MostPopularCourse.tsx (1)
Line range hint 1-70: Duplicate implementation detected
This is an identical implementation to app/unsigned/components/MostPopularCourse.tsx. See previous comments about component consolidation and API integration.
app/unsigned/components/group/UpcomingWorkshopCard.tsx (3)
9-15: LGTM: Improved SSR compatibility in useResponsiveCards initialization
The lazy initialization with window check properly handles server-side rendering by avoiding direct window references during the initial render.
63-64: LGTM: Improved card slider implementation
The simplified transform calculation and dynamic width adjustment create a more robust sliding experience while maintaining proper card spacing.
84-84: LGTM: Simplified navigation button logic
The simplified condition for showing the next button is more readable while maintaining the same functionality.
app/client/components/video/search/NewCourseCard.tsx (1)
3-3: Verify the impact of data source change
The change in import path from new-course to full-mock-data suggests a broader data source reorganization. However, this seems unrelated to the PR's stated objective of API connection.
Let's check for other components using this data source:
✅ Verification successful
Let me verify the content of both data files to understand if this is a legitimate reorganization or potential issue.
Data source change is part of a legitimate enhancement
The change from new-course to full-mock-data is a valid enhancement, as the new data source:
- Includes all courses from the old file plus additional ones
- Adds a new
ingredientPricefield to all courses - Maintains consistent data structure and types
- Is being used consistently across multiple components in both
clientandunsigneddirectories
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using either the old or new data source
echo "Components using old data source:"
rg -l "@/app/(client|unsigned)/data/new-course"
echo -e "\nComponents using new data source:"
rg -l "@/app/(client|unsigned)/data/full-mock-data"
Length of output: 821
Script:
#!/bin/bash
# Check content and structure of both data files
echo "Content of new-course data file:"
fd "new-course.ts" --exec cat {} \;
echo -e "\nContent of full-mock-data file:"
fd "full-mock-data.ts" --exec cat {} \;
Length of output: 60293
app/client/components/my-learning/GroupPage.tsx (1)
Line range hint 1-18: Verify API integration for workshop data
The component is still using static data imported from ../../data/group-course. Given that this PR is titled "Api connection", shouldn't this component fetch data from an API instead?
Let's verify if there are any API-related changes in the codebase:
app/client/components/my-learning/GroupCardEnrolled.tsx (1)
30-30: Verify route handler for updated URL pattern
The URL pattern has been updated to /client/group/learning/${groupId}. Please ensure that:
- The corresponding Next.js route handler exists
- All other components linking to this route are updated
✅ Verification successful
URL route change is properly implemented
The verification confirms that:
- The new route handler exists at
app/client/group/learning/[groupId]/page.tsx - No instances of the old URL pattern (
client/group-learning) were found - The new URL pattern is only used in the updated component
- The route structure follows Next.js app router conventions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the old URL pattern to ensure all occurrences are updated
rg "client/group-learning/[^/]"
# Check for the existence of the new route handler
fd -g "group/learning/[[]*].tsx" app/client
Length of output: 79
Script:
#!/bin/bash
# Search for any routes or components using the group learning URL pattern with variations
rg -i "group.*learning.*\$\{.*\}" --type tsx
# Look for route handlers in potential locations
fd -e tsx -e ts "route.ts" app/client/group
fd -e tsx -e ts "page.ts" app/client/group
# Check for any Link components referencing group learning
ast-grep --pattern 'Link $_ { href: $_ }'
Length of output: 483
Script:
#!/bin/bash
# Let's check the content of the route handler file we found
cat "app/client/group/learning/[groupId]/page.tsx"
# Search for any Link components with the old URL pattern (trying without tsx extension)
rg "client/group-learning"
# Search for any Link components with the new URL pattern
rg "client/group/learning"
Length of output: 1040
app/client/video/complete/page.tsx (1)
1-6: Component implementation is correct
The GroupCompletePage component correctly imports and renders the VideoThankYouPage component.
app/client/types/courses.d.ts (1)
6-6: ingredientPrice property added correctly
The ingredientPrice: number; property has been appropriately added to the Course interface, aligning with the existing naming conventions.
app/chef/profile/edit-profile/page.tsx (1)
1-13: Apply consistent improvements as ChefProfilePage.
The same improvements regarding absolute imports and TypeScript types should be applied here for consistency.
Let's verify the EditChefProfile component exists:
✅ Verification successful
Update imports to use absolute paths and add TypeScript types
- Update relative imports to use absolute paths starting with '@/app/chef'
- Add type annotations for the component function
Example:
import Navbar from '@/app/chef/components/Navbar'
import EditChefProfile from '@/app/chef/components/profile/EditChefProfile'
import Footer from '@/app/chef/components/Footer'
export default function EditChefProfilePage(): JSX.Element {
// ...
}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if EditChefProfile component exists
fd -e tsx -e ts "EditChefProfile" app/chef/components/profile/
Length of output: 110
Script:
#!/bin/bash
# Let's verify the imports in EditChefProfile component
cat app/chef/components/profile/EditChefProfile.tsx
Length of output: 6991
Script:
#!/bin/bash
# Let's check the ChefProfilePage to compare the import structure
cat app/chef/profile/page.tsx
Length of output: 320
app/unsigned/types/group.d.ts (1)
6-6: Verify the placement of pricing data in unsigned context
The ingredientPrice addition is consistent with requirements, but please verify if pricing information should be accessible in the unsigned (unauthenticated) context.
✅ Verification successful
Let me check the actual usage of price-related fields in the unsigned context to better understand the architectural pattern.
Price information is consistently exposed in unsigned context
The addition of ingredientPrice aligns with the existing architectural pattern where pricing information is intentionally exposed in the unsigned context. Evidence shows:
- Multiple price-related fields exist in unsigned types:
groupPrice,coursePrice,ingredientPrice - Price information is used in public-facing components for:
- Course and group listings
- Price range filters
- Display of prices in THB currency
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if pricing-related fields are consistently placed in unsigned context
rg -l "Price|price" "app/unsigned/"
# Check for any authentication guards around pricing data
ast-grep --pattern 'auth$$$Price'
Length of output: 763
Script:
#!/bin/bash
# Check the content of files with price-related fields to understand the context
rg "price|Price" "app/unsigned/types/" -A 2 -B 2
# Check components using price data
rg "price|Price" "app/unsigned/components/" -A 2 -B 2
Length of output: 13245
.gitignore (1)
29-29: Clarify environment file configuration
The file already includes .env*.local pattern on line 29, and now adds .env on line 37. For Next.js applications:
.env.localis typically used for local secrets.envis used for default/public values
Consider documenting in the README which env files should be used for what purpose to avoid confusion.
Also applies to: 37-37
✅ Verification successful
Both .env and .env*.local patterns are valid for this Next.js project
This is a standard Next.js project that follows Next.js environment variable conventions. Having both patterns in .gitignore is correct because:
.envcovers the default environment file that shouldn't be committed.env*.localcovers local override files (e.g.,.env.local,.env.development.local)
However, you're right about documenting the environment setup. The default Next.js README doesn't include any environment configuration guidance, which should be added for the API integration.
- Add environment setup documentation to README.md explaining:
.envfor default/fallback values.env.localfor local development secrets.env.production.localfor production overrides
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there's documentation about env files
rg -l "env|environment" README.md
# Check for existing env files that might conflict
fd -H "^\.env" --exclude node_modules
Length of output: 75
Script:
#!/bin/bash
# Check if README exists and its content
fd -g "README*" -x cat {}
# Look for any environment setup documentation in docs folder
fd -g "*env*.md" -g "*environment*.md" -x cat {}
# Check for Next.js configuration files that might have env documentation
fd -g "next.config.*" -x cat {}
# Look for package.json to verify if it's a Next.js project
cat package.json
Length of output: 2861
app/client/group/learning/[groupId]/page.tsx (1)
14-14:
Pass groupId to GroupLearning component
The component is within a dynamic [groupId] route but doesn't pass the ID to the GroupLearning component.
- <GroupLearning />
+ <GroupLearning groupId={params.groupId} />Add the following at the top of the component:
import { useParams } from 'next/navigation'
export default function GroupLearningPage() {
const params = useParams()⛔ Skipped due to learnings
Learnt from: tong788
PR: FWB-Creators/WeCooked-Frontend#17
File: app/client/components/group/GroupLearning.tsx:15-45
Timestamp: 2024-11-16T20:48:57.204Z
Learning: In the `GroupLearning` component (`app/client/components/group/GroupLearning.tsx`), the data is already being handled elsewhere, so using hardcoded values is acceptable in this context.
app/client/components/video/payment/VideoThankYouPage.tsx (1)
18-26: Consider fetching and displaying actual order details
The thank you message currently uses static text. Since this is part of an API connection PR, consider integrating with the backend to display actual purchase details like order ID, purchased items, and delivery information.
app/client/components/group/course-detail/GroupCourseDetailFirstPage.tsx (1)
10-10: Remove debug console.log statement
See previous comment about removing console.log statements.
app/chef/components/Navbar.tsx (1)
92-92: Verify chef profile route changes across the application
The profile route has been changed from /client/profile to /chef/profile. Let's verify this change is consistent across the application.
✅ Verification successful
Route changes need to be reviewed for consistency
The search results show that /client/profile route is still actively used in client-related components, which is expected. However, we should verify that the chef profile routes are properly implemented and there are no mixed references. Let's check the content of chef profile components.
Route changes are properly implemented and consistent
The verification shows that:
- The
/chef/profileroutes are consistently used within the chef components - The
/client/profileroutes remain in client-specific components - The navigation flow is properly maintained (profile -> edit-profile -> profile)
- There are no mixed references between chef and client profile routes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old profile route
echo "Checking for old route references..."
rg "/client/profile"
# Search for the new chef profile route implementation
echo "Verifying new route implementation..."
fd "profile" app/chef/
Length of output: 802
Script:
#!/bin/bash
# Check content of chef profile components for route consistency
echo "Checking ChefProfile component..."
cat app/chef/components/profile/ChefProfile.tsx
echo -e "\nChecking EditChefProfile component..."
cat app/chef/components/profile/EditChefProfile.tsx
# Search for any mixed references of chef/client profile routes
echo -e "\nChecking for mixed route references..."
rg -A 2 "href=.*profile" app/chef/
Length of output: 16797
app/unsigned/components/video/search/NewCourseCard.tsx (1)
3-3: Verify alignment with API connection objectives
The PR title suggests implementing API connections, but the component is still using mock data from a different static source. Consider implementing actual API integration instead of switching between mock data files.
| const handleSubmit = async (e: FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault() | ||
|
|
||
| try { | ||
| if (!firstname || !lastname || !email || !password) { | ||
| //validation | ||
| // Validation | ||
| throw new Error('All fields are required') | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance form validation and user feedback
The current validation only checks for empty fields. Consider adding:
- Email format validation
- Password strength requirements
- Visual feedback for validation errors
Example implementation:
const validateForm = () => {
const errors: Record<string, string> = {};
if (!firstname.trim()) errors.firstname = 'First name is required';
if (!lastname.trim()) errors.lastname = 'Last name is required';
if (!email.trim()) errors.email = 'Email is required';
else if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) {
errors.email = 'Invalid email format';
}
if (!password) errors.password = 'Password is required';
else if (password.length < 8) {
errors.password = 'Password must be at least 8 characters';
}
return errors;
}| const clientSignUpData = { | ||
| firstName: firstname, | ||
| lastName: lastname, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistent naming convention across state and data structure
There's an inconsistency between state variable names (firstname, lastname) and the object property names (firstName, lastName). This could lead to confusion during maintenance.
Consider updating the state variable names to match the camelCase convention:
- const [firstname, setFirstname] = useState<string>('')
- const [lastname, setLastname] = useState<string>('')
+ const [firstName, setFirstName] = useState<string>('')
+ const [lastName, setLastName] = useState<string>('')
const clientSignUpData = {
- firstName: firstname,
- lastName: lastname,
+ firstName,
+ lastName,
email,
password,
}Committable suggestion skipped: line range outside the PR's diff.
| const handleSubmit = async (e: FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault() | ||
|
|
||
| try { | ||
| if (!firstname || !lastname || !email || !password) { | ||
| //validation | ||
| // Validation | ||
| throw new Error('All fields are required') | ||
| } | ||
|
|
||
| const response = await fetch( | ||
| `${process.env.NEXT_PUBLIC_BACKEND_URL}/user/signup`, | ||
| { | ||
| //change the url to correspond with backend | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(clientSignUpData), | ||
| } | ||
| ) | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error('Registration failed') | ||
| } | ||
|
|
||
| // If the request is successful, redirect the user to the login page |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add loading state and improve error handling
The form submission lacks loading state and proper error feedback to users.
Consider implementing:
- Loading state during API call
- Proper error messages from the API
- Success feedback before redirect
+ const [isLoading, setIsLoading] = useState(false);
+ const [error, setError] = useState<string | null>(null);
const handleSubmit = async (e: FormEvent<HTMLFormElement>) => {
e.preventDefault();
+ setIsLoading(true);
+ setError(null);
try {
// ... existing validation ...
const response = await fetch(
`${process.env.NEXT_PUBLIC_BACKEND_URL}/user/signup`,
{
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(clientSignUpData),
}
);
+ const data = await response.json();
if (!response.ok) {
- throw new Error('Registration failed');
+ throw new Error(data.message || 'Registration failed');
}
+ // Show success message
+ toast.success('Registration successful! Redirecting...');
// Delay redirect slightly to show success message
+ setTimeout(() => {
router.push('/client/login');
+ }, 1500);
} catch (error) {
- console.error('Registration failed:', error);
+ setError(error instanceof Error ? error.message : 'Registration failed');
+ toast.error(error instanceof Error ? error.message : 'Registration failed');
} finally {
+ setIsLoading(false);
}
};Committable suggestion skipped: line range outside the PR's diff.
| const response = await fetch( | ||
| `${process.env.NEXT_PUBLIC_BACKEND_URL}/user/signup`, | ||
| { | ||
| //change the url to correspond with backend | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(clientSignUpData), | ||
| } | ||
| ) |
There was a problem hiding this comment.
💡 Codebase verification
Environment variable configuration needs to be documented
The NEXT_PUBLIC_BACKEND_URL environment variable is widely used across multiple components in the application, but there's no documentation or example configuration for it. This could cause setup issues for other developers.
- Add
.env.examplefile with the requiredNEXT_PUBLIC_BACKEND_URLvariable - Update README.md with environment setup instructions
- Consider adding environment setup steps in the development documentation
🔗 Analysis chain
Verify environment variable configuration
The API endpoint relies on an environment variable. Ensure it's properly configured and documented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if .env.example exists and contains the required variable
if [ -f .env.example ]; then
echo "Checking .env.example for NEXT_PUBLIC_BACKEND_URL..."
grep "NEXT_PUBLIC_BACKEND_URL" .env.example
else
echo ".env.example not found"
fi
# Check if README contains environment setup instructions
if [ -f README.md ]; then
echo "Checking README.md for environment setup instructions..."
rg -i "environment|env|backend.*url" README.md
fi
Length of output: 275
Script:
#!/bin/bash
# Let's check for any environment-related files in the repository
fd -t f "^\.env" -x echo "Found environment file: {}"
# Check for environment setup in any documentation files
fd -t f "(README|CONTRIBUTING|SETUP|DEVELOPMENT|ENV)" -i -x echo "Found documentation file: {}"
# Search for environment variable usage in the codebase
rg "NEXT_PUBLIC_BACKEND_URL" -A 2 -B 2
# Look for environment setup in package.json scripts
if [ -f "package.json" ]; then
echo "Checking package.json for environment setup..."
jq '.scripts' package.json
fi
Length of output: 3924
| @@ -1,6 +1,6 @@ | |||
| 'use client' | |||
| import { useState, useEffect } from 'react' | |||
| import { courses } from '@/app/unsigned/data/most-popular-course' | |||
| import { courses } from '@/app/client/data/full-mock-data' | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the evidence gathered from the codebase:
Consider implementing API integration for popular courses
The current implementation uses static mock data from full-mock-data.ts while other components in the application (SignUpChef, CourseUpload, VideoPayment, etc.) are already using API endpoints. To maintain consistency with the PR's API integration objective:
- Replace the static import with a data fetching mechanism similar to other components
- Consider implementing server-side data fetching for better performance
- Remove duplicate components between unsigned and client directories to improve maintainability
🔗 Analysis chain
Consider replacing mock data with API integration
While the import has been updated to use full-mock-data, this seems inconsistent with the PR's objective of implementing API connections. Consider integrating with the actual API endpoint instead of using mock data.
Let me gather more information about the MostPopularCourse component and its usage.
Let me check if there are any API endpoints related to popular courses that could be used instead of mock data.
Let me check if there's an API endpoint for fetching popular courses.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any API-related implementations
rg -g '!*.test.*' -g '!*.spec.*' "fetch|axios" ./app
Length of output: 756
Script:
#!/bin/bash
# Check for MostPopularCourse component implementations and usages
rg -g '!*.test.*' -g '!*.spec.*' "MostPopularCourse" ./app
# Check the content of both MostPopularCourse components
fd "MostPopularCourse.tsx" ./app --exec cat {}
Length of output: 5780
Script:
#!/bin/bash
# Look for API endpoints or services related to courses
rg -g '!*.test.*' -g '!*.spec.*' "api.*course|service.*course" ./app
# Check the mock data structure
fd "full-mock-data.ts" ./app --exec cat {}
Length of output: 52817
Script:
#!/bin/bash
# Look for API endpoints or routes related to popular courses
rg -g '!*.test.*' -g '!*.spec.*' "popular.*course|course.*popular" ./app
# Check if there's any API route configuration
fd "route.ts|api" ./app --exec cat {}
Length of output: 116
| if (remember) { | ||
| // Store authentication state in cookies | ||
| Cookies.set('Authorization', 'true', { expires: 7 }) // expires in 7 days if "Remember me" is checked | ||
| } else { | ||
|
|
There was a problem hiding this comment.
Enhance cookie security settings
The cookie settings need security improvements:
- Add
httpOnlyflag - Add
secureflag for HTTPS - Add
sameSiteattribute
Update cookie settings:
-Cookies.set('Authorization', 'true', { expires: 7 })
+Cookies.set('Authorization', 'true', {
+ expires: 7,
+ secure: true,
+ sameSite: 'strict',
+ httpOnly: true
+})Committable suggestion skipped: line range outside the PR's diff.
| <div className=" flex justify-between"> | ||
| <div> | ||
| <div className="flex"> | ||
| {[...Array(5)].map((_, index) => ( | ||
| <div key={index} className="flex items-center"> | ||
| <svg | ||
| className="h-5 w-5 text-green-400" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" | ||
| aria-hidden="true" | ||
| > | ||
| <path | ||
| fill="currentColor" | ||
| d="M10 15.27L16.18 19l-1.64-7.03L20 7.24l-7.19-.61L10 0 7.19 6.63 0 7.24l5.46 4.73L3.82 19z" | ||
| /> | ||
| </svg> | ||
| <span className="text-black font-semibold ml-1"></span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 4.5 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Course Rating | ||
| </p> | ||
| </div> | ||
| <div> | ||
| <div className="flex"> | ||
| {[...Array(5)].map((_, index) => ( | ||
| <div key={index} className="flex items-center"> | ||
| <svg | ||
| className="h-5 w-5 text-green-400" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" | ||
| aria-hidden="true" | ||
| > | ||
| <path | ||
| fill="currentColor" | ||
| d="M10 15.27L16.18 19l-1.64-7.03L20 7.24l-7.19-.61L10 0 7.19 6.63 0 7.24l5.46 4.73L3.82 19z" | ||
| /> | ||
| </svg> | ||
| <span className="text-black font-semibold ml-1"></span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 4.5 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Workshop Rating | ||
| </p> | ||
| </div> | ||
| <div> | ||
| <div className="flex"> | ||
| {[...Array(5)].map((_, index) => ( | ||
| <div key={index} className="flex items-center"> | ||
| <svg | ||
| className="h-5 w-5 text-green-400" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" | ||
| aria-hidden="true" | ||
| > | ||
| <path | ||
| fill="currentColor" | ||
| d="M10 15.27L16.18 19l-1.64-7.03L20 7.24l-7.19-.61L10 0 7.19 6.63 0 7.24l5.46 4.73L3.82 19z" | ||
| /> | ||
| </svg> | ||
| <span className="text-black font-semibold ml-1"></span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 4.5 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Overall Rating | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor rating components for better reusability
The rating section contains duplicated code and hardcoded values. Consider extracting it into a reusable component.
Create a new RatingSection component:
interface RatingProps {
value: number;
label: string;
}
function RatingSection({ value, label }: RatingProps) {
return (
<div>
<StarRating value={value} />
<p className="text-[#F9644A] text-3xl font-bold flex justify-center">
{value}
</p>
<p className="text-[#919295] flex justify-center">
{label}
</p>
</div>
);
}Usage:
-<div>
- {/* Repeated rating code */}
-</div>
+<div className="flex justify-between">
+ <RatingSection value={4.5} label="Course Rating" />
+ <RatingSection value={4.5} label="Workshop Rating" />
+ <RatingSection value={4.5} label="Overall Rating" />
+</div>| {[...Array(5)].map((_, index) => ( | ||
| <div key={index} className="flex items-center"> | ||
| <svg | ||
| className="h-5 w-5 text-green-400" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" | ||
| aria-hidden="true" | ||
| > | ||
| <path | ||
| fill="currentColor" | ||
| d="M10 15.27L16.18 19l-1.64-7.03L20 7.24l-7.19-.61L10 0 7.19 6.63 0 7.24l5.46 4.73L3.82 19z" | ||
| /> | ||
| </svg> | ||
| <span className="text-black font-semibold ml-1"></span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 4.5 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Workshop Rating | ||
| </p> | ||
| </div> | ||
| <div> | ||
| <div className="flex"> | ||
| {[...Array(5)].map((_, index) => ( | ||
| <div key={index} className="flex items-center"> | ||
| <svg | ||
| className="h-5 w-5 text-green-400" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" | ||
| aria-hidden="true" | ||
| > | ||
| <path | ||
| fill="currentColor" | ||
| d="M10 15.27L16.18 19l-1.64-7.03L20 7.24l-7.19-.61L10 0 7.19 6.63 0 7.24l5.46 4.73L3.82 19z" | ||
| /> | ||
| </svg> | ||
| <span className="text-black font-semibold ml-1"></span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 4.5 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Overall Rating | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/*middle line*/} | ||
| <div></div> | ||
| <div className="flex justify center pt-8"> | ||
| <div className="border-l border-[#808080]"></div> | ||
| </div> | ||
|
|
||
| {/*right side input */} | ||
| <div className="col-span-4 flex flex-col h-full"> | ||
| <div className="mb-4"> | ||
| <p className="mb-2">E-mail</p> | ||
| <input | ||
| type="text" | ||
| readOnly | ||
| className="w-full px-4 py-2 rounded-lg bg-[#F2F4F8] border-b-2 border-[#C1C7CD] outline-none" | ||
| placeholder="[email protected]" | ||
| /> | ||
| </div> | ||
| <div className="mb-4 flex flex-col h-full"> | ||
| <p className="mb-2">Bio</p> | ||
| <textarea | ||
| readOnly | ||
| className="w-full flex-grow px-4 py-2 rounded-lg bg-[#F2F4F8] border-b-2 border-[#C1C7CD] outline-none" | ||
| placeholder="Chef Tong is a seasoned culinary expert specializing in the art of steak-making. With his unique techniques and high-quality ingredients, he brings out the best flavors in every cut. Join him to discover the secrets behind achieving a perfectly cooked" | ||
| /> | ||
| </div> | ||
| <p className="mb-2">My Courses</p> | ||
| <div className="mb-4 flex justify-around"> | ||
| <div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 13 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Courses | ||
| </p> | ||
| </div> | ||
| <div> | ||
| <p className="text-[#F9644A] text-3xl font-bold flex justify-center"> | ||
| 7 | ||
| </p> | ||
| <p className="text-[#919295] flex justify-center"> | ||
| Workshops | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </form> |
There was a problem hiding this comment.
Add loading states and error handling for profile data
The component displays hardcoded profile data without handling loading or error states.
Implement data fetching and state handling:
interface ProfileData {
firstName: string;
lastName: string;
email: string;
// ... other fields
}
function ChefProfile() {
const [profile, setProfile] = useState<ProfileData | null>(null);
const [isLoading, setIsLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
useEffect(() => {
fetchProfileData();
}, []);
async function fetchProfileData() {
try {
const response = await fetch('/api/profile');
if (!response.ok) throw new Error('Failed to fetch profile');
const data = await response.json();
setProfile(data);
} catch (error) {
setError(error.message);
} finally {
setIsLoading(false);
}
}
if (isLoading) return <LoadingSpinner />;
if (error) return <ErrorMessage message={error} />;
if (!profile) return null;
// Render profile data
}| @@ -6,6 +6,7 @@ export const courses: Course[] = [ | |||
| courseTitle: 'Bistro Bites - French Classics', | |||
| courseCategory: 'French', | |||
| coursePrice: 600, | |||
| ingredientPrice: 20, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review ingredient prices for consistency with course prices
The newly added ingredientPrice property shows some inconsistencies:
- Wide range (10-29) without clear correlation to course complexity
- Some high-priced courses have low ingredient costs (e.g., French Soufflé at 680 with ingredient cost of 24)
- Some similar courses have vastly different ingredient costs
Consider implementing a more systematic pricing model where:
- Ingredient costs are proportional to course prices
- Similar courses in the same category have comparable ingredient costs
- Premium courses justify their higher prices with corresponding ingredient costs
Also applies to: 21-21, 33-33, 45-45, 57-57, 69-69, 81-81, 93-93, 105-105, 117-117, 129-129, 141-141, 153-153, 165-165, 177-177, 189-189, 201-201, 213-213, 225-225, 237-237, 249-249, 261-261, 273-273, 285-285, 297-297, 309-309, 321-321, 333-333, 345-345, 357-357, 369-369, 381-381, 393-393, 405-405, 417-417, 429-429, 441-441, 453-453, 465-465, 477-477, 489-489, 501-501, 513-513, 525-525, 537-537, 549-549, 561-561, 573-573, 585-585, 597-597, 609-609, 621-621, 633-633, 645-645, 657-657, 669-669, 681-681, 693-693, 705-705, 717-717, 729-729, 741-741, 753-753, 765-765, 777-777, 789-789, 801-801, 813-813, 825-825, 837-837, 849-849, 861-861, 873-873, 885-885, 897-897, 909-909, 921-921, 933-933, 945-945, 957-957, 969-969, 981-981, 993-993, 1005-1005
| @@ -710,6 +774,7 @@ export const courses: Course[] = [ | |||
| courseTitle: 'Paris Pastries - Provincial French courseCategory', | |||
| courseCategory: 'French', | |||
| coursePrice: 610, | |||
There was a problem hiding this comment.
Fix typo in course titles
Some course titles contain the word "courseCategory" which appears to be a mistake:
- "Paris Pastries - Provincial French courseCategory"
- "Delhi Delight - Indian courseCategory"
Apply this fix:
- courseTitle: 'Paris Pastries - Provincial French courseCategory',
+ courseTitle: 'Paris Pastries - Provincial French Cuisine',
- courseTitle: 'Delhi Delight - Indian courseCategory',
+ courseTitle: 'Delhi Delight - Indian Cuisine',Also applies to: 848-848
Description
What
Why
Type of change
Please check the relevent option with
x.Checklist:
Local MachineSummary by CodeRabbit
Release Notes
New Features
Footercomponent for enhanced navigation.ChefProfilecomponent for displaying chef details.EditChefProfileandClientProfilecomponents for editing and viewing user profiles.VideoPaymentandVideoThankYouPagecomponents for handling video payments.Bug Fixes
Enhancements
Chores
package.jsonto include new libraries for improved functionality.