Website: General Maintenance and code base enhancements #2413
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive refactoring of layout configurations across the website, primarily focusing on centralizing styling through a new Changes
Suggested reviewers
Possibly related PRs
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
website2/src/app/legal/layout.tsx (1)
Replace placeholder 'yourdomain.com' URLs across all page metadata
Found placeholder URLs in metadata across multiple pages that need to be updated with the actual production domain for proper SEO and social sharing:
- website2/src/app/page.tsx (home page)
- website2/src/app/clean-air-network/layout.tsx
- website2/src/app/solutions/layout.tsx
- website2/src/app/partners/layout.tsx
- website2/src/app/products/layout.tsx
- website2/src/app/legal/layout.tsx
- website2/src/app/explore-data/layout.tsx
- website2/src/app/contact/layout.tsx
- website2/src/app/(about)/about-us/page.tsx
- website2/src/app/(about)/careers/layout.tsx
- website2/src/app/(about)/resources/page.tsx
- website2/src/app/(about)/press/page.tsx
- website2/src/app/(about)/events/layout.tsx
Each file contains placeholder URLs for page URLs, OG images, and canonical links that need to be updated.
🔗 Analysis chain
Line range hint
19-24: Update placeholder URLs in metadata.The metadata contains placeholder URLs (yourdomain.com) which should be replaced with actual production URLs for proper SEO.
Run this script to check for other placeholder URLs in metadata:
Also applies to: 31-34, 42-44
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for placeholder URLs in metadata configurations rg -g '*.tsx' -g '*.ts' "yourdomain.com"Length of output: 3984
website2/src/views/home/FeaturedCarousel.tsx (1)
Line range hint
61-90: Enhance carousel accessibility.The carousel implementation could benefit from additional accessibility features for better screen reader support.
<div - className="flex transition-transform duration-500 ease-in-out" + className="flex transition-transform duration-500 ease-in-out" + role="region" + aria-roledescription="carousel" + aria-label="Featured highlights" style={{ transform: `translateX(-${currentIndex * 100}%)` }} > {highlights.map((item: any) => ( <div key={item.id} + role="group" + aria-roledescription="slide" className="w-full flex-shrink-0 flex flex-col md:flex-row gap-8 md:gap-16" >
🧹 Nitpick comments (35)
website2/src/utils/string-utils.ts (1)
1-6: Consider handling edge cases in formatStringThe function looks clean and follows TypeScript best practices. However, it might benefit from handling edge cases.
Consider this enhanced implementation:
export const formatString = (category: string): string => { + if (!category) return ''; return category .split('_') + .filter(Boolean) // Handle consecutive underscores .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) .join(' '); };website2/src/views/cleanairforum/resources/ResourceCard.tsx (1)
11-58: Enhance accessibility with ARIA attributesThe component could benefit from improved accessibility support.
Consider adding these attributes:
- <div className="bg-white rounded-lg p-5 lg:p-8 shadow"> + <div + className="bg-white rounded-lg p-5 lg:p-8 shadow" + role="article" + aria-labelledby={`resource-title-${resource.id}`} + > <p className="text-blue-500 text-lg font-semibold mb-2"> {formatString(resource.resource_category)} </p> - <h3 className="text-2xl lg:text-4xl font-semibold mb-2"> + <h3 + id={`resource-title-${resource.id}`} + className="text-2xl lg:text-4xl font-semibold mb-2" + >website2/src/views/cleanairforum/resources/ResourcePage.tsx (3)
21-27: Move categories to configuration fileConsider moving the categories array to a dedicated configuration file for better maintainability.
This would align with the PR's goal of centralizing configurations. Consider creating a new file:
// src/configs/resourceConfigs.ts export const resourceCategories = [ 'All', 'Toolkit', 'Technical Report', 'Workshop Report', 'Research Publication', ] as const; export type ResourceCategory = typeof resourceCategories[number];
Line range hint
62-67: Optimize image loading with proper dimensionsWhile the priority attribute is good for above-the-fold images, consider optimizing the image dimensions.
<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} + width={1200} + height={600} + sizes="(max-width: 768px) 100vw, 1200px" className="rounded-lg object-contain w-full" priority />
101-125: Implement virtualization for better performanceFor better performance with large datasets, consider implementing virtualization.
You could use
react-windoworreact-virtualizedto render only the visible resources. This would be especially beneficial as the resource list grows.Example implementation pattern:
import { FixedSizeList } from 'react-window'; const Row = ({ index, style }: { index: number; style: object }) => ( <div style={style}> <ResourceCard resource={paginatedResources[index]} /> </div> ); // In render: <FixedSizeList height={800} itemCount={paginatedResources.length} itemSize={200} width="100%" > {Row} </FixedSizeList>website2/src/configs/mainConfigs.ts (1)
7-11: Consider environment-specific configuration values.While the configuration centralization is good, consider:
- Making these values environment-specific
- Using a more specific templateName (e.g., 'AirQo Public Website')
website2/src/hooks/useResourceFilter.ts (2)
12-19: Enhance category comparison robustness.The string manipulation for category comparison could be improved:
- Multiple spaces in category names aren't handled
- Consider using a more robust transformation method
- selectedCategory.toLowerCase().replace(' ', '_'), + selectedCategory.toLowerCase().trim().replace(/\s+/g, '_'),
5-41: Consider adding error handling for edge cases.The hook could benefit from:
- Validation for negative itemsPerPage
- Handling empty resource arrays
- Boundary checking for currentPage
website2/src/views/cleanairforum/RegisterBanner.tsx (1)
Line range hint
13-20: Move image URL to configuration.Consider moving the hardcoded Cloudinary URL to the configuration system for better maintainability.
website2/src/views/legal/Tabsection.tsx (1)
14-19: Consider enhancing tab configuration.The tabs array could be moved to a separate configuration file for better maintainability.
-const tabs: Tab[] = [ - { label: 'Terms of Service', value: 'terms-of-service' }, - { label: 'Privacy Policy', value: 'privacy-policy' }, - { label: 'AirQo Data', value: 'airqo-data' }, - { label: 'Payment Terms & Refund Policy', value: 'payment-refund-policy' }, -];Create a new file
configs/tabConfigs.ts:export const legalTabs: Tab[] = [ { label: 'Terms of Service', value: 'terms-of-service' }, { label: 'Privacy Policy', value: 'privacy-policy' }, { label: 'AirQo Data', value: 'airqo-data' }, { label: 'Payment Terms & Refund Policy', value: 'payment-refund-policy' }, ];website2/src/app/clean-air-forum/layout.tsx (1)
32-55: Well-structured semantic HTML layout.Good use of semantic HTML elements (header, main, footer) and proper integration of mainConfig for container styling.
Consider adding aria-labels for better accessibility:
-<header className="sticky top-0 z-50"> +<header className="sticky top-0 z-50" aria-label="Main navigation">website2/src/components/layouts/NotificationBanner.tsx (2)
26-44: Consider accessibility improvements for the notification banner.While the implementation is good, consider adding ARIA attributes for better accessibility.
-<Link href={CLEAN_AIR_NETWORK_ROUTE}> +<Link + href={CLEAN_AIR_NETWORK_ROUTE} + aria-label="Join the CLEAN-AIR Network" +>
31-37: Consider performance impact of infinite animation.The infinite animation might impact performance on low-end devices. Consider adding a
prefers-reduced-motionmedia query check.const shouldAnimate = typeof window !== 'undefined' ? !window.matchMedia('(prefers-reduced-motion: reduce)').matches : false; <motion.span className="text-blue-600 hover:text-blue-800 flex items-center" animate={shouldAnimate ? { x: [0, 4, 0] } : {}} transition={shouldAnimate ? { duration: 1.5, repeat: Number.POSITIVE_INFINITY } : {}} >website2/src/components/layouts/ActionButtons2.tsx (1)
Line range hint
15-19: Consider moving external URLs to configuration.The hardcoded Google Form URLs should be moved to a configuration file for better maintainability and easier updates.
Example implementation:
+ import { externalLinks } from '@/configs/links'; // ... - window.open('https://docs.google.com/forms/d/e/1FAIpQLScIPz7VrhfO2ifMI0dPWIQRiGQ9y30LoKUCT-DDyorS7sAKUA/viewform'); + window.open(externalLinks.joinNetworkForm); // ... - window.open('https://docs.google.com/forms/d/14jKDs2uCtMy2a_hzyCiJnu9i0GbxITX_DJxVB4GGP5c/edit'); + window.open(externalLinks.registerEventForm);Also applies to: 36-40
website2/src/components/layouts/Highlight.tsx (1)
Line range hint
14-20: Consider moving Cloudinary URLs to configuration.The hardcoded Cloudinary image URLs should be moved to a configuration file for better maintainability and easier updates.
Example implementation:
+ import { images } from '@/configs/images'; // ... - src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728132440/website/photos/highlights/engineer_byss3s.webp" + src={images.highlights.engineer} // ... - src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728132441/website/photos/highlights/google-org_clia2b.svg" + src={images.highlights.googleOrg}Also applies to: 29-35
website2/src/views/cleanairforum/PaginatedSection.tsx (1)
Line range hint
10-82: Consider extracting pagination logic to a custom hook.The pagination logic could be reused across components. Consider creating a custom hook like
usePagination:function usePagination<T>(items: T[], itemsPerPage: number) { const [currentPage, setCurrentPage] = useState(1); const totalPages = Math.ceil(items.length / itemsPerPage); const paginatedItems = useMemo(() => { const startIndex = (currentPage - 1) * itemsPerPage; return items.slice(startIndex, startIndex + itemsPerPage); }, [items, currentPage, itemsPerPage]); return { currentPage, setCurrentPage, totalPages, paginatedItems, }; }website2/src/views/events/EventCardsSection.tsx (1)
Line range hint
46-48: Consider using CSS Grid's auto-fit for better responsiveness.The current grid layout could be more responsive. Consider this improvement:
-<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-8"> +<div className="grid grid-cols-repeat(auto-fit, minmax(300px, 1fr)) gap-8">This would:
- Automatically adjust columns based on available space
- Eliminate the need for explicit breakpoints
- Maintain consistent card sizes
website2/src/views/home/AppDownloadSection.tsx (1)
23-25: Consider extracting common padding classes to mainConfig.The padding classes (
px-4 sm:px-6 lg:px-8 py-16 lg:py-24) appear to be layout-related. Consider moving these tomainConfigfor consistency with the centralization approach.- className={`${mainConfig.containerClass} relative px-4 sm:px-6 lg:px-8 py-16 lg:py-24`} + className={`${mainConfig.containerClass} ${mainConfig.sectionPadding} relative`}website2/src/views/careers/DetailsPage.tsx (2)
18-18: Consider extracting padding to mainConfig.The padding class (
p-8) could be moved tomainConfigfor consistency.- <div className={`${mainConfig.containerClass} p-8`}> + <div className={`${mainConfig.containerClass} ${mainConfig.contentPadding}`}>
60-62: Consider extracting common styling classes to mainConfig.The styling classes (
bg-white border border-gray-200 rounded-lg) could be moved tomainConfigas a reusable card style.- className={`${mainConfig.containerClass} w-full p-8 bg-white border border-gray-200 rounded-lg mt-6`} + className={`${mainConfig.containerClass} ${mainConfig.cardStyle} w-full mt-6`}website2/src/components/layouts/NewsLetter.tsx (1)
49-51: Consider extracting flex layout patterns to mainConfig.The flex layout classes could be moved to
mainConfigas they might be reused across components.- className={`${mainConfig.containerClass} flex flex-col lg:flex-row items-center lg:items-start lg:justify-between gap-12`} + className={`${mainConfig.containerClass} ${mainConfig.flexColumnToRow} gap-12`}website2/src/views/legal/PRP_Page.tsx (1)
102-102: Consider extracting flex layout patterns to mainConfig.The flex layout classes could be moved to
mainConfigfor consistency with other components.- <div className="w-full flex flex-col md:flex-row"> + <div className={`${mainConfig.flexColumnToRow}`}>website2/src/views/press/PressPage.tsx (1)
Line range hint
59-64: Consider memoizing the skeleton loader component.Since
ArticleSkeletonis a static component, it could benefit from memoization to prevent unnecessary re-renders.- const ArticleSkeleton = () => ( + const ArticleSkeleton = React.memo(() => ( <div className="p-8 lg:px-16 lg:py-12 space-y-8 rounded-lg shadow-sm bg-gray-200 animate-pulse"> {/* ... skeleton content ... */} </div> - ); + ));website2/src/views/solutions/AfricanCities/AfricanCities.tsx (1)
Line range hint
41-45: Consider enhancing error feedback.The error message could be more specific to help users understand what went wrong and what they can do about it.
- <div className="text-center py-20 text-red-500"> - Failed to load data. Please try again later. + <div className="text-center py-20 space-y-4"> + <p className="text-red-500">Unable to load African countries data.</p> + <p className="text-sm text-gray-600"> + Please check your connection and try refreshing the page. + </p> </div>website2/src/views/home/FeaturedCarousel.tsx (1)
Line range hint
124-143: Add keyboard navigation support.Consider adding keyboard navigation support for better accessibility.
+ useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'ArrowLeft') prevSlide(); + if (e.key === 'ArrowRight') nextSlide(); + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, []);website2/src/views/cleanairforum/FeaturedEvent.tsx (2)
Line range hint
48-68: Consider extracting skeleton and error components.These components could be reused across the application. Consider moving them to a shared components directory.
+// In @/components/ui/LoadingSkeleton.tsx +export const EventSkeleton = () => ( + <section + className={`${mainConfig.containerClass} w-full mt-16 bg-blue-50 rounded-lg shadow-sm p-6 lg:px-8 lg:py-12 flex flex-col lg:flex-row items-center gap-6 animate-pulse`} + > + {/* Current skeleton content */} + </section> +); +// In @/components/ui/ErrorMessage.tsx +export const EventError = ({ message }: { message: string }) => ( + <section + className={`${mainConfig.containerClass} w-full mt-16 bg-blue-50 rounded-lg shadow-sm p-6 lg:px-8 lg:py-12 flex flex-col lg:flex-row items-center gap-6`} + > + {/* Current error content */} + </section> +);
Line range hint
95-108: Consider using a date formatting library.Using a library like
date-fnsordayjswould provide more consistent date formatting and better localization support.+import { format } from 'date-fns'; - const formattedDate = end_date - ? new Date(end_date).toLocaleDateString('en-US', { - day: '2-digit', - month: 'long', - year: 'numeric', - }) - : 'Date TBA'; + const formattedDate = end_date + ? format(new Date(end_date), 'MMMM dd, yyyy') + : 'Date TBA'; - const formattedTime = - start_time && end_time - ? `${new Date(`1970-01-01T${start_time}`).toLocaleTimeString([], { - hour: '2-digit', - minute: '2-digit', - })} - ${new Date(`1970-01-01T${end_time}`).toLocaleTimeString([], { - hour: '2-digit', - minute: '2-digit', - })}` - : 'Time TBA'; + const formattedTime = + start_time && end_time + ? `${format(new Date(`1970-01-01T${start_time}`), 'h:mm a')} - ${format( + new Date(`1970-01-01T${end_time}`), + 'h:mm a' + )}` + : 'Time TBA';website2/src/views/publications/ResourcePage.tsx (1)
7-7: Consider moving padding configuration to mainConfig.While the container class is now centralized, the padding classes (
px-4 lg:px-8) are still hardcoded. Consider moving these to mainConfig for consistent padding across components.-<div className={`${mainConfig.containerClass} px-4 lg:px-8`}> +<div className={`${mainConfig.containerClass} ${mainConfig.paddingClass}`}>Also applies to: 70-71
website2/src/views/careers/CareerPage.tsx (1)
119-124: Consider enhancing loading skeleton accessibility.While the loading skeleton implementation is good, consider adding aria-label or role="status" to improve accessibility for screen readers.
<div key={idx} - className="flex items-center justify-between p-4 lg:p-6 bg-[#E0E0E0] rounded-lg animate-pulse" + className="flex items-center justify-between p-4 lg:p-6 bg-[#E0E0E0] rounded-lg animate-pulse" + role="status" + aria-label="Loading job listing" >website2/src/app/products/api/ApiPage.tsx (1)
89-89: Consider extracting common layout patterns.Multiple sections share similar layout patterns. Consider extracting these into reusable layout components to reduce duplication and improve maintainability.
Example of a reusable section layout:
interface SectionLayoutProps { children: React.ReactNode; className?: string; } const SectionLayout: React.FC<SectionLayoutProps> = ({ children, className = '' }) => ( <div className={`${mainConfig.containerClass} ${className}`}> {children} </div> );Also applies to: 120-122, 167-169
website2/src/app/products/mobile-app/MobilePage.tsx (1)
52-54: Well-structured layout configuration.The mainConfig integration maintains the responsive design while centralizing the layout configuration. The existing flex and grid arrangements are preserved appropriately.
However, consider adding JSDoc comments for the motion variants to improve code documentation.
+/** + * Animation variant for container elements + * Applies a staggered fade-in effect to children + */ const containerVariants = { hidden: { opacity: 0 }, visible: { opacity: 1, transition: { staggerChildren: 0.1, duration: 0.5, ease: 'easeOut', }, }, };Also applies to: 91-91, 121-123, 181-183, 237-239
website2/src/components/dialogs/EngagementDialog.tsx (1)
Line range hint
141-165: Consider enhancing form validation.The form validation could be improved by:
- Adding client-side email format validation
- Implementing a more robust terms acceptance check
const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); setLoading(true); setSubmissionError(null); + // Email validation + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(formData.email)) { + setSubmissionError('Please enter a valid email address'); + setLoading(false); + return; + } + + // Terms acceptance validation + if (!formData.termsAccepted) { + setSubmissionError('Please accept the terms and conditions'); + setLoading(false); + return; + } if (formData.termsAccepted && selectedCategory) {website2/src/views/cleanairforum/events/SingleEvent.tsx (1)
280-288: Consider simplifying date formatting.The date formatting logic could be simplified using a helper function to reduce code duplication.
+const formatTime = (time: string) => format(new Date(`1970-01-01T${time}`), 'h:mm a'); + <span className="text-sm text-gray-500"> - {format( - new Date(`1970-01-01T${session.start_time}`), - 'h:mm a', - )}{' '} - -{' '} - {format( - new Date(`1970-01-01T${session.end_time}`), - 'h:mm a', - )} + {formatTime(session.start_time)} - {formatTime(session.end_time)} </span>website2/src/views/cleanairforum/TabNavigation.tsx (2)
9-14: Good move on extracting the tabs array! 🎯Moving the tabs configuration outside the component prevents unnecessary re-creation on each render. Consider defining this as a constant using
as constfor better type inference:-const tabs = [ +const tabs = [ { label: 'About', value: '/clean-air-network' }, { label: 'Membership', value: '/clean-air-network/membership' }, { label: 'Events', value: '/clean-air-network/events' }, { label: 'Resources', value: '/clean-air-network/resources' }, -]; +] as const;
27-45: Clean and semantic implementation! ♿Great use of semantic HTML with the
navelement and proper styling transitions. Consider adding ARIA labels for better accessibility:- <nav className="border-y pt-4 overflow-x-auto bg-white border-gray-200"> + <nav + className="border-y pt-4 overflow-x-auto bg-white border-gray-200" + aria-label="Clean Air Network navigation" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (58)
website2/src/app/clean-air-forum/glossary/page.tsx(1 hunks)website2/src/app/clean-air-forum/layout.tsx(2 hunks)website2/src/app/clean-air-forum/logistics/page.tsx(2 hunks)website2/src/app/clean-air-forum/partners/page.tsx(5 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(4 hunks)website2/src/app/clean-air-forum/speakers/page.tsx(3 hunks)website2/src/app/legal/layout.tsx(2 hunks)website2/src/app/products/analytics/AnalyticsPage.tsx(6 hunks)website2/src/app/products/api/ApiPage.tsx(6 hunks)website2/src/app/products/calibrate/CalibratePage.tsx(6 hunks)website2/src/app/products/mobile-app/MobilePage.tsx(6 hunks)website2/src/app/products/monitor/MonitorPage.tsx(7 hunks)website2/src/components/dialogs/EngagementDialog.tsx(2 hunks)website2/src/components/layouts/ActionButtons.tsx(2 hunks)website2/src/components/layouts/ActionButtons2.tsx(1 hunks)website2/src/components/layouts/Footer.tsx(2 hunks)website2/src/components/layouts/Highlight.tsx(1 hunks)website2/src/components/layouts/Navbar.tsx(2 hunks)website2/src/components/layouts/NewsLetter.tsx(2 hunks)website2/src/components/layouts/NotificationBanner.tsx(1 hunks)website2/src/components/ui/Pagination.tsx(1 hunks)website2/src/configs/mainConfigs.ts(1 hunks)website2/src/hooks/useResourceFilter.ts(1 hunks)website2/src/types/index.ts(1 hunks)website2/src/utils/string-utils.ts(1 hunks)website2/src/views/about/AboutPage.tsx(5 hunks)website2/src/views/careers/CareerPage.tsx(3 hunks)website2/src/views/careers/DetailsPage.tsx(3 hunks)website2/src/views/cleanairforum/FeaturedEvent.tsx(3 hunks)website2/src/views/cleanairforum/PaginatedSection.tsx(2 hunks)website2/src/views/cleanairforum/RegisterBanner.tsx(1 hunks)website2/src/views/cleanairforum/TabNavigation.tsx(1 hunks)website2/src/views/cleanairforum/events/EventsPage.tsx(3 hunks)website2/src/views/cleanairforum/events/SingleEvent.tsx(5 hunks)website2/src/views/cleanairforum/membership/MemberPage.tsx(1 hunks)website2/src/views/cleanairforum/resources/ResourceCard.tsx(1 hunks)website2/src/views/cleanairforum/resources/ResourcePage.tsx(3 hunks)website2/src/views/events/EventCardsSection.tsx(2 hunks)website2/src/views/events/EventPage.tsx(6 hunks)website2/src/views/events/SingleEvent.tsx(4 hunks)website2/src/views/home/AnalyticsContentSection.tsx(2 hunks)website2/src/views/home/AppDownloadSection.tsx(2 hunks)website2/src/views/home/FeaturedCarousel.tsx(3 hunks)website2/src/views/home/HomePlayerSection.tsx(2 hunks)website2/src/views/home/HomeStatsSection.tsx(2 hunks)website2/src/views/legal/AirQoDataPage.tsx(1 hunks)website2/src/views/legal/PP_Page.tsx(1 hunks)website2/src/views/legal/PRP_Page.tsx(1 hunks)website2/src/views/legal/TOSPage.tsx(1 hunks)website2/src/views/legal/Tabsection.tsx(2 hunks)website2/src/views/press/PressPage.tsx(5 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(3 hunks)website2/src/views/solutions/communities/CommunityPage.tsx(6 hunks)website2/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- website2/src/app/clean-air-forum/speakers/page.tsx
- website2/src/views/legal/AirQoDataPage.tsx
- website2/src/app/clean-air-forum/partners/page.tsx
- website2/src/app/clean-air-forum/program-committee/page.tsx
- website2/src/app/clean-air-forum/schedule/page.tsx
- website2/src/app/clean-air-forum/resources/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
website2/src/views/cleanairforum/events/SingleEvent.tsx
[error] 253-253: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 301-301: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/src/views/events/SingleEvent.tsx
[error] 180-180: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 228-228: 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 (64)
website2/src/configs/mainConfigs.ts (1)
1-5: Well-structured type definition.The Config type clearly defines the expected shape of the configuration object with appropriate types.
website2/src/types/index.ts (2)
32-32: Good use of type alias for resource categories.Using a type alias derived from the Resource interface ensures type consistency and reduces duplication.
13-30: Comprehensive Resource interface with good type safety.The interface is well-structured with appropriate types. However:
resource_linkallows null - ensure proper null checking in UI components- Consider adding JSDoc comments for better documentation
✅ Verification successful
Proper null handling confirmed for
resource_linkThe codebase already implements thorough null checking for
resource_linkin components, with both TypeScript type safety and runtime validation. The implementation in ResourceCard.tsx uses a belt-and-suspenders approach with conditional rendering and explicit null checks.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for null handling of resource_link in components rg "resource_link" -A 3 -g "*.tsx"Length of output: 1007
website2/src/hooks/useResourceFilter.ts (1)
23-30: Good use of useMemo for pagination optimization.The pagination logic is well-implemented with proper performance optimization using useMemo.
website2/src/views/cleanairforum/RegisterBanner.tsx (2)
9-9: Good adoption of centralized configuration.The component correctly uses mainConfig for consistent layout styling.
Line range hint
33-37: Implement onClick handler for register button.The register button has an empty onClick handler. Please implement the registration functionality or add a TODO comment if it's pending.
website2/src/app/clean-air-forum/logistics/page.tsx (1)
15-16: Verify mainConfig integration for layout consistency.The hardcoded max-width and centering classes have been removed. To maintain layout consistency, ensure that these styles are now being applied through the mainConfig configuration.
Also applies to: 33-33
✅ Verification successful
✓ Layout configuration is properly integrated
The removed max-width and centering classes are correctly managed through mainConfig.containerClass at the layout level, ensuring consistent styling across all forum pages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if mainConfig is properly imported and used in the parent layout rg -l "mainConfig.containerClass" "website2/src/app/clean-air-forum/"Length of output: 111
Script:
#!/bin/bash # Check the layout implementation cat website2/src/app/clean-air-forum/layout.tsxLength of output: 1685
website2/src/views/legal/Tabsection.tsx (2)
9-12: Well-structured type definition for tabs.Good addition of TypeScript interface for improved type safety.
37-47: Good use of Next.js Link component.Replacing programmatic navigation with Link components improves performance and follows Next.js best practices.
website2/src/app/clean-air-forum/layout.tsx (1)
20-20: Good loading state management.Proper handling of loading state improves user experience.
Also applies to: 25-29
website2/src/components/layouts/NotificationBanner.tsx (1)
11-11: Good route management practice.Extracting route as a constant improves maintainability.
website2/src/components/layouts/ActionButtons.tsx (1)
5-5: Clean integration of mainConfig for container styling.The implementation correctly integrates the centralized layout configuration while preserving the existing flex layout classes.
Also applies to: 15-17
website2/src/components/layouts/ActionButtons2.tsx (1)
4-5: Clean integration of mainConfig for container styling.The implementation correctly integrates the centralized layout configuration while preserving the existing flex layout classes.
Also applies to: 10-12
website2/src/app/legal/layout.tsx (1)
5-5: Clean integration of mainConfig for container styling.The implementation correctly integrates the centralized layout configuration while maintaining the legal-page-content class.
Also applies to: 55-57
website2/src/components/layouts/Highlight.tsx (1)
5-6: Clean integration of mainConfig for container styling.The implementation correctly integrates the centralized layout configuration while preserving the existing styling classes.
Also applies to: 9-11
website2/src/views/home/AnalyticsContentSection.tsx (1)
4-4: Clean implementation of centralized layout configuration!The changes effectively integrate the centralized layout configuration while maintaining the component's existing styling utilities.
Also applies to: 32-34
website2/src/components/ui/Pagination.tsx (4)
9-10: Well-structured prop additions with good defaults!The new optional props enhance component flexibility while maintaining backward compatibility.
Also applies to: 17-18
20-28: Nice performance optimization with useCallback!The page change handler is properly memoized with the correct dependencies.
30-44: Consider edge case handling in pagination logic.While the pagination logic is sound, it might benefit from additional validation:
- Ensure maxVisiblePages is always positive
- Handle the case when maxVisiblePages > totalPages
50-53: Great accessibility improvements!The addition of semantic HTML (nav element) and ARIA attributes enhances accessibility.
Also applies to: 72-73, 83-83
website2/src/views/cleanairforum/PaginatedSection.tsx (1)
5-5: Clean integration of centralized layout configuration.The mainConfig integration preserves existing padding utilities while standardizing the container width.
Also applies to: 38-38
website2/src/views/events/EventCardsSection.tsx (1)
9-9: Clean integration of centralized layout configuration.The mainConfig integration maintains existing spacing utilities while standardizing the container width.
Also applies to: 39-41
website2/src/views/home/AppDownloadSection.tsx (1)
4-4: LGTM! Import aligns with centralized configuration approach.The addition of
mainConfigimport supports the PR's objective of centralizing layout configurations.website2/src/views/careers/DetailsPage.tsx (1)
9-9: LGTM! Import aligns with centralized configuration approach.The addition of
mainConfigimport supports the PR's objective of centralizing layout configurations.website2/src/components/layouts/NewsLetter.tsx (1)
5-5: LGTM! Import aligns with centralized configuration approach.The addition of
mainConfigimport supports the PR's objective of centralizing layout configurations.website2/src/views/press/PressPage.tsx (2)
8-8: Clean implementation of centralized styling!The integration of
mainConfigfor container classes enhances maintainability and consistency across the application.Also applies to: 47-47, 56-56
113-113: Good UX enhancement with scroll-to-top functionality!Adding the
scrollToTopprop improves user experience when navigating through paginated content.website2/src/views/solutions/AfricanCities/AfricanCities.tsx (1)
4-4: Well-structured responsive container implementation!The combination of
mainConfig.containerClasswith responsive padding classes provides a clean and maintainable solution.Also applies to: 53-53
website2/src/views/publications/ResourcePage.tsx (1)
163-163: Great UX improvement!Adding
scrollToTop={true}to the Pagination component ensures users are taken to the top of the content when changing pages, improving navigation experience.website2/src/views/home/HomePlayerSection.tsx (1)
10-10: Well-structured layout separation!Good job keeping component-specific layout classes (
flex-col-reverse lg:flex-row items-center justify-between) separate from the centralized container configuration.Also applies to: 69-71
website2/src/views/solutions/AfricanCities/AfricanCityPage.tsx (1)
9-9: Clean layout implementation!Good separation between the centralized container configuration and section-specific styling (
text-center).Also applies to: 50-50
website2/src/views/cleanairforum/membership/MemberPage.tsx (1)
6-6: Excellent skeleton loader implementation!The skeleton loader's layout perfectly mirrors the actual content's layout while maintaining proper separation between centralized container configuration and component-specific spacing classes.
Also applies to: 13-15
website2/src/views/events/EventPage.tsx (2)
9-9: LGTM! Import of mainConfig for centralized layout configuration.The addition of mainConfig import aligns with the PR's objective of centralizing layout configurations.
55-57: Consistent application of containerClass across all sections.The changes effectively implement the centralized layout configuration across loading states, error states, featured events section, tabs section, and event cards loading state. This enhances maintainability and ensures consistent layout behavior.
Also applies to: 74-76, 87-89, 155-157, 184-186
website2/src/views/careers/CareerPage.tsx (3)
9-9: LGTM! Import of mainConfig for centralized layout configuration.The addition of mainConfig import aligns with the PR's objective of centralizing layout configurations.
66-66: Enhanced spacing with margin-bottom addition.The addition of
mb-16to the header section improves the visual hierarchy and spacing between sections.
84-110: Well-structured categories section with improved loading state.The categories section has been effectively restructured with:
- Proper spacing and padding
- Responsive flex layout
- Enhanced loading skeleton implementation
website2/src/components/layouts/Footer.tsx (1)
11-11: LGTM! Consistent layout configuration in footer.The changes appropriately implement the centralized layout configuration in the footer component, maintaining consistency with other components.
Also applies to: 22-22
website2/src/app/products/api/ApiPage.tsx (2)
8-8: LGTM! Import of mainConfig for centralized layout configuration.The addition of mainConfig import aligns with the PR's objective of centralizing layout configurations.
52-54: Consistent application of containerClass across all sections.The changes effectively implement the centralized layout configuration across all major sections of the API page, ensuring consistent layout behavior and improved maintainability.
Also applies to: 89-89, 120-122, 167-169, 216-216
website2/src/components/layouts/Navbar.tsx (1)
189-191: LGTM! Clean implementation of centralized styling.The change effectively implements the centralized layout configuration using
mainConfig.containerClass.website2/src/views/cleanairforum/events/EventsPage.tsx (2)
128-128: LGTM! Consistent implementation of container class.The banner section correctly implements the centralized layout configuration.
155-157: LGTM! Clean implementation of filter section layout.The filter section properly utilizes the centralized container class while maintaining the existing padding and flex layout.
website2/src/app/products/calibrate/CalibratePage.tsx (3)
52-54: LGTM! Clean implementation of hero section layout.The grid layout and responsive design are preserved while implementing the centralized container class.
90-90: LGTM! Effective implementation in Why Calibrate section.The section maintains its grid structure while adopting the centralized styling approach.
123-125: LGTM! Consistent implementation across remaining sections.The Cost Effective, Calibrate Your Data, and Simple User Interface sections all correctly implement the centralized container class while preserving their unique layouts and responsive behaviors.
Also applies to: 177-179, 227-227
website2/src/views/home/HomeStatsSection.tsx (1)
141-141: LGTM! Clean implementation of stats section layout.The section effectively implements the centralized container class while maintaining the spacing and nested layout structure.
website2/src/views/legal/PP_Page.tsx (1)
228-228: LGTM! Clean integration of mainConfig.The change aligns with the PR's objective of centralizing layout configuration.
website2/src/app/products/analytics/AnalyticsPage.tsx (2)
9-9: Import placement looks good.The mainConfig import is properly placed with other imports.
53-54: Consistent application of mainConfig across sections.The layout configuration has been standardized effectively across all sections while maintaining the specific grid and flex arrangements for each section.
Also applies to: 91-91, 123-124, 197-198, 245-245
website2/src/app/products/monitor/MonitorPage.tsx (2)
53-55: Clean integration of mainConfig across all sections.The layout configuration has been standardized effectively while preserving the specific layout requirements for each section.
Also applies to: 95-97, 154-156, 201-203, 244-244, 273-275
Line range hint
122-134: Verify documentation links accessibility.The component includes links to various documentation PDFs. Let's verify their accessibility.
Also applies to: 282-297
✅ Verification successful
Documentation links are accessible and properly hosted ✓
Both the Maintenance Manual and User Guide PDFs are accessible and returning successful HTTP 200 responses.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accessibility of documentation links # List of documentation URLs to check urls=( "https://res.cloudinary.com/dbibjvyhm/image/upload/v1716038904/website/docs/Binos-Maintenance-Manual_agusuh.pdf" "https://res.cloudinary.com/dbibjvyhm/image/upload/v1716038903/website/docs/AirQoMonitorUserGuideV04_ogbfjs.pdf" ) # Check each URL for url in "${urls[@]}"; do echo "Checking $url" status_code=$(curl -s -o /dev/null -w "%{http_code}" "$url") if [ "$status_code" -eq 200 ]; then echo "✓ Accessible (HTTP $status_code)" else echo "✗ Not accessible (HTTP $status_code)" fi doneLength of output: 1284
website2/src/views/events/SingleEvent.tsx (1)
14-14: LGTM! Clean layout configuration.The use of
mainConfig.containerClassfor consistent layout styling is a good practice. It centralizes the layout configuration and makes it easier to maintain.Also applies to: 83-84, 137-138, 209-210
website2/src/views/solutions/communities/CommunityPage.tsx (1)
11-11: LGTM! Consistent layout configuration.The integration of
mainConfig.containerClassacross various sections maintains layout consistency while preserving the animation configurations.Also applies to: 54-54, 81-81, 160-160, 317-317
website2/src/components/dialogs/EngagementDialog.tsx (1)
8-8: LGTM! Clean dialog layout configuration.The use of
mainConfig.containerClassfor dialog content maintains consistency with the project's layout standards.Also applies to: 353-355
website2/src/views/about/AboutPage.tsx (3)
9-9: LGTM! Good move centralizing the layout configuration.The addition of mainConfig import aligns with the PR's objective of improving maintainability through centralized styling.
172-180: LGTM! Clean and semantic markup structure.The introduction section's markup is well-structured with appropriate spacing and typography classes.
189-193: Verify image dimensions and optimize loading strategy.The images are now served from Cloudinary CDN, which is great for performance. However, let's verify a few things:
- Ensure the image dimensions (1024x480) are optimal for the layout
- Consider using responsive image sizes with
srcSet- Verify if
eagerloading is necessary for all imagesAlso applies to: 197-201
✅ Verification successful
Current image implementation is well-optimized 🎯
The current setup is actually quite solid:
- Images are appropriately placed in a vision section with hover effects
- Eager loading is justified for these above-fold, interactive elements
- Dimensions are optimal for the layout (1024x480 for main, 1024x49 for overlay)
- Cloudinary CDN + responsive classes already provide good optimization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if these images are used above the fold rg -A 5 "teamImage_ganc1y|Frame_ugwgqr" src/Length of output: 91
Script:
#!/bin/bash # Search for the image references and surrounding context rg -B 5 -A 10 "teamImage_ganc1y|Frame_ugwgqr" # Look for similar image implementations and srcSet usage ast-grep --pattern 'src="https://res.cloudinary.com/dbibjvyhm/image/upload/$$$"' # Find all Image components with their props ast-grep --pattern '<Image $$$>'Length of output: 2288
website2/src/views/legal/TOSPage.tsx (1)
601-601: LGTM! Simplified container class.The container class has been simplified to use
w-full, which aligns with the PR's objective of standardizing layout approaches across the website.website2/src/app/clean-air-forum/glossary/page.tsx (1)
16-17: LGTM! Consistent layout approach.The container and divider styling changes maintain consistency with other pages while preserving the layout structure.
website2/tsconfig.json (1)
29-30: LGTM! Well-structured path configuration.The addition of
@/configs/*path alias supports the new centralized configuration approach and maintains consistency with the existing path structure.website2/src/views/cleanairforum/TabNavigation.tsx (3)
1-8: Clean and well-organized imports! 👌The imports are properly structured with a clear separation of Next.js components, types, and local configurations. The 'use client' directive is correctly placed at the top.
19-23: Smart handling of the root path edge case! 🧠The
isActiveTabfunction elegantly handles the special case for the root path while usingstartsWithfor sub-paths. This prevents false positives in path matching.
Line range hint
1-49: Excellent refactoring of the navigation component! 🌟The changes have significantly improved the component by:
- Using semantic HTML
- Centralizing styles through mainConfig
- Optimizing performance by moving static data outside
- Simplifying the active tab logic
- Using proper TypeScript types
This aligns perfectly with the PR's objectives of enhancing code maintainability and user experience.
| onClick={() => { | ||
| if (resource.resource_link) { | ||
| window.open( | ||
| resource.resource_link, | ||
| '_blank', | ||
| 'noopener,noreferrer', | ||
| ); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add URL validation before opening external links
While the link handling includes security attributes, it would be safer to validate URLs before opening them.
Consider this enhancement:
onClick={() => {
- if (resource.resource_link) {
+ if (resource.resource_link && /^https?:\/\//i.test(resource.resource_link)) {
window.open(
resource.resource_link,
'_blank',
'noopener,noreferrer',
);
+ } else {
+ console.error('Invalid or missing resource URL');
}
}}📝 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.
| onClick={() => { | |
| if (resource.resource_link) { | |
| window.open( | |
| resource.resource_link, | |
| '_blank', | |
| 'noopener,noreferrer', | |
| ); | |
| } | |
| }} | |
| onClick={() => { | |
| if (resource.resource_link && /^https?:\/\//i.test(resource.resource_link)) { | |
| window.open( | |
| resource.resource_link, | |
| '_blank', | |
| 'noopener,noreferrer', | |
| ); | |
| } else { | |
| console.error('Invalid or missing resource URL'); | |
| } | |
| }} |
| className="prose lg:prose-xl" | ||
| dangerouslySetInnerHTML={{ | ||
| __html: eventDetailsHtml || 'Event details coming soon.', | ||
| }} | ||
| ></div> |
There was a problem hiding this comment.
💡 Codebase verification
Confirmed XSS vulnerability in HTML rendering implementation
The current implementation in website2/src/utils/quillUtils.ts lacks HTML sanitization, which could allow malicious script injection. The convertDeltaToHtml function:
- Accepts and returns raw HTML without sanitization
- Uses QuillDeltaToHtmlConverter without security options
- Lacks protection against script injection
Recommended fixes:
- Install and implement DOMPurify or a similar HTML sanitization library
- Apply sanitization before returning HTML in
convertDeltaToHtml - Never accept raw HTML input without sanitization
🔗 Analysis chain
Potential XSS vulnerability with dangerouslySetInnerHTML.
The use of dangerouslySetInnerHTML could expose users to cross-site scripting (XSS) attacks. Please ensure that convertDeltaToHtml properly sanitizes the HTML output.
Let's verify the HTML sanitization in the utility function:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for sanitization in the utility function
rg -A 5 "convertDeltaToHtml" src/utils/
Length of output: 90
Script:
#!/bin/bash
# Search for convertDeltaToHtml in the entire website2 directory
rg -l "convertDeltaToHtml" website2/
# Find imports in SingleEvent.tsx
ast-grep --pattern 'import { $$$ } from "$_"' website2/src/views/events/SingleEvent.tsx
# Search for the function definition
ast-grep --pattern 'function convertDeltaToHtml($_) { $$$ }' website2/
ast-grep --pattern 'const convertDeltaToHtml = $_' website2/
Length of output: 3618
Script:
#!/bin/bash
# Check for isHtml function implementation
ast-grep --pattern 'function isHtml($_) { $$$ }' website2/
ast-grep --pattern 'const isHtml = $_' website2/
# Look for sanitization-related imports
rg -A 1 "import.*sanitize|import.*DOMPurify|import.*QuillDeltaToHtmlConverter" website2/src/utils/quillUtils.ts
Length of output: 880
🧰 Tools
🪛 Biome (1.9.4)
[error] 180-180: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
| className="prose lg:prose-xl" | ||
| dangerouslySetInnerHTML={{ | ||
| __html: eventDetailsHtml || 'Event details coming soon.', | ||
| }} | ||
| ></div> |
There was a problem hiding this comment.
💡 Codebase verification
Confirmed XSS vulnerability in HTML content handling
The review comment is valid. The convertDeltaToHtml function in quillUtils.ts either:
- Passes through raw HTML content directly if detected
- Converts Delta format without subsequent sanitization
To fix this:
- Install and implement DOMPurify:
npm install dompurify @types/dompurify - Modify
quillUtils.tsto sanitize HTML output before returning - Apply the fix to both SingleEvent components
🔗 Analysis chain
Potential XSS vulnerability with dangerouslySetInnerHTML.
Similar to the other event component, ensure that the HTML content is properly sanitized before rendering.
Let's verify the sanitization:
Also applies to: 301-304
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for HTML sanitization in the codebase
rg -A 5 "sanitize.*html|html.*sanitize" src/
Length of output: 91
Script:
#!/bin/bash
# Check package.json for sanitization libraries
cat website2/package.json | jq '.dependencies + .devDependencies'
# Search for sanitization-related code and DOMPurify usage
rg -i "(sanitize|purify|xss|dompurify)" website2/src/
# Look for eventDetailsHtml assignment or processing
rg "eventDetailsHtml" website2/src/
Length of output: 2229
Script:
#!/bin/bash
# Check for quill-delta-to-html usage
rg -A 5 "quill-delta-to-html" website2/src/
# Check event_details processing
rg -A 5 "event_details" website2/src/
Length of output: 1562
Script:
#!/bin/bash
# Check the implementation of convertDeltaToHtml
cat website2/src/utils/quillUtils.ts
Length of output: 2807
🧰 Tools
🪛 Biome (1.9.4)
[error] 253-253: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
website2/src/views/about/AboutPage.tsx (3)
44-47: Consider adding type safety to partner filtering.The current implementation uses
anytype for partners, which could lead to runtime errors. Let's add proper typing:- const filteredPartners = partners.filter( - (partner: any) => partner.website_category !== 'cleanair', - ); + interface Partner { + id: string | number; + website_category: string; + partner_logo_url?: string; + } + const filteredPartners = partners.filter( + (partner: Partner) => partner.website_category !== 'cleanair', + );
314-337: Enhance semantic structure of values list.The values list could benefit from better semantic markup and accessibility improvements.
- <ul className="space-y-6 text-lg text-gray-700 leading-relaxed"> + <ul className="space-y-6 text-lg text-gray-700 leading-relaxed" role="list"> <li> - <span className="font-bold">Citizen Focus:</span> + <h3 className="text-xl font-bold mb-2">Citizen Focus</h3> <br /> At AirQo, we believe that the main beneficiary of our work should be the citizen. </li>
390-396: Consider adding analytics tracking to partner CTA.The "Partner with Us" button is a key conversion point but lacks analytics tracking.
<CustomButton onClick={() => dispatch(openModal())} + data-analytics="partner-cta-click" className="bg-transparent p-0 m-0 flex items-center text-blue-700" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website2/src/views/about/AboutPage.tsx(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-website2-deploy-preview
| <PaginatedSection | ||
| logos={filteredPartners.map((partner: any) => ({ | ||
| id: partner.id, | ||
| logoUrl: partner.partner_logo_url || '', | ||
| }))} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing partner logos.
The partner logo mapping doesn't handle cases where partner_logo_url is undefined or invalid.
<PaginatedSection
- logos={filteredPartners.map((partner: any) => ({
- id: partner.id,
- logoUrl: partner.partner_logo_url || '',
- }))}
+ logos={filteredPartners
+ .filter((partner: Partner) => partner.partner_logo_url)
+ .map((partner: Partner) => ({
+ id: partner.id,
+ logoUrl: partner.partner_logo_url!,
+ }))}
/>Committable suggestion skipped: line range outside the PR's diff.
| <div className="relative w-full h-[400px] flex items-center justify-center"> | ||
| {/* Background Abstract Shape Image */} | ||
| <Image | ||
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about_us_vector_3_wiw2ie.png" | ||
| alt="Abstract Background" | ||
| width={800} | ||
| height={400} | ||
| className="absolute w-[300px] h-[200px] lg:w-[410px] lg:h-[225px]" | ||
| loading="eager" | ||
| /> | ||
|
|
||
| {/* Outline Abstract Shape Image */} | ||
| <Image | ||
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about-us-vector-2_mioxcy.png" | ||
| alt="Abstract Outline" | ||
| width={800} | ||
| height={400} | ||
| className="absolute w-[350px] h-[230px] lg:w-[450px] lg:h-[300px] object-contain" | ||
| loading="eager" | ||
| /> | ||
|
|
||
| {/* Text */} | ||
| <span className="text-gray-800 text-lg lg:text-[48px] relative"> | ||
| Our vision | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility of the vision section.
The vision section uses decorative images but lacks proper ARIA attributes.
- <div className="relative w-full h-[400px] flex items-center justify-center">
+ <div
+ className="relative w-full h-[400px] flex items-center justify-center"
+ role="presentation"
+ >
<Image
src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about_us_vector_3_wiw2ie.png"
alt="Abstract Background"
+ aria-hidden="true"
width={800}
height={400}
className="absolute w-[300px] h-[200px] lg:w-[410px] lg:h-[225px]"
loading="eager"
/>📝 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.
| <div className="relative w-full h-[400px] flex items-center justify-center"> | |
| {/* Background Abstract Shape Image */} | |
| <Image | |
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about_us_vector_3_wiw2ie.png" | |
| alt="Abstract Background" | |
| width={800} | |
| height={400} | |
| className="absolute w-[300px] h-[200px] lg:w-[410px] lg:h-[225px]" | |
| loading="eager" | |
| /> | |
| {/* Outline Abstract Shape Image */} | |
| <Image | |
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about-us-vector-2_mioxcy.png" | |
| alt="Abstract Outline" | |
| width={800} | |
| height={400} | |
| className="absolute w-[350px] h-[230px] lg:w-[450px] lg:h-[300px] object-contain" | |
| loading="eager" | |
| /> | |
| {/* Text */} | |
| <span className="text-gray-800 text-lg lg:text-[48px] relative"> | |
| Our vision | |
| </span> | |
| </div> | |
| <div | |
| className="relative w-full h-[400px] flex items-center justify-center" | |
| role="presentation" | |
| > | |
| {/* Background Abstract Shape Image */} | |
| <Image | |
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about_us_vector_3_wiw2ie.png" | |
| alt="Abstract Background" | |
| aria-hidden="true" | |
| width={800} | |
| height={400} | |
| className="absolute w-[300px] h-[200px] lg:w-[410px] lg:h-[225px]" | |
| loading="eager" | |
| /> | |
| {/* Outline Abstract Shape Image */} | |
| <Image | |
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about-us-vector-2_mioxcy.png" | |
| alt="Abstract Outline" | |
| width={800} | |
| height={400} | |
| className="absolute w-[350px] h-[230px] lg:w-[450px] lg:h-[300px] object-contain" | |
| loading="eager" | |
| /> | |
| {/* Text */} | |
| <span className="text-gray-800 text-lg lg:text-[48px] relative"> | |
| Our vision | |
| </span> | |
| </div> |
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728295735/website/photos/about/teamImage_ganc1y.png" | ||
| alt="Abstract Outline" | ||
| width={1024} | ||
| height={480} | ||
| className="object-cover w-auto h-auto" | ||
| loading="eager" | ||
| /> | ||
|
|
||
| {/* Outline Abstract Shape Image */} | ||
| <Image | ||
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728294910/website/photos/about/about-us-vector-2_mioxcy.png" | ||
| src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728295909/website/photos/about/Frame_ugwgqr.png" | ||
| alt="Abstract Outline" | ||
| width={800} | ||
| height={400} | ||
| className="absolute w-[350px] h-[230px] lg:w-[450px] lg:h-[300px] object-contain" | ||
| width={1024} | ||
| height={49} | ||
| className="absolute w-auto h-auto object-cover" |
There was a problem hiding this comment.
💡 Codebase verification
🎨 Critical: Large unoptimized image impacts page performance
The teamImage (12MB PNG) is significantly impacting page load performance. Here are the key issues:
teamImage_ganc1y.pngis 12MB in size and eagerly loaded- No Cloudinary optimization parameters are being used
- Hardcoded dimensions might not match actual image dimensions
Quick fixes:
- src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728295735/website/photos/about/teamImage_ganc1y.png"
+ src="https://res.cloudinary.com/dbibjvyhm/image/upload/f_auto,q_auto,w_1024/v1728295735/website/photos/about/teamImage_ganc1y.png"
- loading="eager"
+ loading="lazy"🔗 Analysis chain
Verify image dimensions and optimize loading strategy.
The images use hardcoded dimensions which might not match the actual image sizes from Cloudinary. This could affect the Cumulative Layout Shift (CLS) metric.
Consider using Cloudinary's transformation parameters to serve optimized images:
- src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728295735/website/photos/about/teamImage_ganc1y.png"
+ src="https://res.cloudinary.com/dbibjvyhm/image/upload/f_auto,q_auto/v1728295735/website/photos/about/teamImage_ganc1y.png"🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify actual dimensions of the images from Cloudinary
# Note: Using curl to fetch image metadata without downloading the full image
for url in \
"https://res.cloudinary.com/dbibjvyhm/image/upload/v1728295735/website/photos/about/teamImage_ganc1y.png" \
"https://res.cloudinary.com/dbibjvyhm/image/upload/v1728295909/website/photos/about/Frame_ugwgqr.png"
do
echo "Checking: $url"
curl -sI "$url" | grep -i "content-length\|content-type"
done
Length of output: 1684
Script:
#!/bin/bash
# Search for other usages of these images and any image optimization configs
rg -l "teamImage_ganc1y|Frame_ugwgqr"
# Check for image optimization related configs
fd -e config.js -e next.config.js -e next.config.mjs
Length of output: 464
|
New Website2 changes available for preview here |
|
New Website2 changes available for preview here |
1 similar comment
|
New Website2 changes available for preview here |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website2/src/hooks/swrConfig.ts (2)
18-20: LGTM! Consider monitoring failed requests.The retry configuration is well-documented and reasonable. The 5-second retry interval provides a good balance between quick recovery and avoiding network congestion.
Consider adding monitoring for failed requests to ensure the reduced retry count (3 total attempts) is sufficient for your use case.
21-28: Consider extracting retry configuration to constants.The custom retry function works correctly but duplicates values from the config. Consider making it more maintainable:
+// At the top of the file +const MAX_RETRY_COUNT = 2; +const RETRY_DELAY = 5000; + export const swrOptions: SWRConfiguration = { // ... other config - errorRetryCount: 2, - errorRetryInterval: 5000, + errorRetryCount: MAX_RETRY_COUNT, + errorRetryInterval: RETRY_DELAY, onErrorRetry: (error, key, config, revalidate, { retryCount }) => { - if (retryCount >= 2) return; + if (retryCount >= MAX_RETRY_COUNT) return; - setTimeout(() => revalidate({ retryCount: retryCount + 1 }), 5000); + setTimeout(() => revalidate({ retryCount: retryCount + 1 }), RETRY_DELAY); }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website2/src/configs/mainConfigs.ts(1 hunks)website2/src/hooks/swrConfig.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website2/src/configs/mainConfigs.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-website2-deploy-preview
🔇 Additional comments (1)
website2/src/hooks/swrConfig.ts (1)
1-1: Verify the impact of disabling automatic revalidation.Disabling
revalidateOnFocusandrevalidateOnReconnectmeans that data won't automatically refresh when users return to the tab or reconnect to the network. This could lead to stale data being displayed.Please confirm if this aligns with the application's requirements, especially for time-sensitive data like air quality metrics.
Also applies to: 3-3, 5-5, 8-8
|
New Website2 changes available for preview here |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
Release Notes
New Features
mainConfig.useResourceFilterfor managing resource filtering and pagination.ResourceCardcomponent for displaying resource information.Improvements
Linkinstead of imperative routing.Refactoring
Technical Enhancements