-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Website: Code enhancement and Research Page UI improvements #2374
Conversation
📝 WalkthroughWalkthroughThis pull request represents a comprehensive refactoring of the website's project structure, focusing on reorganizing components, implementing custom data fetching hooks, and standardizing import paths. The changes primarily involve moving components from various sections to a centralized Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Views as Views Directory
participant Hooks as Custom Hooks
participant API as API Service
Dev->>Views: Reorganize Components
Dev->>Hooks: Create SWR-based Hooks
Hooks->>API: Fetch Data
API-->>Hooks: Return Data
Hooks-->>Views: Provide Managed State
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🔭 Outside diff range comments (6)
website2/src/app/clean-air-forum/program-committee/page.tsx (1)
Line range hint
62-69
: Add proper TypeScript interface for person object.Using
any
type reduces type safety. Consider creating a proper interface:interface CommitteeMember { id: string; category: string; // Add other required properties }Then update the code:
- {displayedMembers?.map((person: any) => ( + {displayedMembers?.map((person: CommitteeMember) => (website2/src/app/partners/[id]/page.tsx (1)
Line range hint
106-110
: Add type safety for partner descriptionsThe mapping of descriptions needs proper typing.
interface PartnerDescription { id: string; description: string; } // Then in the map: partner.descriptions.map((desc: PartnerDescription) => ...)website2/src/app/clean-air-forum/schedule/page.tsx (2)
Line range hint
44-60
: Add type safety to session itemsThe mapping of sessions needs proper typing and error handling.
interface Session { start_time: string; session_title: string; session_details: string; } {sessions?.map((item: Session, index: number) => ...)}
Line range hint
96-103
: Enhance type safety for program mappingSimilar to sessions, the program mapping needs proper typing.
interface Program { id: string; title: string; sub_text: string; sessions: Session[]; } {data?.programs?.map((program: Program) => ...)}website2/src/views/careers/CareerPage.tsx (1)
Line range hint
38-54
: Add TypeScript interfaces for career and department dataThe filtering and grouping logic uses
any
types. Consider adding proper interfaces:interface Career { id: string; title: string; department: string; closing_date: string; type: string; } interface Department { id: string; name: string; } const filteredJobs = careers?.filter((career: Career) => { // ... rest of the code });website2/src/views/events/EventPage.tsx (1)
Line range hint
132-141
: Improve image accessibility and performance.The image implementation could benefit from better accessibility and performance optimizations.
Consider these improvements:
<Image src={firstFeaturedEvent.event_image_url} alt={firstFeaturedEvent.title} width={800} height={600} - layout="responsive" - objectFit="cover" + sizes="(max-width: 768px) 100vw, 800px" + style={{ + maxWidth: '100%', + height: 'auto', + objectFit: 'cover', + }} priority + loading="eager" + onError={(e) => { + e.currentTarget.src = '/fallback-image.jpg'; + }} className="rounded-lg shadow-md w-full max-h-[400px] max-w-[500px] object-cover h-auto transition-transform duration-500 ease-in-out transform lg:hover:scale-110 cursor-pointer" />
🧹 Nitpick comments (41)
website2/src/views/careers/DetailsPage.tsx (1)
Line range hint
15-45
: Consider extracting the skeleton loader into a reusable component.The loading skeleton is well-implemented with proper animations and structure. However, it might be beneficial to move it into a separate component like
CareerDetailsSkeleton
for better maintainability and reuse across similar detail pages.website2/src/views/cleanairforum/resources/ResourcePage.tsx (6)
17-21
: Enhance error handling with retry mechanismThe current error handling is basic. Consider implementing a retry mechanism and providing more detailed error feedback to users.
const { cleanAirResources, isLoading, isError, mutate } = useCleanAirResources(); // Add retry button in error state const handleRetry = () => { mutate(); // Re-fetch data };
33-49
: Optimize category mapping and improve type safetyThe category mapping is hardcoded within the component and uses
any
type. Consider:
- Moving the category map to a constant
- Using proper TypeScript types for the filter callback
const CATEGORY_MAP: Record<string, string> = { Toolkits: 'toolkit', 'Technical Reports': 'technical_report', 'Workshop Reports': 'workshop_report', 'Research Publications': 'research_publication', }; return cleanAirResources?.filter( (resource: Resource) => resource.resource_category === CATEGORY_MAP[selectedCategory], ) || [];
51-60
: Sync pagination state with URL parametersConsider syncing the pagination and filter state with URL parameters. This would enable:
- Shareable filtered/paginated views
- Browser back/forward navigation support
- Preserved state on page refresh
import { useRouter, useSearchParams } from 'next/navigation'; // Use URL parameters for state const searchParams = useSearchParams(); const router = useRouter(); const page = searchParams.get('page') || '1'; const category = searchParams.get('category') || 'All';
63-78
: Extract loading skeleton and use theme variablesConsider:
- Moving the LoadingSkeleton to a separate component
- Using theme variables for colors instead of hardcoded values
- Making the skeleton match the actual content structure more closely
// In a separate file: LoadingSkeleton.tsx import { cn } from '@/lib/utils'; export const LoadingSkeleton = ({ itemsPerPage }: { itemsPerPage: number }) => ( <div className="space-y-6"> {Array.from({ length: itemsPerPage }).map((_, index) => ( <div key={index} className={cn( "bg-card rounded-lg p-5 lg:p-8 animate-pulse", "border border-border" )} > {/* skeleton content */} </div> ))} </div> );
156-170
: Improve external link handlingThe direct
window.open
calls could be improved for better security and user experience:
- Add error handling for failed opens
- Consider using a custom hook for external links
- Add loading state for downloads
const useExternalLink = () => { const openLink = (url: string) => { try { window.open(url, '_blank', 'noopener,noreferrer'); } catch (error) { console.error('Failed to open link:', error); // Show user feedback } }; return { openLink }; };Also applies to: 173-186
134-191
: Simplify conditional rendering logicThe nested conditional rendering could be simplified using early returns or a more declarative approach.
const renderContent = () => { if (isLoading) return <LoadingSkeleton />; if (isError) return <NoData message="Failed to load resources. Please try again later." />; if (paginatedResources.length === 0) return <NoData />; return ( <div className="space-y-6"> {/* Resource cards */} </div> ); };website2/src/app/(about)/events/[id]/page.tsx (1)
Line range hint
3-11
: Consider adding proper type definition for route params.The
params: any
type annotation could be more specific to improve type safety.-const page = ({ params }: { params: any }) => { +const page = ({ params }: { params: { id: string } }) => {website2/src/components/sections/solutions/HeroSection.tsx (2)
8-16
: Add proper typing for animation variants.The interface uses
any
for variant types, which defeats TypeScript's type safety benefits.- containerVariants: any; - itemVariants: any; + containerVariants: VariantProps; + itemVariants: VariantProps; +interface VariantProps { + hidden: object; + visible: object; +}
18-44
: Consider extracting configuration values to constants.The component looks well-structured, but has hardcoded values that could be configuration constants.
+const DEFAULT_BG_COLOR = 'bg-[#E9F7EF]'; +const DEFAULT_VIEWPORT_CONFIG = { once: true, amount: 0.2 }; const HeroSection: React.FC<HeroSectionProps> = ({ - bgColor = 'bg-[#E9F7EF]', + bgColor = DEFAULT_BG_COLOR, ... }) => {website2/src/app/clean-air-forum/layout.tsx (1)
19-23
: Consider adding error handling for useForumEvents hook.While the implementation of SWR through useForumEvents is good, consider handling potential error states:
- const { forumEvents } = useForumEvents(); + const { forumEvents, error } = useForumEvents(); + + if (error) { + console.error('Failed to fetch forum events:', error); + // Consider showing an error UI component + }website2/tailwind.config.ts (1)
79-79
: Consider migrating from require to ES modules.The ESLint disable comment for require imports suggests a technical debt. Consider updating to ES modules syntax:
- // eslint-disable-next-line @typescript-eslint/no-require-imports - plugins: [require('tailwindcss-animate')], + plugins: [(await import('tailwindcss-animate')).default],website2/src/app/clean-air-forum/program-committee/page.tsx (1)
27-27
: Remove unnecessary optional chaining on Math object.The Math object is a built-in global object that's always defined, making the optional chaining unnecessary:
- () => Math?.ceil(committeeMembers?.length / membersPerPage), + () => Math.ceil(committeeMembers?.length / membersPerPage),website2/src/app/clean-air-forum/speakers/page.tsx (3)
21-25
: Consider using TypeScript interfaces for person data structureThe filtering logic looks good, but using
any
type for the person object reduces type safety. Consider defining an interface for the person object structure.interface Person { id: string; category: string; // add other relevant fields }Also applies to: 27-31
34-39
: Enhance null safety in pagination calculationsWhile the optional chaining is good, consider adding fallback values to prevent potential issues with undefined arrays.
-const totalKeyNotePages = Math.ceil(KeyNoteSpeakers?.length / membersPerPage); +const totalKeyNotePages = Math.ceil((KeyNoteSpeakers?.length || 0) / membersPerPage);Also applies to: 43-46
Line range hint
72-78
: Add error boundaries for image renderingThe MemberCard components look good, but consider adding error boundaries to handle potential image loading failures gracefully.
Also applies to: 98-104
website2/src/app/partners/[id]/page.tsx (2)
12-13
: Consider using zod for runtime type validationThe type assertion for params looks good, but consider using zod for runtime validation of the id parameter.
import { z } from 'zod'; const paramsSchema = z.object({ id: z.string() }); const { id } = paramsSchema.parse(params);
Line range hint
35-45
: Enhance error handling with specific error messagesThe error handling looks clean, but consider displaying more specific error messages based on the error type.
-<p className="text-red-500 text-xl">Failed to load partner details.</p> +<p className="text-red-500 text-xl"> + {isError instanceof Error ? isError.message : 'Failed to load partner details.'} +</p>website2/src/views/press/PressPage.tsx (1)
78-81
: Add error handling for image loadingConsider adding error handling for the Image component.
<Image src={article.publisher_logo_url || '/default-logo.png'} alt="logo" width={100} height={30} className="object-contain mix-blend-multiply" + onError={(e) => { + e.currentTarget.src = '/default-logo.png'; + }} />website2/src/views/solutions/AfricanCities/AfricanCities.tsx (1)
11-20
: Consider improving type safety and code organizationWhile the logic works, consider these improvements:
- Add proper TypeScript interfaces for the country and city objects
- Extract the default selection logic into a separate function for better readability
+interface City { + id: string; + city_name: string; + // add other properties as needed +} + +interface Country { + id: string; + country_name: string; + city: City[]; + country_flag_url: string; +} + +const getDefaultSelections = (countries: Country[]) => { + if (!countries.length) return { country: null, city: null }; + const country = countries[0]; + const city = country.city?.length ? country.city[0] : null; + return { country, city }; +}; + useEffect(() => { - if (africanCountries && africanCountries.length > 0) { - const defaultCountry = africanCountries[0]; - setSelectedCountry(defaultCountry); - if (defaultCountry.city && defaultCountry.city.length > 0) { - setSelectedCity(defaultCountry.city[0]); - } + if (africanCountries?.length) { + const { country, city } = getDefaultSelections(africanCountries); + setSelectedCountry(country); + setSelectedCity(city); } }, [africanCountries]);website2/src/app/clean-air-forum/resources/page.tsx (1)
38-38
: Good use of optional chaining, consider adding TypeScript interfacesThe addition of optional chaining operators improves code safety. Consider defining TypeScript interfaces for the data structures to make the code more maintainable.
interface ResourceFile { id: string; resource_summary: string; file_url: string; } interface Session { id: string; session_title: string; resource_files?: ResourceFile[]; } interface Resource { id: string; resource_title: string; resource_sessions?: Session[]; }Also applies to: 133-133, 141-141
website2/src/views/home/FeaturedCarousel.tsx (2)
13-23
: Consider simplifying the slide navigation logicThe navigation logic is correct but could be more concise.
-const nextSlide = () => { - if (highlights?.length > 0) { - setCurrentIndex((prev) => (prev + 1) % highlights.length); - } -}; - -const prevSlide = () => { - if (highlights?.length > 0) { - setCurrentIndex( - (prev) => (prev - 1 + highlights.length) % highlights.length, - ); - } -}; +const updateSlide = (direction: 1 | -1) => { + if (highlights?.length > 0) { + setCurrentIndex((prev) => + (prev + direction + highlights.length) % highlights.length + ); + } +}; + +const nextSlide = () => updateSlide(1); +const prevSlide = () => updateSlide(-1);
44-56
: Consider adding a retry mechanism for error statesWhile the error handling is clear, consider adding a retry button to improve user experience.
if (isError) { return ( - <div className="flex justify-center items-center h-64"> - <p className="text-red-500"> - Failed to load highlights. Please try again later. - </p> + <div className="flex flex-col justify-center items-center h-64 space-y-4"> + <p className="text-red-500">Failed to load highlights.</p> + <button + onClick={() => mutate()} // Add mutate from SWR + className="px-4 py-2 text-sm text-blue-600 border border-blue-600 rounded hover:bg-blue-50" + > + Try Again + </button> </div> ); }website2/src/views/publications/ResourcePage.tsx (1)
34-44
: Add type safety to the filtering logicThe filtering logic is well-optimized with useMemo, but could benefit from proper TypeScript types.
+interface Publication { + id: string; + category: string; + title: string; + authors: string; + link?: string; + link_title?: string; + resource_file_url?: string; +} + const filteredResources = useMemo(() => { if (!publications) return []; - return publications.filter((resource: any) => { + return publications.filter((resource: Publication) => { if (Array.isArray(selectedTab)) { return selectedTab.includes(resource.category); } else { return resource.category === selectedTab; } }); }, [publications, selectedTab]);website2/src/hooks/useApiHooks.tsx (2)
27-39
: Consider implementing error retry and timeout configurationsThe hooks use default SWR options. For better user experience, consider customizing retry behavior and timeouts based on the specific needs of each endpoint.
Example enhancement:
+const pressArticlesOptions = { + ...swrOptions, + errorRetryCount: 3, + dedupingInterval: 60000, // 1 minute +}; export function usePressArticles() { const { data, error, isLoading, mutate } = useSWR( 'pressArticles', getPressArticles, - swrOptions, + pressArticlesOptions, ); // ... rest of the code }Also applies to: 41-53, 55-67, 69-81
1-25
: Consider implementing a custom fetcher with error handlingThe hooks could benefit from a centralized fetcher function that handles common error cases and response parsing.
const fetcher = async <T>(url: string): Promise<T> => { const response = await fetch(url); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } return response.json(); };website2/src/views/careers/CareerPage.tsx (1)
131-133
: Consider implementing retry logic for failed requestsThe error handling is good, but consider adding a retry button for failed requests.
{careersError || departmentsError ? ( <div className="text-center"> <NoData message="Failed to load careers or departments." /> <CustomButton onClick={() => { mutate(); }} className="mt-4"> Retry </CustomButton> </div> ) : null}website2/src/views/cleanairforum/membership/MemberPage.tsx (2)
35-70
: Consider enhancing type safety for partner data.The implementation looks clean and efficient with good use of
useMemo
for performance optimization. However, usingany
type for partners reduces type safety.Consider defining proper interfaces:
interface Partner { id: string; website_category: string; type: string; partner_logo_url?: string; partner_logo?: string; } const categorize = (type: string) => cleanairPartners .filter((partner: Partner) => partner.type.toLowerCase() === type) .map((partner: Partner) => ({ id: partner.id, logoUrl: partner.partner_logo_url || partner.partner_logo || '', }));
Line range hint
105-127
: Consider extracting repeated section structure into a component.The loading state handling and section structure is duplicated across multiple partner sections. This could be made more maintainable by extracting it into a reusable component.
Consider creating a
PartnerSection
component:interface PartnerSectionProps { title: React.ReactNode; description: React.ReactNode; logos: Array<{ id: string; logoUrl: string }>; isLoading: boolean; bgColor?: string; } const PartnerSection: React.FC<PartnerSectionProps> = ({ title, description, logos, isLoading, bgColor }) => ( <section className="w-full"> {isLoading ? ( <SkeletonPaginatedSection /> ) : ( <PaginatedSection title={title} description={description} logos={logos} bgColor={bgColor} /> )} </section> );Also applies to: 131-153, 154-177, 180-210
website2/src/views/events/EventPage.tsx (1)
69-78
: Enhance error feedback for better user experience.While the error handling is good, the error message could be more specific and provide users with possible actions.
Consider enhancing the error message:
- <h1 className="text-4xl font-bold mb-4">Error Loading Events</h1> - <p className="text-lg text-gray-600"> - We couldn't fetch the events at this time. Please try again - later. - </p> + <h1 className="text-4xl font-bold mb-4">Unable to Load Events</h1> + <p className="text-lg text-gray-600"> + We encountered an issue while fetching the events. This might be due + to network connectivity or server availability. + </p> + <button + onClick={() => window.location.reload()} + className="mt-4 text-blue-600 hover:text-blue-700" + > + Click here to refresh the page + </button>website2/src/views/cleanairforum/events/EventsPage.tsx (3)
36-44
: Consider moving categories to a configuration file.Hardcoding categories in the component makes maintenance more difficult and violates separation of concerns.
Consider moving categories to a separate configuration file:
// src/config/eventCategories.ts export const EVENT_CATEGORIES = [ { name: 'All Categories', value: '' }, { name: 'Webinar', value: 'webinar' }, // ... other categories ] as const; export type EventCategory = typeof EVENT_CATEGORIES[number]['value'];This would make it easier to maintain and reuse categories across the application.
59-79
: Enhance type safety in filtering logic.While the filtering logic is well-optimized with
useMemo
, it could benefit from stronger typing.Consider adding proper types:
interface Event { id: string; start_date: string; event_category: string; } const filteredEvents = useMemo<Event[]>(() => { if (!cleanAirEvents) return []; let events = [...cleanAirEvents]; if (selectedMonth) { events = events.filter((event: Event) => { const eventDate = new Date(event.start_date); return format(eventDate, 'MMMM') === selectedMonth; }); } if (selectedCategory) { events = events.filter( (event: Event) => event.event_category === selectedCategory, ); } return events; }, [cleanAirEvents, selectedMonth, selectedCategory]);
Line range hint
271-278
: Enhance pagination accessibility.The pagination implementation could benefit from improved accessibility features.
Consider adding ARIA labels and keyboard navigation:
<Pagination totalPages={totalPastPages} currentPage={currentPastPage} onPageChange={setCurrentPastPage} + aria-label="Past events navigation" + role="navigation" />website2/src/views/home/HomeStatsSection.tsx (1)
130-136
: Well-implemented error handling.Good addition of error handling with a user-friendly error message. However, consider providing more specific error information to help users understand what went wrong.
- <div className="text-red-500 text-lg text-center"> - Something went wrong - </div> + <div className="text-red-500 text-lg text-center space-y-2"> + <p>Failed to load impact numbers</p> + <p className="text-sm">Please try refreshing the page</p> + </div>website2/src/views/events/SingleEvent.tsx (1)
52-58
: Enhanced error handling with user feedback.The error handling implementation is good but could be more informative. Consider adding a retry mechanism for better user experience.
- <div className="w-full flex items-center justify-center py-20"> - <p className="text-red-500 text-lg"> - Failed to load event details. Please try again later. - </p> - </div> + <div className="w-full flex flex-col items-center justify-center py-20 space-y-4"> + <p className="text-red-500 text-lg"> + Failed to load event details. Please try again later. + </p> + <button + onClick={() => window.location.reload()} + className="text-blue-600 hover:text-blue-800 underline" + > + Retry + </button> + </div>website2/src/views/about/AboutPage.tsx (1)
388-393
: Improved partner logo rendering with proper typing.The partner logo mapping is now more type-safe and maintainable. Consider adding a type definition for the partner object to further improve type safety.
type Partner = { id: string; partner_logo_url: string; }; // Then use it in the map function partners.map((partner: Partner) => ({ id: partner.id, logoUrl: partner.partner_logo_url || '', }))website2/package.json (1)
43-43
: Consider pinning the SWR versionWhile ^2.3.0 is stable, consider pinning to an exact version (2.3.0) to ensure consistent builds across environments.
- "swr": "^2.3.0", + "swr": "2.3.0",website2/src/views/solutions/research/ResearchPage.tsx (4)
1-13
: Consider organizing importsThe imports are mixed between third-party and local imports. Consider grouping them:
- React and next imports
- Third-party libraries
- Local components and utilities
'use client'; +// React and Next.js import React from 'react'; import Image from 'next/image'; import { useRouter } from 'next/navigation'; +// Third-party libraries import { motion } from 'framer-motion'; import { BarChart3, Shield, Users, Zap } from 'lucide-react'; +// Local imports import HeroSection from '@/components/sections/solutions/HeroSection'; import { CustomButton, Divider } from '@/components/ui'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { cn } from '@/lib/utils';
46-91
: Consider moving CONTENT to a separate fileThe CONTENT object is quite large and could be moved to a separate constants file for better maintainability.
321-331
: Consider extracting the fellowship application URLThe Google Forms URL should be moved to the CONTENT constant or environment variables for easier maintenance.
374-383
: Consider making the CTA button more accessibleThe call-to-action button could benefit from better accessibility:
- Add aria-label
- Ensure proper contrast ratio for the text color
<CustomButton onClick={() => router.push('/explore-data')} + aria-label="Explore our digital air quality tools" className="bg-[#0CE87E] text-black rounded-lg max-w-5xl mx-auto w-full py-16 text-center" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
website2/package-lock.json
is excluded by!**/package-lock.json
website2/src/app/favicon.ico
is excluded by!**/*.ico
📒 Files selected for processing (61)
website2/README.md
(1 hunks)website2/package.json
(2 hunks)website2/src/app/(about)/about-us/page.tsx
(1 hunks)website2/src/app/(about)/careers/[id]/page.tsx
(1 hunks)website2/src/app/(about)/careers/page.tsx
(1 hunks)website2/src/app/(about)/events/[id]/page.tsx
(1 hunks)website2/src/app/(about)/events/page.tsx
(1 hunks)website2/src/app/(about)/press/page.tsx
(1 hunks)website2/src/app/(about)/resources/page.tsx
(1 hunks)website2/src/app/clean-air-forum/layout.tsx
(1 hunks)website2/src/app/clean-air-forum/page.tsx
(1 hunks)website2/src/app/clean-air-forum/partners/page.tsx
(1 hunks)website2/src/app/clean-air-forum/program-committee/page.tsx
(2 hunks)website2/src/app/clean-air-forum/resources/page.tsx
(2 hunks)website2/src/app/clean-air-forum/schedule/page.tsx
(2 hunks)website2/src/app/clean-air-forum/speakers/page.tsx
(3 hunks)website2/src/app/clean-air-network/CleanAirPage.tsx
(1 hunks)website2/src/app/clean-air-network/event-details/page.tsx
(0 hunks)website2/src/app/clean-air-network/events/[id]/page.tsx
(1 hunks)website2/src/app/clean-air-network/events/page.tsx
(1 hunks)website2/src/app/clean-air-network/membership/page.tsx
(1 hunks)website2/src/app/clean-air-network/resources/ResourcePage.tsx
(0 hunks)website2/src/app/clean-air-network/resources/page.tsx
(1 hunks)website2/src/app/home/page.tsx
(1 hunks)website2/src/app/layout.tsx
(2 hunks)website2/src/app/legal/airqo-data/page.tsx
(1 hunks)website2/src/app/legal/layout.tsx
(1 hunks)website2/src/app/legal/payment-refund-policy/page.tsx
(1 hunks)website2/src/app/legal/privacy-policy/page.tsx
(1 hunks)website2/src/app/legal/terms-of-service/page.tsx
(1 hunks)website2/src/app/partners/[id]/page.tsx
(4 hunks)website2/src/app/products/mobile-app/MobilePage.tsx
(1 hunks)website2/src/app/solutions/african-cities/page.tsx
(1 hunks)website2/src/app/solutions/communities/page.tsx
(1 hunks)website2/src/app/solutions/research/ResearchPage.tsx
(0 hunks)website2/src/app/solutions/research/page.tsx
(1 hunks)website2/src/components/layouts/Navbar.tsx
(1 hunks)website2/src/components/sections/solutions/HeroSection.tsx
(1 hunks)website2/src/hooks/swrConfig.ts
(1 hunks)website2/src/hooks/useApiHooks.tsx
(1 hunks)website2/src/views/about/AboutPage.tsx
(3 hunks)website2/src/views/careers/CareerPage.tsx
(5 hunks)website2/src/views/careers/DetailsPage.tsx
(4 hunks)website2/src/views/cleanairforum/PaginatedSection.tsx
(1 hunks)website2/src/views/cleanairforum/events/EventsPage.tsx
(8 hunks)website2/src/views/cleanairforum/membership/MemberPage.tsx
(7 hunks)website2/src/views/cleanairforum/resources/ResourcePage.tsx
(1 hunks)website2/src/views/events/EventPage.tsx
(4 hunks)website2/src/views/events/SingleEvent.tsx
(10 hunks)website2/src/views/home/FeaturedCarousel.tsx
(3 hunks)website2/src/views/home/HomePage.tsx
(1 hunks)website2/src/views/home/HomePlayerSection.tsx
(2 hunks)website2/src/views/home/HomeStatsSection.tsx
(4 hunks)website2/src/views/press/PressPage.tsx
(2 hunks)website2/src/views/publications/ResourcePage.tsx
(4 hunks)website2/src/views/solutions/AfricanCities/AfricanCities.tsx
(2 hunks)website2/src/views/solutions/AfricanCities/AfricanCityPage.tsx
(2 hunks)website2/src/views/solutions/communities/CommunityPage.tsx
(2 hunks)website2/src/views/solutions/research/ResearchPage.tsx
(1 hunks)website2/tailwind.config.ts
(2 hunks)website2/tsconfig.json
(1 hunks)
💤 Files with no reviewable changes (3)
- website2/src/app/clean-air-network/event-details/page.tsx
- website2/src/app/solutions/research/ResearchPage.tsx
- website2/src/app/clean-air-network/resources/ResourcePage.tsx
✅ Files skipped from review due to trivial changes (25)
- website2/src/app/clean-air-network/events/[id]/page.tsx
- website2/src/app/legal/privacy-policy/page.tsx
- website2/src/app/(about)/events/page.tsx
- website2/src/app/legal/payment-refund-policy/page.tsx
- website2/src/app/solutions/research/page.tsx
- website2/src/app/clean-air-forum/page.tsx
- website2/src/app/legal/airqo-data/page.tsx
- website2/src/views/cleanairforum/PaginatedSection.tsx
- website2/src/app/(about)/press/page.tsx
- website2/src/app/(about)/careers/page.tsx
- website2/src/app/solutions/african-cities/page.tsx
- website2/src/app/(about)/resources/page.tsx
- website2/src/app/clean-air-network/membership/page.tsx
- website2/README.md
- website2/src/app/clean-air-network/resources/page.tsx
- website2/src/app/solutions/communities/page.tsx
- website2/src/app/clean-air-network/events/page.tsx
- website2/src/app/clean-air-network/CleanAirPage.tsx
- website2/src/app/legal/terms-of-service/page.tsx
- website2/src/app/products/mobile-app/MobilePage.tsx
- website2/src/app/home/page.tsx
- website2/src/app/clean-air-forum/partners/page.tsx
- website2/src/views/home/HomePage.tsx
- website2/src/app/(about)/careers/[id]/page.tsx
- website2/src/app/legal/layout.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
website2/src/views/careers/DetailsPage.tsx
[error] 90-90: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-website2-deploy-preview
🔇 Additional comments (24)
website2/src/views/careers/DetailsPage.tsx (2)
11-12
: Clean implementation with proper typing! 👍Good job on using proper TypeScript types for props and abstracting data fetching into a custom hook.
97-107
: 🛠️ Refactor suggestionAdd security attributes for external link.
When opening external URLs, it's important to add proper security attributes to protect against potential security vulnerabilities.
<CustomButton onClick={() => window.open(careerDetails.apply_url, '_blank')} - rel="noopener noreferrer" + target="_blank" + rel="noopener noreferrer" className="bg-green-500 hover:bg-green-600 text-white font-semibold" > Apply Now </CustomButton>Likely invalid or redundant comment.
website2/src/app/(about)/events/[id]/page.tsx (1)
1-1
: LGTM! Import path change aligns with project restructuring.The transition to absolute imports using
@/views
prefix improves maintainability and follows the PR's folder restructuring objectives.website2/src/hooks/swrConfig.ts (1)
1-18
: Verify SWR configuration values for optimal performance.The configuration is well-structured and documented. However, a few considerations:
- The
dedupingInterval
of 60 seconds might be too long for frequently changing data.- With
refreshInterval: 0
, consider if any endpoints need real-time updates.Consider creating separate configurations for different types of data:
export const swrConfigs = { static: { ...swrOptions, dedupingInterval: 60000, }, dynamic: { ...swrOptions, dedupingInterval: 10000, refreshInterval: 30000, } };✅ Verification successful
Current SWR configuration is well-suited for the codebase's needs
The configuration aligns perfectly with the observed usage patterns across 17 API endpoints, primarily serving static content and reference data. The 60-second deduping interval provides a good balance between performance and freshness, while the disabled refresh interval is appropriate since there's no evidence of real-time data requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find SWR hook usages echo "=== SWR Hook Usages ===" ast-grep --pattern 'useSWR($$$)' echo -e "\n=== SWR Configuration Imports ===" rg "import.*swrConfig" -A 2 echo -e "\n=== API Endpoint Usage with SWR ===" rg "useSWR\(['\"].*['\"]" --type ts --type tsx -A 2Length of output: 6585
website2/src/app/layout.tsx (1)
42-51
: Well-implemented loading state with Suspense.Good use of Suspense for handling loading states during maintenance checks. The implementation provides a smooth user experience.
Consider adding a timeout to the loading state for better user feedback in case of prolonged loading:
const LOADING_TIMEOUT = 5000; // 5 seconds // Add this before the Suspense component const [showTimeout, setShowTimeout] = useState(false); useEffect(() => { const timer = setTimeout(() => setShowTimeout(true), LOADING_TIMEOUT); return () => clearTimeout(timer); }, []); // Modify the loading component <Loading showTimeout={showTimeout} />website2/src/app/(about)/about-us/page.tsx (2)
3-3
: LGTM! Import path updated to use absolute imports.The change from relative to absolute imports aligns with the codebase restructuring effort, improving maintainability and clarity.
Line range hint
15-35
: Update placeholder domains in metadata configuration.The metadata configuration contains placeholder URLs (e.g., 'yourdomain.com'). These should be updated to use the actual production domain to ensure proper SEO functionality.
Run this script to check for other placeholder domains:
website2/src/app/clean-air-forum/layout.tsx (1)
26-50
: LGTM! Clean implementation of Suspense boundary.The use of Suspense for loading states is a good practice, providing a better user experience during data fetching.
website2/tailwind.config.ts (1)
8-8
: LGTM! Added views directory to Tailwind content configuration.This change ensures proper scanning of utility classes in the new views directory structure.
website2/src/app/clean-air-forum/program-committee/page.tsx (1)
Line range hint
13-29
: LGTM! Effective use of useMemo for performance optimization.The memoization of filtered committee members and pagination calculations helps prevent unnecessary recalculations.
website2/src/views/solutions/AfricanCities/AfricanCities.tsx (2)
4-4
: Nice refactoring to use custom hook!The switch to
useAfricanCountries
hook improves code organization and follows React best practices for data fetching.Also applies to: 7-7
37-49
: Well-implemented loading and error states!Good job on providing clear feedback to users with:
- Skeleton loading UI for better user experience
- Clear error message when data fetching fails
website2/src/views/home/FeaturedCarousel.tsx (1)
Line range hint
26-42
: Excellent loading state implementation!Great job on implementing a skeleton UI with:
- Appropriate placeholder dimensions
- Smooth animation
- Proper spacing and layout matching the actual content
website2/src/views/home/HomePlayerSection.tsx (1)
13-13
: Import path update and useEffect dependency fixThe addition of
isBackgroundVideoPlaying
to the useEffect dependency array is crucial as this state variable is used within the effect. This fixes a potential bug where the background video playback state might not sync correctly with modal state changes.Also applies to: 53-53
website2/src/views/solutions/AfricanCities/AfricanCityPage.tsx (1)
6-8
: Well-structured component extractionThe extraction of the hero section into a separate component improves reusability and maintainability. The animation variants are properly passed down, maintaining the original functionality while making the code more modular.
Also applies to: 36-44
website2/src/views/careers/CareerPage.tsx (1)
13-22
: Good implementation of custom hooks and loading statesThe refactoring to use custom hooks improves separation of concerns and the combined loading state provides a better user experience.
website2/src/components/layouts/Navbar.tsx (1)
22-22
: LGTM! Import path update aligns with project restructuring.The change follows the broader refactoring pattern of moving components to the views directory for better organization.
website2/src/views/home/HomeStatsSection.tsx (1)
14-14
: Great improvement in data fetching with custom hooks!The transition from direct API calls to using the
useImpactNumbers
hook improves code organization and reusability. The hook-based approach provides better state management for loading and error states.Also applies to: 16-16, 69-69
website2/src/views/events/SingleEvent.tsx (1)
17-17
: Improved type safety with proper prop typing.Good improvement replacing
any
with a specific type for theid
prop. This enhances type safety and code maintainability.Also applies to: 19-19
website2/src/views/solutions/communities/CommunityPage.tsx (1)
41-48
: Excellent component abstraction with HeroSection.The refactoring to use a dedicated
HeroSection
component improves code reusability and maintainability. The props structure is well-thought-out, providing flexibility while maintaining consistency.website2/src/views/about/AboutPage.tsx (1)
11-15
: Well-structured data fetching with custom hooks.Great improvement using custom hooks for data fetching. The separation of concerns is clear, and the loading states are well-managed.
Also applies to: 37-41
website2/package.json (1)
10-10
: Good addition of the lint:fix script!This will help automate fixing of linting issues and maintain code consistency.
website2/tsconfig.json (1)
28-29
: LGTM! Good path alias additionThe @/views/* path alias follows the existing pattern and supports the codebase restructuring effort.
website2/src/views/solutions/research/ResearchPage.tsx (1)
1-388
: Overall implementation looks solid!The Research page implementation is well-structured with good use of:
- Framer Motion for animations
- Responsive design
- Component composition
- Error boundaries (from CustomButton)
A few minor suggestions above, but overall great work! 👍
if (isError) { | ||
return ( | ||
<div className="text-center text-xl text-red-500 p-8"> | ||
Unable to load career details. Please try again later. | ||
</div> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling with retry mechanism and logging.
Consider these improvements:
- Add a retry button for failed requests
- Implement error logging for debugging
- Display more specific error messages when possible
Example implementation:
if (isError) {
return (
- <div className="text-center text-xl text-red-500 p-8">
- Unable to load career details. Please try again later.
+ <div className="text-center p-8">
+ <p className="text-xl text-red-500 mb-4">
+ Unable to load career details. Please try again later.
+ </p>
+ <CustomButton
+ onClick={() => mutate()} // SWR's mutate function to retry
+ className="bg-gray-500 hover:bg-gray-600 text-white"
+ >
+ Retry
+ </CustomButton>
</div>
);
}
Committable suggestion skipped: line range outside the PR's diff.
dangerouslySetInnerHTML={{ | ||
__html: careerDetails.career_details || 'No details available.', | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Sanitize HTML content to prevent XSS attacks.
The use of dangerouslySetInnerHTML
without sanitization poses a significant security risk. Career details from the API could contain malicious scripts.
Implement one of these solutions:
- Use a HTML sanitizer like DOMPurify:
+ import DOMPurify from 'dompurify';
<div
className="prose prose-lg max-w-none text-gray-700 leading-relaxed"
dangerouslySetInnerHTML={{
- __html: careerDetails.career_details || 'No details available.',
+ __html: DOMPurify.sanitize(careerDetails.career_details || 'No details available.'),
}}
/>
- Or better yet, use a Markdown renderer if the content is in Markdown format:
+ import ReactMarkdown from 'react-markdown';
- <div
- className="prose prose-lg max-w-none text-gray-700 leading-relaxed"
- dangerouslySetInnerHTML={{
- __html: careerDetails.career_details || 'No details available.',
- }}
- />
+ <ReactMarkdown className="prose prose-lg max-w-none text-gray-700 leading-relaxed">
+ {careerDetails.career_details || 'No details available.'}
+ </ReactMarkdown>
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
'use client'; | ||
|
||
import Image from 'next/image'; | ||
import React, { useMemo, useState } from 'react'; | ||
|
||
import { | ||
CustomButton, | ||
DropdownMenu, | ||
DropdownMenuContent, | ||
DropdownMenuItem, | ||
DropdownMenuTrigger, | ||
NoData, | ||
Pagination, | ||
} from '@/components/ui'; | ||
import { useCleanAirResources } from '@/hooks/useApiHooks'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add TypeScript interfaces for better type safety
Consider defining proper interfaces for the resource type instead of using any
. This will improve type safety and make the code more maintainable.
interface Resource {
resource_title: string;
resource_category: string;
resource_authors: string;
resource_link?: string;
resource_file?: string;
}
<Image | ||
src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728132390/website/cleanAirForum/images/resource_apztbz.webp" | ||
alt="Air Quality Management Banner" | ||
width={800} | ||
height={400} | ||
className="rounded-lg object-contain w-full" | ||
/> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Move image URL to environment configuration
The Cloudinary URL should not be hardcoded. Consider moving it to environment configuration or a constants file.
// In env.ts or constants.ts
export const IMAGES = {
resourceBanner: process.env.NEXT_PUBLIC_RESOURCE_BANNER_URL || 'fallback_url'
};
{displayedArticles.map((article: any) => ( | ||
<div | ||
key={article.id} | ||
className={`p-8 lg:px-16 lg:py-12 space-y-6 rounded-lg shadow-sm transition-shadow hover:shadow-md bg-card-custom-gradient ${ | ||
idx === itemsPerPage ? 'lg:col-span-2' : '' | ||
}`} | ||
className={`p-8 lg:px-16 lg:py-12 space-y-6 rounded-lg shadow-sm transition-shadow hover:shadow-md bg-card-custom-gradient`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add type safety for press articles
The article mapping needs proper typing.
interface PressArticle {
id: string;
publisher_logo_url: string;
date_published: string;
article_title: string;
article_intro: string;
article_link: string;
}
{displayedArticles.map((article: PressArticle) => ...)}
className={`grid grid-cols-$ | ||
{selectedCity.content[0].image.length === 2 ? '2' : '1'} gap-4 items-center`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix template literal syntax in className
There's a syntax error in the template literal that would break the grid layout.
-className={`grid grid-cols-$
- {selectedCity.content[0].image.length === 2 ? '2' : '1'} gap-4 items-center`}
+className={`grid grid-cols-${
+ selectedCity.content[0].image.length === 2 ? '2' : '1'
+} gap-4 items-center`}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
className={`grid grid-cols-$ | |
{selectedCity.content[0].image.length === 2 ? '2' : '1'} gap-4 items-center`} | |
className={`grid grid-cols-${ | |
selectedCity.content[0].image.length === 2 ? '2' : '1' | |
} gap-4 items-center`} |
) : ( | ||
<div className="space-y-6"> | ||
{displayedResources.map((resource, idx) => ( | ||
{displayedResources.map((resource: any, idx: any) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type safety and key prop usage
Two issues need attention:
- Using
any
type for resource and index - Using array index as key (not recommended if items can be reordered)
-{displayedResources.map((resource: any, idx: any) => (
+{displayedResources.map((resource: Publication) => (
<div
- key={idx}
+ key={resource.id}
className="bg-card-custom-gradient p-6 lg:p-16 rounded-lg"
>
Committable suggestion skipped: line range outside the PR's diff.
export function useEventDetails(id: any) { | ||
const { data, error, isLoading, mutate } = useSWR( | ||
id ? `eventDetails/${id}` : null, | ||
() => getEventDetails(id), | ||
swrOptions, | ||
); | ||
return { | ||
eventDetails: data, | ||
isLoading, | ||
isError: error, | ||
mutate, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add proper TypeScript types for hook parameters and return values
The hooks using id: any
parameter type should be properly typed. Consider creating interfaces for the data structures and using them consistently across hooks.
Example improvement for one hook:
-export function useEventDetails(id: any) {
+interface EventDetails {
+ id: string;
+ // add other event properties
+}
+
+export function useEventDetails(id: string | null) {
const { data, error, isLoading, mutate } = useSWR(
id ? `eventDetails/${id}` : null,
() => getEventDetails(id),
swrOptions,
);
return {
- eventDetails: data,
+ eventDetails: data as EventDetails | null,
isLoading,
- isError: error,
+ isError: error as Error | null,
mutate,
};
}
Also applies to: 125-137, 223-235, 251-263
const MotionSection = ({ className = '', children, ...props }: any) => ( | ||
<motion.section | ||
className={`max-w-5xl mx-auto px-4 ${className}`} | ||
initial="hidden" | ||
whileInView="visible" | ||
viewport={{ once: true, amount: 0.2 }} | ||
variants={animations.container} | ||
{...props} | ||
> | ||
{children} | ||
</motion.section> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add proper TypeScript typing for MotionSection props
Replace any
with proper TypeScript interface for better type safety.
-const MotionSection = ({ className = '', children, ...props }: any) => (
+interface MotionSectionProps extends React.PropsWithChildren {
+ className?: string;
+ [key: string]: any;
+}
+
+const MotionSection = ({ className = '', children, ...props }: MotionSectionProps) => (
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const MotionSection = ({ className = '', children, ...props }: any) => ( | |
<motion.section | |
className={`max-w-5xl mx-auto px-4 ${className}`} | |
initial="hidden" | |
whileInView="visible" | |
viewport={{ once: true, amount: 0.2 }} | |
variants={animations.container} | |
{...props} | |
> | |
{children} | |
</motion.section> | |
); | |
interface MotionSectionProps extends React.PropsWithChildren { | |
className?: string; | |
[key: string]: any; | |
} | |
const MotionSection = ({ className = '', children, ...props }: MotionSectionProps) => ( | |
<motion.section | |
className={`max-w-5xl mx-auto px-4 ${className}`} | |
initial="hidden" | |
whileInView="visible" | |
viewport={{ once: true, amount: 0.2 }} | |
variants={animations.container} | |
{...props} | |
> | |
{children} | |
</motion.section> | |
); |
New Website2 changes available for preview here |
Summary of Changes (What does this PR do?)
Research Page Implementation: Developed the Research page with updated and dynamic content based on the latest requirements.
SWR Integration: Integrated SWR for efficient data fetching, caching, and improved user experience, ensuring smoother page interactions.
UI Enhancements: Applied consistent animations and design improvements to align with the project’s overall aesthetics.
Folder Structure Refactor: Organized the folder structure for better scalability and maintainability, adhering to industry best practices.
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
Release Notes
New Features
HeroSection
component for consistent page headersImprovements
Chores