Add Argus AI project, update resume, and revamp boot screen#2
Add Argus AI project, update resume, and revamp boot screen#2sisodiajatin wants to merge 2 commits into
Conversation
- Add Argus AI Code Review Assistant as a new project with description, GitHub link, and screenshot - Update resume PDF to Jatin_Sisodia-SE.pdf and wire up download link - Replace Apple-style boot screen with retro BIOS/CRT boot animation - Switch desktop icons from absolute positioning to flex column layout - Reorder project folder positions on desktop Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces the boot UI with a staged, per-character animated boot sequence; adds Spline-based live wallpaper, new UI primitives (Card, Spotlight, SplineSceneBasic, Spline loader), Tailwind class utilities and CRT-style CSS, adds Argus project entry, updates dependencies and resume PDF reference, and minor Home component class changes. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant BootScreen
participant DOM as ScreenRef
participant GSAP
participant Main as AppMain
App->>BootScreen: mount + start boot sequence
BootScreen->>DOM: render phased lines (typing per-character)
BootScreen->>DOM: update memtest progress & scroll
BootScreen-->>BootScreen: detect sequence complete
BootScreen->>GSAP: trigger fade animation on ScreenRef
GSAP->>DOM: animate opacity/fade-out
GSAP->>App: onComplete callback
App->>Main: set main opacity → 1 (reveal UI)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| } | ||
| }, [completedLines, typingLine, memValue]); | ||
|
|
||
| const sleep = useCallback( |
There was a problem hiding this comment.
🟡 Potential Memory Leak in sleep Function
⚡ Performance | Severity: warning
The sleep function uses setTimeout and setInterval, which can cause memory leaks if not properly cleared. Although clearInterval and clearTimeout are used, it's essential to ensure they are always called to prevent potential issues.
Suggested fix:
Consider using a more modern approach to handle asynchronous delays, such as using async/await with Promises.
| const bottomRef = useRef(null); | ||
| const abortRef = useRef(false); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🟡 Interval Not Cleared in Error Cases
⚡ Performance | Severity: warning
The interval for toggling the cursor is not cleared in case of an error or when the component is unmounted. This could lead to memory leaks or unexpected behavior.
Suggested fix:
Ensure that the interval is cleared in all cases, including when the component is unmounted or an error occurs.
| }, | ||
|
|
||
| // ▶ Project 3 | ||
| { |
There was a problem hiding this comment.
🟢 Potential Information Disclosure
🔒 Security | Severity: suggestion
The addition of a new project with a link to a GitHub repository may potentially disclose sensitive information if the repository is not properly secured.
Suggested fix:
Ensure that the linked repository does not contain sensitive information and is properly secured.
| fileType: "pdf", | ||
| // you can add `href` if you want to open a hosted resume | ||
| // href: "/your/resume/path.pdf", | ||
| href: "/files/Jatin_Sisodia-SE.pdf", |
There was a problem hiding this comment.
🟢 Resume File Link
🔒 Security | Severity: suggestion
The resume file link is now hosted locally. Ensure that the file does not contain sensitive information and is properly secured.
| href: "/files/Jatin_Sisodia-SE.pdf", | |
| Review the resume file for sensitive information and consider hosting it securely or using a different method for sharing it. |
| @apply fixed top-12 left-4 bottom-20 z-0 max-sm:hidden; | ||
|
|
||
| ul { | ||
| @apply flex flex-col gap-2 h-full content-start; |
There was a problem hiding this comment.
🟢 CSS Class Naming Convention
✨ Style | Severity: suggestion
The CSS class names could be more descriptive and follow a consistent naming convention. For example, instead of .bios-gray, consider using a more descriptive name like .bios-text-gray.
Suggested fix:
Consider renaming CSS classes to follow a consistent naming convention, e.g., .bios-text-gray instead of .bios-gray.
| @apply fixed inset-0 z-[9999] bg-black flex flex-col items-center justify-center gap-8; | ||
| @apply fixed inset-0 z-[9999]; | ||
| background: #0a0a0e; | ||
| animation: crt-flicker 0.15s ease-in-out 0.3s 1; |
There was a problem hiding this comment.
🟡 Animation Performance
⚡ Performance | Severity: warning
The animation crt-flicker may cause performance issues if not optimized properly. Consider using will-change property or optimizing the animation to reduce the number of repaints.
Suggested fix:
Add will-change property to the animated element or optimize the animation to reduce the number of repaints.
| <h2>Resume.pdf</h2> | ||
|
|
||
| <a href="files/Jatin_Sisodia.pdf" download className="cursor-pointer" title="Download Resume"> | ||
| <a href="files/Jatin_Sisodia-SE.pdf" download className="cursor-pointer" title="Download Resume"> |
There was a problem hiding this comment.
🟡 Potential Download Link Issue
🐛 Bug | Severity: warning
The download link may not work as expected if the file path is incorrect. Consider using a more robust way to handle file downloads.
Suggested fix:
Verify the file path and consider using a library or a more robust way to handle file downloads.
| <li | ||
| key={project.id} | ||
| className={clsx("group folder", project.windowPosition)} | ||
| className="group folder" |
There was a problem hiding this comment.
🟢 Class Name Removal
✨ Style | Severity: suggestion
The clsx import is removed, but the className attribute is still used. Consider removing or replacing the className attribute to avoid confusion.
Suggested fix:
Remove or replace the className attribute to avoid confusion.
| "@types/react": "^19.2.5", | ||
| "@types/react-dom": "^19.2.3", | ||
| "@vitejs/plugin-react": "^5.1.1", | ||
| "baseline-browser-mapping": "^2.10.0", |
There was a problem hiding this comment.
🟢 Unused Dependency
✨ Style | Severity: suggestion
The baseline-browser-mapping dependency may not be used in the project. Consider removing unused dependencies to keep the project clean.
Suggested fix:
Remove unused dependencies to keep the project clean.
👁️ Argus Code ReviewPR SummaryWhat this PR doesThis PR introduces a new Argus AI project, updates the resume, and revamps the boot screen to mimic a retro BIOS boot sequence. The boot screen now displays a simulated boot process with various system checks and loading animations. The changes aim to enhance the overall user experience and add a unique touch to the portfolio. Key changes
Notable decisionsThe decision to use a retro BIOS boot sequence as the new boot screen design is notable, as it adds a unique and nostalgic touch to the portfolio. The use of a simulated system check and loading animation also enhances the overall user experience. Status: 🟡 Minor issues found
Top Findings
SummaryCode Review SummaryThe pull request introduces significant changes to the codebase, including the addition of the Argus AI project, updates to the resume, and a revamp of the boot screen. Overall, the changes are substantial, but there are several issues that need to be addressed to ensure the code is maintainable, efficient, and reliable. Key Issues
Suggestions for Improvement
Positive Observations
Reviewed commit |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/BootScreen.jsx (1)
204-204: Consider stabilizingonCompleteto prevent boot sequence restarts.If the parent component doesn't memoize
onComplete, every parent re-render creates a new function reference, causing this effect to re-run and restart the entire boot sequence from the beginning.♻️ Option 1: Use a ref to store the callback
+ const onCompleteRef = useRef(onComplete); + useEffect(() => { + onCompleteRef.current = onComplete; + }, [onComplete]); useEffect(() => { abortRef.current = false; // ... rest of effect gsap.to(screenRef.current, { opacity: 0, duration: 0.4, ease: "power2.inOut", - onComplete: () => onComplete?.(), + onComplete: () => onCompleteRef.current?.(), }); // ... - }, [onComplete, sleep]); + }, [sleep]);♻️ Option 2: Document that parent should memoize the callback
Add a comment at the component definition:
/** * `@param` {Object} props * `@param` {() => void} props.onComplete - Called when boot animation completes. * Should be memoized (useCallback) to prevent animation restarts on parent re-renders. */ const BootScreen = ({ onComplete }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/BootScreen.jsx` at line 204, The effect in BootScreen re-runs when the parent passes a new onComplete reference; fix by stabilizing the callback: inside BootScreen create a ref (e.g., onCompleteRef) and update it in a small useEffect when the onComplete prop changes, then use onCompleteRef.current inside the boot sequence effect so the effect's dependency list can omit onComplete (keep sleep if needed); reference the BootScreen component, the effect that currently lists [onComplete, sleep], and the new onCompleteRef to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 38: Remove the redundant devDependency "baseline-browser-mapping" from
package.json (the entry "baseline-browser-mapping": "^2.10.0") because it is
provided transitively by browserslist; delete that dependency entry from the
devDependencies block, then update the lockfile by running your package manager
(e.g., npm install or npm ci) and run the test/build pipeline to verify nothing
breaks; keep a note that browserslist is the transitive provider so no code
changes are required.
In `@src/components/BootScreen.jsx`:
- Around line 102-115: The sleep function (useCallback named sleep) leaks
intervals because cid is only cleared when abortRef.current is true; update the
Promise executor to always clear the interval and timeout when resolving: on
normal timeout resolution clearInterval(cid) and clearTimeout(tid) before
resolve, and in the abort branch also clear both; ensure both tid and cid are
referenced in both paths so the interval is not left running after the Promise
resolves in BootScreen.jsx (sleep, abortRef, tid, cid).
In `@src/windows/Resume.jsx`:
- Around line 21-25: Update the resume file paths to use the absolute path with
a leading slash to match RESUME_LOCATION; specifically change the anchor href
(the <a> with title "Download Resume") and the Document component's file prop
(Document file="...") from "files/Jatin_Sisodia-SE.pdf" to
"/files/Jatin_Sisodia-SE.pdf" so links work reliably across routes and
deployments.
---
Nitpick comments:
In `@src/components/BootScreen.jsx`:
- Line 204: The effect in BootScreen re-runs when the parent passes a new
onComplete reference; fix by stabilizing the callback: inside BootScreen create
a ref (e.g., onCompleteRef) and update it in a small useEffect when the
onComplete prop changes, then use onCompleteRef.current inside the boot sequence
effect so the effect's dependency list can omit onComplete (keep sleep if
needed); reference the BootScreen component, the effect that currently lists
[onComplete, sleep], and the new onCompleteRef to locate and change the code.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/files/Jatin_Sisodia-SE.pdfis excluded by!**/*.pdfpublic/images/argus.pngis excluded by!**/*.png
📒 Files selected for processing (6)
package.jsonsrc/components/BootScreen.jsxsrc/components/Home.jsxsrc/constants/index.jssrc/index.csssrc/windows/Resume.jsx
| "@types/react": "^19.2.5", | ||
| "@types/react-dom": "^19.2.3", | ||
| "@vitejs/plugin-react": "^5.1.1", | ||
| "baseline-browser-mapping": "^2.10.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for any direct imports or usage of baseline-browser-mapping
rg "baseline-browser-mapping" --type js --type ts --type json --type mjs --type cjs -g '!node_modules' -g '!dist' -g '!build'Repository: sisodiajatin/MacOS_Portfolio
Length of output: 99
🏁 Script executed:
# Also search for patterns that might reference it indirectly
rg "baseline" --type js --type ts --type json -g '!node_modules' -g '!dist' -g '!build'Repository: sisodiajatin/MacOS_Portfolio
Length of output: 520
🏁 Script executed:
# Check the package.json file structure to understand context
cat package.json | head -60Repository: sisodiajatin/MacOS_Portfolio
Length of output: 1277
🏁 Script executed:
# Check for vite and eslint config files that might use this
fd -e "config.js" -e "config.ts" -e "eslintrc*" -e ".eslintrc*" -e "vite.config.*" | head -20Repository: sisodiajatin/MacOS_Portfolio
Length of output: 103
🏁 Script executed:
# Check if baseline-browser-mapping is a peer dependency of any installed package
rg "baseline-browser-mapping" package-lock.json -A 5 -B 5 | head -100Repository: sisodiajatin/MacOS_Portfolio
Length of output: 1374
🏁 Script executed:
# Look for any config files in the root
ls -la | grep -E "vite|eslint|prettier|tsconfig"Repository: sisodiajatin/MacOS_Portfolio
Length of output: 205
🏁 Script executed:
# Check eslint.config.js for browserslist usage
cat eslint.config.jsRepository: sisodiajatin/MacOS_Portfolio
Length of output: 829
🏁 Script executed:
# Check vite.config.js for browserslist usage
cat vite.config.jsRepository: sisodiajatin/MacOS_Portfolio
Length of output: 1445
🏁 Script executed:
# Search for browserslist usage in the codebase
rg "browserslist" --type js --type ts --type json -g '!node_modules'Repository: sisodiajatin/MacOS_Portfolio
Length of output: 1107
Remove the redundant baseline-browser-mapping devDependency.
This package is not directly used anywhere in the codebase and is already installed as a transitive dependency of browserslist. Explicitly listing it in devDependencies is unnecessary since modern npm automatically installs transitive dependencies. Removing it will simplify the dependency tree and reduce the package.json footprint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 38, Remove the redundant devDependency
"baseline-browser-mapping" from package.json (the entry
"baseline-browser-mapping": "^2.10.0") because it is provided transitively by
browserslist; delete that dependency entry from the devDependencies block, then
update the lockfile by running your package manager (e.g., npm install or npm
ci) and run the test/build pipeline to verify nothing breaks; keep a note that
browserslist is the transitive provider so no code changes are required.
| const sleep = useCallback( | ||
| (ms) => | ||
| new Promise((resolve) => { | ||
| const tid = setTimeout(resolve, ms); | ||
| const cid = setInterval(() => { | ||
| if (abortRef.current) { | ||
| clearTimeout(tid); | ||
| clearInterval(cid); | ||
| resolve(); | ||
| } | ||
| }, 50); | ||
| }), | ||
| [] | ||
| ); |
There was a problem hiding this comment.
Memory leak: interval not cleared on normal promise resolution.
The sleep function creates an interval (cid) that checks for abort every 50ms, but this interval is only cleared when abortRef.current becomes true. On normal timeout completion, the interval continues running until the component unmounts, accumulating leaked intervals.
🐛 Proposed fix
const sleep = useCallback(
(ms) =>
new Promise((resolve) => {
- const tid = setTimeout(resolve, ms);
+ const tid = setTimeout(() => {
+ clearInterval(cid);
+ resolve();
+ }, ms);
const cid = setInterval(() => {
if (abortRef.current) {
clearTimeout(tid);
clearInterval(cid);
resolve();
}
}, 50);
}),
[]
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/BootScreen.jsx` around lines 102 - 115, The sleep function
(useCallback named sleep) leaks intervals because cid is only cleared when
abortRef.current is true; update the Promise executor to always clear the
interval and timeout when resolving: on normal timeout resolution
clearInterval(cid) and clearTimeout(tid) before resolve, and in the abort branch
also clear both; ensure both tid and cid are referenced in both paths so the
interval is not left running after the Promise resolves in BootScreen.jsx
(sleep, abortRef, tid, cid).
| <a href="files/Jatin_Sisodia-SE.pdf" download className="cursor-pointer" title="Download Resume"> | ||
| <Download className="icon"/> | ||
| </a> | ||
| </div> | ||
| <Document file="files/Jatin_Sisodia.pdf" > | ||
| <Document file="files/Jatin_Sisodia-SE.pdf" > |
There was a problem hiding this comment.
Add leading slash to file paths for consistency and reliability.
The paths files/Jatin_Sisodia-SE.pdf are relative, but RESUME_LOCATION in constants uses the absolute path /files/Jatin_Sisodia-SE.pdf. Relative paths can break when navigating from different routes or if the app is deployed to a subdirectory.
🔧 Proposed fix
- <a href="files/Jatin_Sisodia-SE.pdf" download className="cursor-pointer" title="Download Resume">
+ <a href="/files/Jatin_Sisodia-SE.pdf" download className="cursor-pointer" title="Download Resume">
<Download className="icon"/>
</a>
</div>
- <Document file="files/Jatin_Sisodia-SE.pdf" >
+ <Document file="/files/Jatin_Sisodia-SE.pdf" >📝 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.
| <a href="files/Jatin_Sisodia-SE.pdf" download className="cursor-pointer" title="Download Resume"> | |
| <Download className="icon"/> | |
| </a> | |
| </div> | |
| <Document file="files/Jatin_Sisodia.pdf" > | |
| <Document file="files/Jatin_Sisodia-SE.pdf" > | |
| <a href="/files/Jatin_Sisodia-SE.pdf" download className="cursor-pointer" title="Download Resume"> | |
| <Download className="icon"/> | |
| </a> | |
| </div> | |
| <Document file="/files/Jatin_Sisodia-SE.pdf" > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/windows/Resume.jsx` around lines 21 - 25, Update the resume file paths to
use the absolute path with a leading slash to match RESUME_LOCATION;
specifically change the anchor href (the <a> with title "Download Resume") and
the Document component's file prop (Document file="...") from
"files/Jatin_Sisodia-SE.pdf" to "/files/Jatin_Sisodia-SE.pdf" so links work
reliably across routes and deployments.
Swap shader-based background for a Spline 3D scene, add UI components (card, spotlight), smooth boot-to-desktop transition with opacity fade, and adjust CSS animations for the new background. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| } | ||
| }, [completedLines, typingLine, memValue]); | ||
|
|
||
| const sleep = useCallback( |
There was a problem hiding this comment.
🟡 Potential Memory Leak in sleep Function
⚡ Performance | Severity: warning
The sleep function uses setTimeout and setInterval which can cause memory leaks if not properly cleared. Although clearInterval and clearTimeout are used, it's essential to ensure they are always called to prevent potential memory leaks.
Suggested fix:
Ensure that clearInterval and clearTimeout are always called, even in error cases. Consider using a try-finally block to guarantee cleanup.
| const bottomRef = useRef(null); | ||
| const abortRef = useRef(false); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🟢 Interval for Cursor Blinking Could Be Optimized
⚡ Performance | Severity: suggestion
The interval for blinking the cursor is set to 530ms. This could potentially be optimized for better performance or to make the blinking more consistent with typical terminal experiences.
| useEffect(() => { | |
| Consider adjusting the interval for better consistency or performance. For example, using a more standard blink rate like 500ms could improve the user experience. |
| const spotlightLeft = useTransform(mouseX, (x) => `${x - size / 2}px`); | ||
| const spotlightTop = useTransform(mouseY, (y) => `${y - size / 2}px`); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🟢 Potential for Code Duplication in useEffect
✨ Style | Severity: suggestion
The useEffect hook sets up event listeners and then removes them. While this is good practice, there's a potential for code duplication if similar setups are needed elsewhere. Consider extracting a function for adding and removing event listeners.
| useEffect(() => { | |
| Extract a separate function for adding and removing event listeners to avoid potential duplication and improve maintainability. |
| }, | ||
|
|
||
| // ▶ Project 3 | ||
| { |
There was a problem hiding this comment.
🟢 Deeply Nested Objects in Constants
✨ Style | Severity: suggestion
The constants file contains deeply nested objects which can be hard to read and maintain. Consider flattening the structure or breaking it into smaller, more manageable pieces.
Suggested fix:
Consider breaking down deeply nested objects into smaller constants or using a data structure that is easier to manage and extend.
| import { SplineScene } from '#components/ui/splite'; | ||
|
|
||
| const LiveWallpaper = ({ darkMode }) => { | ||
| const LiveWallpaper = () => { |
There was a problem hiding this comment.
🟡 Removed darkMode prop
🐛 Bug | Severity: warning
The darkMode prop has been removed from the LiveWallpaper component. This might cause issues if the component relies on this prop for its functionality.
| const LiveWallpaper = () => { | |
| Add the darkMode prop back to the LiveWallpaper component if it's still needed: | |
| ```jsx | |
| const LiveWallpaper = ({ darkMode }) => { |
| enableTransition={false} | ||
| /> | ||
| </ShaderGradientCanvas> | ||
| <div className="live-wallpaper spline-bg"> |
There was a problem hiding this comment.
🟢 New SplineScene component
✨ Style | Severity: suggestion
The new SplineScene component is used without any error handling. It's a good practice to add error handling for third-party components.
| <div className="live-wallpaper spline-bg"> | |
| Add error handling for the SplineScene component: | |
| ```jsx | |
| try { | |
| return <SplineScene scene="https://prod.spline.design/kZDDjO5HuC9GJUM2/scene.splinecode" className="w-full h-full" /> | |
| } catch (error) { | |
| console.error(error) | |
| return <div>Error loading SplineScene</div> | |
| } |
| import { Suspense, lazy } from 'react' | ||
| const Spline = lazy(() => import('@splinetool/react-spline')) | ||
|
|
||
| export function SplineScene({ scene, className }) { |
There was a problem hiding this comment.
🟡 Lazy loading of Spline component
⚡ Performance | Severity: warning
The Spline component is lazy loaded using the Suspense component. This might cause performance issues if the component is not properly optimized.
| export function SplineScene({ scene, className }) { | |
| Optimize the Spline component for lazy loading: | |
| ```jsx | |
| const Spline = lazy(() => import('@splinetool/react-spline')) |
|
|
||
| @import "tailwindcss"; | ||
|
|
||
| @keyframes spotlight { |
There was a problem hiding this comment.
🟢 New CSS keyframes animation
✨ Style | Severity: suggestion
The new CSS keyframes animation is added without any comments or explanations. It's a good practice to add comments for complex CSS code.
Suggested fix:
Add comments to explain the purpose of the keyframes animation:
/* Spotlight animation */
@keyframes spotlight {
/* ... */
}|
|
||
| <main | ||
| className={darkMode ? "dark-mode" : ""} | ||
| style={{ |
There was a problem hiding this comment.
🟢 New inline styles
✨ Style | Severity: suggestion
The new inline styles are added to the main element. It's a good practice to avoid using inline styles and instead use CSS classes.
Suggested fix:
Replace the inline styles with a CSS class:
<main className="main" onContextMenu={handleContextMenu}>
/* ... */
</main>There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/BootScreen.jsx (1)
102-113:⚠️ Potential issue | 🔴 Critical
sleepstill leaks intervals on normal completion.The interval is cleared only when
abortRef.currentbecomes true. On normal timeout resolution it keeps running, and this accumulates quickly during the boot sequence.🐛 Suggested fix
const sleep = useCallback( (ms) => new Promise((resolve) => { - const tid = setTimeout(resolve, ms); - const cid = setInterval(() => { - if (abortRef.current) { - clearTimeout(tid); - clearInterval(cid); - resolve(); - } - }, 50); + let settled = false; + const finish = () => { + if (settled) return; + settled = true; + clearTimeout(tid); + clearInterval(cid); + resolve(); + }; + const tid = setTimeout(finish, ms); + const cid = setInterval(() => { + if (abortRef.current) finish(); + }, 50); }), [] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/BootScreen.jsx` around lines 102 - 113, The sleep function's interval (cid) isn't cleared when the timeout completes normally, leaking intervals; update the useCallback sleep implementation so that before resolving on normal timeout it clears the interval (clearInterval(cid)) and also ensure both the timeout (tid) and interval (cid) are cleared when abortRef.current is true; locate the sleep function (the useCallback using abortRef and setTimeout/setInterval) and add clearInterval(cid) in the normal resolution path and keep the existing clearTimeout/clearInterval in the abort branch.
🧹 Nitpick comments (3)
src/components/ui/spotlight.jsx (1)
21-30: Avoid permanent parent style mutation from insideSpotlight.This mutates the parent container’s layout/clipping and never restores previous values on unmount, which can cause side effects outside this component.
♻️ Suggested fix
useEffect(() => { - if (containerRef.current) { - const parent = containerRef.current.parentElement; - if (parent) { - parent.style.position = 'relative'; - parent.style.overflow = 'hidden'; - setParentElement(parent); - } - } + if (!containerRef.current) return; + const parent = containerRef.current.parentElement; + if (!parent) return; + + const prevPosition = parent.style.position; + const prevOverflow = parent.style.overflow; + + if (!parent.style.position) parent.style.position = 'relative'; + if (!parent.style.overflow) parent.style.overflow = 'hidden'; + setParentElement(parent); + + return () => { + parent.style.position = prevPosition; + parent.style.overflow = prevOverflow; + }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/spotlight.jsx` around lines 21 - 30, Spotlight's useEffect mutates parent DOM styles (position and overflow) permanently; capture the parent's previous style values when setting parent.style.position and parent.style.overflow in the effect (using containerRef.current.parentElement), then in the effect cleanup restore those previous values (and clear setParentElement if needed); only set the styles if parent exists and ensure cleanup runs to revert to original position/overflow to avoid leaking layout/clipping changes outside the Spotlight component.src/components/ui/SplineSceneBasic.jsx (2)
7-36: Consider accepting props for reusability.The component has a generic name suggesting reuse, but all content (title, description, scene URL) is hardcoded. If this is a one-off demo, that's fine. Otherwise, accepting props would make it more flexible.
♻️ Suggested props interface for reusability
-export function SplineSceneBasic() { +export function SplineSceneBasic({ + title = "Interactive 3D", + description = "Bring your UI to life with beautiful 3D scenes. Create immersive experiences that capture attention and enhance your design.", + sceneUrl = "https://prod.spline.design/kZDDjO5HuC9GJUM2/scene.splinecode", + className, +}) { return ( - <Card className="w-full h-[500px] bg-black/[0.96] relative overflow-hidden"> + <Card className={cn("w-full h-[500px] bg-black/[0.96] relative overflow-hidden", className)}> <Spotlight className="-top-40 left-0 md:left-60 md:-top-20" fill="white" /> <div className="flex h-full"> {/* Left content */} <div className="flex-1 p-8 relative z-10 flex flex-col justify-center"> <h1 className="text-4xl md:text-5xl font-bold bg-clip-text text-transparent bg-gradient-to-b from-neutral-50 to-neutral-400"> - Interactive 3D + {title} </h1> <p className="mt-4 text-neutral-300 max-w-lg"> - Bring your UI to life with beautiful 3D scenes. Create immersive experiences - that capture attention and enhance your design. + {description} </p> </div> {/* Right content */} <div className="flex-1 relative"> <SplineScene - scene="https://prod.spline.design/kZDDjO5HuC9GJUM2/scene.splinecode" + scene={sceneUrl} className="w-full h-full" /> </div> </div> </Card> ) }You'll also need to import
cnfrom#lib/utilsif you add the className prop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/SplineSceneBasic.jsx` around lines 7 - 36, The SplineSceneBasic component currently hardcodes title, description and scene URL; make it reusable by adding props (e.g., title, description, scene, className) to the SplineSceneBasic function signature, provide sensible defaults for each prop, replace the hardcoded strings/URL with those props inside the JSX, and merge incoming className with the existing Card/SplineScene classes (import and use cn from '#lib/utils' if you accept className). Ensure the prop names are exported/typed if using TypeScript and preserve existing layout and accessibility.
3-3: Consider renamingsplite.jsxtospline.jsxfor clarity.The file
src/components/ui/splite.jsxexists and is consistently imported across the codebase (used in 2 places). However, the naming appears unusual and doesn't align with the actual Spline 3D library. Renaming tospline.jsxwould improve maintainability and reduce confusion for future developers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/SplineSceneBasic.jsx` at line 3, Rename the component file currently named splite.jsx to spline.jsx and update all imports accordingly: change the import in SplineSceneBasic.jsx from "#components/ui/splite" to "#components/ui/spline" (and update the other two places that import splite.jsx), then ensure the component's default export name (if any) inside the file remains unchanged so consumers continue to work; commit the file rename so version control records the move and run the app/tests to verify no import paths remain referencing "splite".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.jsx`:
- Around line 96-104: Right-clicks on the background miss handleContextMenu
because the <main> element is non-hit-testable while the LiveWallpaper layer is
hit-testable; fix by either moving the onContextMenu={handleContextMenu} up to
the enclosing wrapper (the root div that contains <LiveWallpaper /> and <main>)
so it receives background right-clicks, or make the <main> actually hit-testable
(remove pointer-events: none in the CSS rule that targets main or add
pointer-events: auto) so the existing onContextMenu on <main> fires; update the
App.jsx to attach the handler to the wrapper or adjust the CSS, and keep
handleContextMenu and LiveWallpaper references unchanged.
---
Duplicate comments:
In `@src/components/BootScreen.jsx`:
- Around line 102-113: The sleep function's interval (cid) isn't cleared when
the timeout completes normally, leaking intervals; update the useCallback sleep
implementation so that before resolving on normal timeout it clears the interval
(clearInterval(cid)) and also ensure both the timeout (tid) and interval (cid)
are cleared when abortRef.current is true; locate the sleep function (the
useCallback using abortRef and setTimeout/setInterval) and add
clearInterval(cid) in the normal resolution path and keep the existing
clearTimeout/clearInterval in the abort branch.
---
Nitpick comments:
In `@src/components/ui/SplineSceneBasic.jsx`:
- Around line 7-36: The SplineSceneBasic component currently hardcodes title,
description and scene URL; make it reusable by adding props (e.g., title,
description, scene, className) to the SplineSceneBasic function signature,
provide sensible defaults for each prop, replace the hardcoded strings/URL with
those props inside the JSX, and merge incoming className with the existing
Card/SplineScene classes (import and use cn from '#lib/utils' if you accept
className). Ensure the prop names are exported/typed if using TypeScript and
preserve existing layout and accessibility.
- Line 3: Rename the component file currently named splite.jsx to spline.jsx and
update all imports accordingly: change the import in SplineSceneBasic.jsx from
"#components/ui/splite" to "#components/ui/spline" (and update the other two
places that import splite.jsx), then ensure the component's default export name
(if any) inside the file remains unchanged so consumers continue to work; commit
the file rename so version control records the move and run the app/tests to
verify no import paths remain referencing "splite".
In `@src/components/ui/spotlight.jsx`:
- Around line 21-30: Spotlight's useEffect mutates parent DOM styles (position
and overflow) permanently; capture the parent's previous style values when
setting parent.style.position and parent.style.overflow in the effect (using
containerRef.current.parentElement), then in the effect cleanup restore those
previous values (and clear setParentElement if needed); only set the styles if
parent exists and ensure cleanup runs to revert to original position/overflow to
avoid leaking layout/clipping changes outside the Spotlight component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 982e0af2-bed4-4204-b93d-a64ab26ff37d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
index.htmlpackage.jsonsrc/App.jsxsrc/components/BootScreen.jsxsrc/components/LiveWallpaper.jsxsrc/components/ui/SplineSceneBasic.jsxsrc/components/ui/card.jsxsrc/components/ui/splite.jsxsrc/components/ui/spotlight.jsxsrc/index.csssrc/lib/utils.js
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
| <LiveWallpaper /> | ||
|
|
||
| <main | ||
| className={darkMode ? "dark-mode" : ""} | ||
| style={{ | ||
| opacity: isBooting ? 0 : 1, | ||
| transition: "opacity 0.6s ease-in-out", | ||
| }} | ||
| onContextMenu={handleContextMenu} |
There was a problem hiding this comment.
Desktop context menu can miss empty-background right-clicks.
onContextMenu is attached to <main> (Line 104), but main is non-hit-testable in CSS (src/index.css Line 70) while the Spline wallpaper is hit-testable (src/index.css Lines 39-46). Result: right-clicks on empty desktop/background bypass this handler.
💡 Suggested fix
--- a/src/App.jsx
+++ b/src/App.jsx
@@
- <LiveWallpaper />
+ <LiveWallpaper onContextMenu={handleContextMenu} />--- a/src/components/LiveWallpaper.jsx
+++ b/src/components/LiveWallpaper.jsx
@@
-const LiveWallpaper = () => {
+const LiveWallpaper = ({ onContextMenu }) => {
@@
- <div className="live-wallpaper spline-bg">
+ <div className="live-wallpaper spline-bg" onContextMenu={onContextMenu}>
<SplineScene
scene="https://prod.spline.design/kZDDjO5HuC9GJUM2/scene.splinecode"
className="w-full h-full"
/>
</div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.jsx` around lines 96 - 104, Right-clicks on the background miss
handleContextMenu because the <main> element is non-hit-testable while the
LiveWallpaper layer is hit-testable; fix by either moving the
onContextMenu={handleContextMenu} up to the enclosing wrapper (the root div that
contains <LiveWallpaper /> and <main>) so it receives background right-clicks,
or make the <main> actually hit-testable (remove pointer-events: none in the CSS
rule that targets main or add pointer-events: auto) so the existing
onContextMenu on <main> fires; update the App.jsx to attach the handler to the
wrapper or adjust the CSS, and keep handleContextMenu and LiveWallpaper
references unchanged.
Summary
Jatin_Sisodia-SE.pdfand fixed download linksTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Updates