Merge incoming theme/build updates and fix lint regressions#9
Merge incoming theme/build updates and fix lint regressions#9
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Merges incoming build/performance/theme-related updates and resolves lint/format regressions across the Next.js app, including metadata normalization and hook dependency fixes.
Changes:
- Normalized formatting (double quotes/semicolons/wrapping) across pages, UI components, and MDX metadata.
- Updated/added several UI utilities (text effects, animated background, spotlight, morphing dialog) and introduced a
ThemeTogglecomponent. - Adjusted build/runtime config and SEO-related routes (robots/sitemap), plus documentation whitespace cleanups.
Reviewed changes
Copilot reviewed 33 out of 38 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| server.log | Adds a runtime log artifact file. |
| next.config.mjs | Removes swcMinify and keeps MDX + security headers config. |
| lib/utils.ts | Formats cn() utility and imports. |
| lib/portfolio-data.ts | Normalizes portfolio content formatting/structure. |
| lib/constants.ts | Normalizes string literal + semicolon. |
| components/ui/text-morph.tsx | Formatting + minor string/JSX normalization. |
| components/ui/text-loop.tsx | Formatting + interval loop logic retained. |
| components/ui/text-effect.tsx | Formatting + minor refactors (unused-var handling). |
| components/ui/spotlight.tsx | Formatting + event listener wiring. |
| components/ui/scroll-progress.tsx | Formatting + minor options normalization. |
| components/ui/morphing-dialog.tsx | Formatting + dialog/focus handling code retained. |
| components/ui/magnetic.tsx | Formatting + effect dependency updates. |
| components/ui/animated-background.tsx | Improves typing around Children.map/cloneElement and formatting. |
| components/theme-toggle.tsx | Introduces a client theme toggle component. |
| app/skills/page.tsx | Formatting/wrapping updates. |
| app/sitemap.ts | Formatting/string normalization. |
| app/robots.ts | Formatting/string normalization. |
| app/projects/page.tsx | Formatting/wrapping updates. |
| app/page.tsx | Formatting + role rotator effect dependency fix. |
| app/open-source/page.tsx | Formatting/wrapping updates. |
| app/not-found.tsx | Formatting + HTML entity escaping for apostrophes. |
| app/layout.tsx | Metadata/viewport formatting and layout structuring updates. |
| app/globals.css | Moves Google Fonts @import and whitespace normalization. |
| app/experience/page.tsx | Formatting/wrapping updates. |
| app/contact/page.tsx | Formatting/wrapping updates and escaping for apostrophes. |
| app/components/tech-timeline.tsx | Formatting/wrapping updates. |
| app/components/navigation.tsx | Formatting/wrapping updates. |
| app/components/footer.tsx | Formatting/wrapping updates. |
| app/certifications/page.tsx | Formatting/wrapping updates. |
| app/blog/layout.tsx | Formatting + copy button logic retained. |
| app/blog/exploring-the-intersection-of-design-ai-and-design-engineering/page.mdx | Normalizes metadata + canonical wrapping; adds eslint-disable header. |
| app/blog/example-mdx-metadata/page.mdx | Normalizes metadata example formatting. |
| app/achievements/page.tsx | Formatting/wrapping updates. |
| app/about/page.tsx | Formatting/wrapping updates. |
| MAINTENANCE_GUIDE.md | Whitespace cleanup. |
| FINAL_OPTIMIZATION.md | Whitespace cleanup. |
| DELIVERY_SUMMARY.txt | Whitespace cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -217,25 +217,25 @@ function MorphingDialogContent({ | |||
| > | |||
There was a problem hiding this comment.
MorphingDialogTrigger sets aria-controls to motion-ui-morphing-dialog-content-${uniqueId}, but MorphingDialogContent doesn’t define a corresponding id on the dialog element. Add an id here to match aria-controls (or adjust/remove aria-controls) so assistive tech can correctly associate the trigger with the dialog.
| import { useTheme } from "next-themes"; | ||
| import { useEffect, useState } from "react"; | ||
| import { Moon, Sun } from "lucide-react"; | ||
|
|
||
| export function ThemeToggle() { | ||
| const { setTheme, theme } = useTheme(); | ||
| const [mounted, setMounted] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| setMounted(true); | ||
| }, []); | ||
|
|
||
| if (!mounted) { | ||
| return ( | ||
| <button | ||
| className="rounded-full p-2 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-800" | ||
| aria-label="Toggle theme" | ||
| > | ||
| <Sun className="h-5 w-5 text-zinc-900 dark:text-zinc-100 opacity-0" /> | ||
| </button> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <button | ||
| onClick={() => setTheme(theme === "dark" ? "light" : "dark")} | ||
| className="rounded-full p-2 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-800" | ||
| aria-label="Toggle theme" | ||
| > | ||
| {theme === "dark" ? ( | ||
| <Moon className="h-5 w-5 text-zinc-900 dark:text-zinc-100" /> | ||
| ) : ( | ||
| <Sun className="h-5 w-5 text-zinc-900 dark:text-zinc-100" /> | ||
| )} | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
This ThemeToggle component is introduced but there are no references to it anywhere in the codebase (so it won’t render). Either wire it into the UI (e.g., navigation/header) or remove it to avoid dead code.
| import { useTheme } from "next-themes"; | |
| import { useEffect, useState } from "react"; | |
| import { Moon, Sun } from "lucide-react"; | |
| export function ThemeToggle() { | |
| const { setTheme, theme } = useTheme(); | |
| const [mounted, setMounted] = useState(false); | |
| useEffect(() => { | |
| setMounted(true); | |
| }, []); | |
| if (!mounted) { | |
| return ( | |
| <button | |
| className="rounded-full p-2 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-800" | |
| aria-label="Toggle theme" | |
| > | |
| <Sun className="h-5 w-5 text-zinc-900 dark:text-zinc-100 opacity-0" /> | |
| </button> | |
| ); | |
| } | |
| return ( | |
| <button | |
| onClick={() => setTheme(theme === "dark" ? "light" : "dark")} | |
| className="rounded-full p-2 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-800" | |
| aria-label="Toggle theme" | |
| > | |
| {theme === "dark" ? ( | |
| <Moon className="h-5 w-5 text-zinc-900 dark:text-zinc-100" /> | |
| ) : ( | |
| <Sun className="h-5 w-5 text-zinc-900 dark:text-zinc-100" /> | |
| )} | |
| </button> | |
| ); | |
| } | |
| // ThemeToggle component removed because it was not referenced anywhere in the codebase. | |
| // This avoids keeping dead code around. Reintroduce and wire it into the UI if needed. |
| @import url('https://fonts.googleapis.com/css2?family=JetBrains+Mono:wght@400;500;600;700&display=swap'); | ||
| @import 'tailwindcss'; | ||
|
|
There was a problem hiding this comment.
globals.css imports JetBrains Mono from fonts.googleapis.com even though fonts are already configured via next/font/google in app/layout.tsx. This adds an extra external request (and can fail in locked-down environments, as noted in the PR). Prefer removing the CSS @import and relying on next/font for self-hosted fonts.
| <button | ||
| className="rounded-full p-2 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-800" | ||
| aria-label="Toggle theme" | ||
| > | ||
| <Sun className="h-5 w-5 text-zinc-900 dark:text-zinc-100 opacity-0" /> | ||
| </button> |
There was a problem hiding this comment.
These buttons omit an explicit type. If ThemeToggle is ever rendered inside a
type="button" for both the mounted and fallback button renders.
| onClick={() => setTheme(theme === "dark" ? "light" : "dark")} | ||
| className="rounded-full p-2 transition-colors hover:bg-zinc-100 dark:hover:bg-zinc-800" | ||
| aria-label="Toggle theme" | ||
| > | ||
| {theme === "dark" ? ( | ||
| <Moon className="h-5 w-5 text-zinc-900 dark:text-zinc-100" /> | ||
| ) : ( | ||
| <Sun className="h-5 w-5 text-zinc-900 dark:text-zinc-100" /> | ||
| )} |
There was a problem hiding this comment.
next-themes theme can be "system" or temporarily undefined; using it directly for both the icon and toggle decision can lead to inconsistent UI. Prefer resolvedTheme for display and toggle logic, and handle the non-"dark"/"light" case explicitly.
| parentElement.addEventListener("mousemove", handleMouseMove); | ||
| parentElement.addEventListener("mouseenter", () => setIsHovered(true)); | ||
| parentElement.addEventListener("mouseleave", () => setIsHovered(false)); | ||
|
|
||
| return () => { | ||
| parentElement.removeEventListener('mousemove', handleMouseMove) | ||
| parentElement.removeEventListener('mouseenter', () => setIsHovered(true)) | ||
| parentElement.removeEventListener('mouseleave', () => setIsHovered(false)) | ||
| } | ||
| }, [parentElement, handleMouseMove]) | ||
| parentElement.removeEventListener("mousemove", handleMouseMove); | ||
| parentElement.removeEventListener("mouseenter", () => setIsHovered(true)); | ||
| parentElement.removeEventListener("mouseleave", () => | ||
| setIsHovered(false), | ||
| ); |
There was a problem hiding this comment.
The mouseenter/mouseleave listeners are added with inline arrow functions but removed with new inline arrow functions, so the cleanup never removes the original listeners. This will leak listeners and can keep state updates firing after unmount. Store the handlers in stable variables (e.g., useCallback) and pass the same function references to both addEventListener and removeEventListener.
| return ( | ||
| <motion.div | ||
| ref={triggerRef} | ||
| layoutId={`dialog-${uniqueId}`} | ||
| className={cn('relative cursor-pointer', className)} | ||
| className={cn("relative cursor-pointer", className)} |
There was a problem hiding this comment.
The dialog trigger ref stored in context (used for focus restoration in MorphingDialogContent) is never attached to the actual trigger element—MorphingDialogTrigger uses its own optional triggerRef prop instead. This means triggerRef.current will remain null and focus restoration on close won’t work. Attach the context triggerRef to the trigger element (or merge the passed-in ref with the context ref).
| useEffect(() => { | ||
| setTimeout(() => { | ||
| setText('Copy') | ||
| }, 2000) | ||
| }, [text]) | ||
| setText("Copy"); | ||
| }, 2000); | ||
| }, [text]); |
There was a problem hiding this comment.
This effect sets a timeout on every text change but never clears the previous timeout. If the user clicks quickly after mount, an earlier timeout can overwrite "Copied" prematurely, and timeouts can run after unmount. Store the timeout id and clear it in the cleanup; also consider only scheduling the reset when text is "Copied".
| useEffect(() => { | ||
| if (!trigger) return | ||
| if (!trigger) return; | ||
|
|
||
| const intervalMs = interval * 1000 | ||
| const intervalMs = interval * 1000; | ||
| const timer = setInterval(() => { | ||
| setCurrentIndex((current) => { | ||
| const next = (current + 1) % items.length | ||
| onIndexChange?.(next) | ||
| return next | ||
| }) | ||
| }, intervalMs) | ||
| return () => clearInterval(timer) | ||
| }, [items.length, interval, onIndexChange, trigger]) | ||
| const next = (current + 1) % items.length; | ||
| onIndexChange?.(next); | ||
| return next; | ||
| }); | ||
| }, intervalMs); |
There was a problem hiding this comment.
If children is an empty array, items.length is 0 and (current + 1) % items.length becomes NaN, which will break rendering (items[currentIndex]). Guard against items.length === 0 (and possibly skip the interval when length <= 1).
Accepted incoming changes from the fix-build-perf-theme branch to resolve the build/performance/theme update workstream. Consolidated updates across layout, data, and UI to align with the incoming version.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
fonts.googleapis.com/home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/Rohan-Portfolio/Rohan-Portfolio/node_modules/.bin/../next/dist/bin/next build 53 -j ACCEPT(dns block)/opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node /home/REDACTED/work/Rohan-Portfolio/Rohan-Portfolio/node_modules/.pnpm/next@15.1.10_react-dom@19.2.4_react@19.2.4__react@19.2.4/node_modules/next/dist/server/lib/start-server.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.