Website: Integrate Google Analytics and dynamic sitemap.xml setup #2494
Conversation
📝 WalkthroughWalkthroughThe pull request integrates Google Analytics into the application by conditionally rendering a new GA component within the Changes
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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
🧹 Nitpick comments (4)
website2/src/components/GoogleAnalytics.tsx (1)
15-19: Consider adding error handling for the GA script.The script loading looks good, but consider adding an
onErrorhandler to track script loading failures.<Script src={`https://www.googletagmanager.com/gtag/js?id=${measurementId}`} strategy="afterInteractive" + onError={(e) => { + console.error('Error loading Google Analytics:', e); + }} // Optionally, use strategy="lazyOnload" if you experience performance issues. />website2/src/app/layout.tsx (1)
37-37: Add type safety for environment variable.Consider adding type checking for the GA measurement ID.
- const GA_MEASUREMENT_ID = process.env.NEXT_PUBLIC_GA_MEASUREMENT_ID; + const GA_MEASUREMENT_ID = process.env.NEXT_PUBLIC_GA_MEASUREMENT_ID as string | undefined; + + // Validate measurement ID format + if (GA_MEASUREMENT_ID && !GA_MEASUREMENT_ID.startsWith('G-')) { + console.warn('Invalid Google Analytics measurement ID format'); + }website2/src/app/sitemap.xml/route.ts (1)
58-58: Consider using actual last modification dates.Instead of using the current date for all URLs, consider tracking actual content modification dates.
- <lastmod>${new Date().toISOString().split('T')[0]}</lastmod> + <lastmod>${getLastModifiedDate(path)}</lastmod>Add this helper function:
async function getLastModifiedDate(path: string): Promise<string> { // You could fetch this from your CMS or content metadata // For now, fallback to current date return new Date().toISOString().split('T')[0]; }website2/src/components/layouts/NewsLetter.tsx (1)
46-63: Consider increasing the auto-reset timer duration.While the auto-reset functionality is well-implemented with proper cleanup, 5 seconds might be too short for users to read the success message. Consider increasing it to 8-10 seconds for better user experience.
- }, 5000); + }, 8000);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
website2/src/app/layout.tsx(2 hunks)website2/src/app/robots.txt/route.ts(1 hunks)website2/src/app/sitemap.xml/route.ts(1 hunks)website2/src/components/GoogleAnalytics.tsx(1 hunks)website2/src/components/layouts/NewsLetter.tsx(2 hunks)website2/src/views/careers/CareerPage.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-website2-deploy-preview
🔇 Additional comments (4)
website2/src/components/layouts/NewsLetter.tsx (3)
38-41: Nice improvement to error handling!The error status is now consistently set in both the catch block and the else block, ensuring users always receive proper feedback.
70-125: Excellent responsive layout implementation!The split layout for larger screens with proper spacing and alignment provides a better visual hierarchy and user experience. Good use of Tailwind's responsive classes.
127-152: Great feedback UI implementation!The centered feedback states with emojis and clear messaging provide excellent user experience. Good job on accessibility with proper role and aria-label attributes.
website2/src/views/careers/CareerPage.tsx (1)
136-136: Excellent improvement to the no positions message!The new message is more engaging and informative, providing better context while encouraging users to check back later. This aligns well with maintaining user engagement even when no positions are available.
| export async function GET() { | ||
| const robots = ` | ||
| User-agent: * | ||
| Allow: / | ||
| Sitemap: https://airqo.net/sitemap.xml | ||
| `; | ||
|
|
||
| return new Response(robots, { | ||
| status: 200, | ||
| headers: { | ||
| 'Content-Type': 'text/plain', | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the hardcoded sitemap URL.
The implementation looks good, but let's ensure the hardcoded sitemap URL is correct and matches your production environment.
🏁 Script executed:
#!/bin/bash
# Description: Verify the sitemap URL is accessible
curl -I https://airqo.net/sitemap.xmlLength of output: 872
Action Required: Update the sitemap URL in the robots.txt route.
While the dynamic robots.txt implementation is correctly structured and follows Next.js conventions, our verification indicates that the sitemap URL ("https://airqo.net/sitemap.xml") returns a 404 error. This suggests that the URL might be incorrect or not yet deployed in the production environment.
- Please verify the correct sitemap URL for your production setup.
- Update the hardcoded URL in
website2/src/app/robots.txt/route.tsaccordingly.
| <Script id="ga-script" strategy="afterInteractive"> | ||
| {` | ||
| window.dataLayer = window.dataLayer || []; | ||
| function gtag(){ dataLayer.push(arguments); } | ||
| gtag('js', new Date()); | ||
| gtag('config', '${measurementId}'); | ||
| `} | ||
| </Script> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding cookie consent check.
The GA initialization looks good, but consider checking for user consent before initializing tracking.
<Script id="ga-script" strategy="afterInteractive">
{`
window.dataLayer = window.dataLayer || [];
function gtag(){ dataLayer.push(arguments); }
gtag('js', new Date());
+ // Only initialize GA if user has consented to tracking
+ if (localStorage.getItem('cookieConsent') === 'true') {
gtag('config', '${measurementId}');
+ }
`}
</Script>📝 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.
| <Script id="ga-script" strategy="afterInteractive"> | |
| {` | |
| window.dataLayer = window.dataLayer || []; | |
| function gtag(){ dataLayer.push(arguments); } | |
| gtag('js', new Date()); | |
| gtag('config', '${measurementId}'); | |
| `} | |
| </Script> | |
| <Script id="ga-script" strategy="afterInteractive"> | |
| {` | |
| window.dataLayer = window.dataLayer || []; | |
| function gtag(){ dataLayer.push(arguments); } | |
| gtag('js', new Date()); | |
| // Only initialize GA if user has consented to tracking | |
| if (localStorage.getItem('cookieConsent') === 'true') { | |
| gtag('config', '${measurementId}'); | |
| } | |
| `} | |
| </Script> |
| const urlsXml = staticRoutes | ||
| .map((route) => { | ||
| // Remove leading/trailing slashes, then create the full URL | ||
| const path = route.replace(/^\/|\/$/g, ''); | ||
| return ` | ||
| <url> | ||
| <loc>${baseUrl}/${path}</loc> | ||
| <lastmod>${new Date().toISOString().split('T')[0]}</lastmod> | ||
| <priority>0.80</priority> | ||
| </url> | ||
| `; | ||
| }) | ||
| .join(''); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding dynamic routes to sitemap.
The static routes look good, but consider adding support for dynamic routes (e.g., blog posts, events).
const urlsXml = staticRoutes
.map((route) => {
const path = route.replace(/^\/|\/$/g, '');
return generateUrlEntry(baseUrl, path);
})
+ .concat(await getDynamicRoutes()) // Add dynamic routes
.join('');
+async function getDynamicRoutes() {
+ // Fetch dynamic routes from your CMS or database
+ // Example: blog posts, events, etc.
+ return [];
+}
+
+function generateUrlEntry(baseUrl: string, path: string) {
+ return `
+ <url>
+ <loc>${baseUrl}/${path}</loc>
+ <lastmod>${new Date().toISOString().split('T')[0]}</lastmod>
+ <priority>0.80</priority>
+ </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.
| const urlsXml = staticRoutes | |
| .map((route) => { | |
| // Remove leading/trailing slashes, then create the full URL | |
| const path = route.replace(/^\/|\/$/g, ''); | |
| return ` | |
| <url> | |
| <loc>${baseUrl}/${path}</loc> | |
| <lastmod>${new Date().toISOString().split('T')[0]}</lastmod> | |
| <priority>0.80</priority> | |
| </url> | |
| `; | |
| }) | |
| .join(''); | |
| const urlsXml = staticRoutes | |
| .map((route) => { | |
| const path = route.replace(/^\/|\/$/g, ''); | |
| return generateUrlEntry(baseUrl, path); | |
| }) | |
| .concat(await getDynamicRoutes()) | |
| .join(''); | |
| async function getDynamicRoutes() { | |
| // Fetch dynamic routes from your CMS or database | |
| // Example: blog posts, events, etc. | |
| return []; | |
| } | |
| function generateUrlEntry(baseUrl: string, path: string) { | |
| return ` | |
| <url> | |
| <loc>${baseUrl}/${path}</loc> | |
| <lastmod>${new Date().toISOString().split('T')[0]}</lastmod> | |
| <priority>0.80</priority> | |
| </url> | |
| `; | |
| } |
|
New Website2 changes available for preview here |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
website2/src/views/solutions/AfricanCities/AfricanCities.tsx (2)
104-124: Great responsive design implementation with room for optimization.The new grid layout with progressive enhancement for mobile is well thought out. Consider these optimizations:
- Add responsive image sizes to optimize downloads:
<Image src={img.image_url} alt={`City Image ${img.id}`} width={500} height={500} + sizes="(max-width: 640px) 100vw, 50vw" className="rounded-lg shadow-md object-cover w-full h-full transition-transform duration-500 ease-in-out transform hover:scale-110 cursor-pointer" />
- Prevent layout shifts during hover:
-className="rounded-lg shadow-md object-cover w-full h-full transition-transform duration-500 ease-in-out transform hover:scale-110 cursor-pointer" +className="rounded-lg shadow-md object-cover w-full h-full transition-transform duration-500 ease-in-out transform hover:scale-110 cursor-pointer will-change-transform"
125-134: Optimize the overlay blob image loading.Since this is a decorative overlay, consider these performance optimizations:
<Image src="https://res.cloudinary.com/dbibjvyhm/image/upload/v1728132435/website/photos/about_us_vector_3_p0mihk.png" alt="Blob Overlay" width={510} height={360} + loading="lazy" + priority={false} className="object-cover rounded-lg w-full h-full" />Also, consider adding
role="presentation"since this is a decorative element.website2/src/views/solutions/communities/CommunityPage.tsx (2)
112-148: Consider optimizing image dimensions for performance.The image section restructuring improves responsive behavior and accessibility with proper alt text. However, the increased image dimensions (500x500, 500x700) might impact page load performance.
Consider:
- Using Next.js's built-in image optimization features:
<Image src="..." alt="Champion 2" width={500} height={500} + quality={75} + priority={false} className="rounded-lg object-cover w-full h-full" />
- Implementing lazy loading for images below the fold
- Using responsive image sizes:
<Image src="..." alt="Champion 2" width={500} height={500} + sizes="(max-width: 640px) 100vw, 50vw" className="rounded-lg object-cover w-full h-full" />Also applies to: 241-277
94-94: Use Tailwind's built-in text sizes for consistency.Using exact pixel values (
text-[32px]) for text sizes might lead to inconsistency across the application. Tailwind provides predefined text sizes that maintain a consistent type scale.Consider using Tailwind's built-in text sizes:
-<h2 className="text-[32px] leading-[36px] font-medium"> +<h2 className="text-4xl leading-tight font-medium">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
website2/.env.sample(1 hunks)website2/src/views/solutions/AfricanCities/AfricanCities.tsx(2 hunks)website2/src/views/solutions/communities/CommunityPage.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- website2/.env.sample
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-website2-deploy-preview
🔇 Additional comments (2)
website2/src/views/solutions/AfricanCities/AfricanCities.tsx (1)
38-38: Nice improvement on the early return!Explicitly returning
nullinstead of an implicitundefinedfollows React best practices and improves type safety.website2/src/views/solutions/communities/CommunityPage.tsx (1)
81-81:❓ Verification inconclusive
Verify layout breakpoints on various screen sizes.
Changing from
lg:grid-cols-2tosm:grid-cols-2triggers the two-column layout on smaller screens (640px) instead of larger screens (1024px). This might cause content to feel cramped on tablets and smaller laptops.Consider testing the layout on various devices to ensure optimal content presentation:
Also applies to: 218-218
🏁 Script executed:
#!/bin/bash # Description: Search for similar grid breakpoint patterns in the codebase # to ensure consistency in responsive design approach # Search for grid-cols-2 usage with breakpoints rg "grid-cols-\d+\s+\w+:grid-cols-2" --type tsxLength of output: 77
Verify layout breakpoints on various screen sizes.
Changing from
lg:grid-cols-2tosm:grid-cols-2activates the two-column layout at 640px instead of 1024px. This change may cause content to appear cramped on tablets and smaller laptops. Since our initial script returned an error due to the unrecognized file type "tsx", please manually verify the responsive behavior across devices. Consider testing the layout to ensure optimal presentation and consistency with other grid implementations.
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
Summary by CodeRabbit