Conversation
…ction and user experience
…roved privacy messaging
ColdByDefault
added
enhancement
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR introduces version 6.0.4 with a new PageSpeed Insights integration feature and improved locale detection. The changes add a live performance monitoring widget that displays Google Lighthouse scores, replaces the browser translation notice with a more sophisticated locale auto-detection system, and internationalizes the cookie banner.
Changes:
- Added SpeedInsight component with real-time Google PageSpeed Insights integration
- Replaced BrowserTranslationNotice with LocaleAutoDetect for improved locale handling
- Added translations for speedInsight, localeDetect, and CookieBanner across all supported languages (en, de, es, fr, sv)
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| types/configs/speed-insight.ts | New TypeScript type definitions for PageSpeed API responses and component state |
| components/speed-insight/SpeedInsight.tsx | Main UI component rendering live PageSpeed scores with tabbed desktop/mobile views |
| components/speed-insight/SpeedInsight.logic.ts | Data fetching hook with caching and cooldown management |
| components/speed-insight/SpeedInsight.constants.ts | Configuration constants for thresholds, caching, and API endpoints |
| components/speed-insight/index.ts | Barrel export for SpeedInsight component |
| app/api/speed-insight/route.ts | API route handler fetching and parsing Google PageSpeed Insights data |
| components/languages/locale-auto-detect.tsx | New component for client-side locale detection with toast notifications |
| components/languages/language-switcher.tsx | Updated to handle "unknown" browser language cookie and improved detection |
| components/languages/browser-translation-notice.tsx | Removed legacy component replaced by locale-auto-detect |
| components/languages/index.ts | Updated exports to replace BrowserTranslationNotice with LocaleAutoDetect |
| components/cookies/cookies-banner.tsx | Internationalized all text strings using next-intl translations |
| components/ui/tabs.tsx | Code formatting improvements and standardized semicolons |
| messages/*.json | Added speedInsight, localeDetect, and CookieBanner translation keys across all locales |
| proxy.ts | Updated comment referencing renamed component |
| app/page.tsx | Integrated SpeedInsight component with dynamic loading |
| app/layout.tsx | Replaced BrowserTranslationNotice with LocaleAutoDetect |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export async function GET(request: Request): Promise<NextResponse> { | ||
| const { searchParams } = new URL(request.url); | ||
| const forceRefresh = searchParams.has("refresh"); | ||
|
|
||
| try { | ||
| const [desktop, mobile] = await Promise.all([ | ||
| fetchPageSpeed("desktop", forceRefresh), | ||
| fetchPageSpeed("mobile", forceRefresh), | ||
| ]); | ||
|
|
||
| const body: SpeedInsightApiResponse = { desktop, mobile }; | ||
|
|
||
| return NextResponse.json(body, { | ||
| headers: { | ||
| "Cache-Control": "public, s-maxage=3600, stale-while-revalidate=7200", | ||
| "X-Content-Type-Options": "nosniff", | ||
| "X-Frame-Options": "DENY", | ||
| "X-XSS-Protection": "1; mode=block", | ||
| "Referrer-Policy": "strict-origin-when-cross-origin", | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| console.error("PageSpeed API Error:", error); | ||
|
|
||
| return NextResponse.json( | ||
| { | ||
| error: "Failed to fetch PageSpeed data", | ||
| message: sanitizeErrorMessage(error), | ||
| desktop: null, | ||
| mobile: null, | ||
| } satisfies SpeedInsightApiResponse & { message: string }, | ||
| { | ||
| status: 500, | ||
| headers: { | ||
| "X-Content-Type-Options": "nosniff", | ||
| "X-Frame-Options": "DENY", | ||
| }, | ||
| }, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The speed-insight API route lacks rate limiting, which could allow abuse of the Google PageSpeed API. Other API routes in this codebase (e.g., app/api/github/route.ts, app/api/about/route.ts) implement RateLimiter to protect against excessive requests. Consider adding rate limiting to prevent abuse and protect the Google PageSpeed API quota, especially since API calls are expensive and the endpoint is publicly accessible.
| if (currentLocale !== detectedLocale) { | ||
| setCookie("PORTFOLIOVERSIONLATEST_LOCALE", detectedLocale); | ||
| // Store what we detected so we can show it after refresh | ||
| sessionStorage.setItem( | ||
| "locale-detection-result", | ||
| JSON.stringify({ | ||
| browserLang, | ||
| switchedTo: detectedLocale, | ||
| isSupported: isSupportedLocale(browserLang), | ||
| serverDetected: false, | ||
| }), | ||
| ); | ||
| router.refresh(); | ||
| return; |
There was a problem hiding this comment.
The component calls router.refresh() to update the locale, but this occurs inside an effect that depends on isClient and router. After calling router.refresh(), the component returns early, which prevents the toast from being shown. However, the data is stored in sessionStorage, so it will be shown after the refresh. Consider adding a comment explaining this two-step process (store data -> refresh -> show toast) to improve code maintainability.
| "acceptAll": "Accept All Cookies", | ||
| "essentialOnly": "Essential Only", | ||
| "declineAnalytics": "Decline Analytics", | ||
| "close": "close" |
There was a problem hiding this comment.
The "close" value should be capitalized to "Close" for consistency with other translations (de.json, es.json, fr.json, sv.json all have capitalized versions: "Schließen", "Cerrar", "Fermer", "Stäng"). This inconsistency affects the UI where the close button text appears in lowercase only in English.
| "close": "close" | |
| "close": "Close" |
| className={cn( | ||
| "bg-muted text-muted-foreground inline-flex h-9 w-fit items-center justify-center rounded-lg p-[3px]", | ||
| className | ||
| "bg-muted text-muted-foreground inline-flex h-9 w-fit items-center justify-center rounded-lg p-0.75", |
There was a problem hiding this comment.
The class "p-0.75" is not a valid Tailwind CSS spacing utility. Tailwind's default spacing scale doesn't include 0.75. The valid options are p-0, p-0.5, p-1, p-1.5, p-2, p-2.5, p-3, etc. This should likely be "p-0.5" (2px) or "p-1" (4px) to match Tailwind's spacing scale. The original value "p-[3px]" was explicit and valid - changing to an invalid utility class will cause the padding to not be applied.
| "bg-muted text-muted-foreground inline-flex h-9 w-fit items-center justify-center rounded-lg p-0.75", | |
| "bg-muted text-muted-foreground inline-flex h-9 w-fit items-center justify-center rounded-lg p-[3px]", |
No description provided.