Refactor: Extract SparkleEffect component#23
Conversation
Co-authored-by: codewithaman07 <135147451+codewithaman07@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the affirmations page by extracting the inline sparkle particle/canvas effect into a reusable client component, improving reuse and keeping page-level code focused on page behavior.
Changes:
- Added a new
SparkleEffectclient component underapp/components/. - Removed the inline
SparkleEffectimplementation fromapp/affirmations/page.tsxand replaced it with an import + usage. - Cleaned up React imports in
app/affirmations/page.tsxto match updated usage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/components/SparkleEffect.tsx | New extracted canvas particle effect component. |
| app/affirmations/page.tsx | Removes inline component and uses extracted SparkleEffect. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let animationId: number; | ||
| const animate = () => { | ||
| ctx.clearRect(0, 0, canvas.width, canvas.height); | ||
| let alive = false; | ||
|
|
||
| particles.forEach(p => { | ||
| p.x += p.vx; | ||
| p.y += p.vy; | ||
| p.vy += 0.1; | ||
| p.life -= 0.02; | ||
| p.opacity = Math.max(0, p.life); | ||
| p.size *= 0.99; | ||
|
|
||
| if (p.life > 0) { | ||
| alive = true; | ||
| ctx.save(); | ||
| ctx.globalAlpha = p.opacity; | ||
| ctx.fillStyle = p.color; | ||
| ctx.beginPath(); | ||
|
|
||
| // Draw star shape | ||
| const s = p.size; | ||
| for (let j = 0; j < 5; j++) { | ||
| const angle = (j * 4 * Math.PI) / 5 - Math.PI / 2; | ||
| const method = j === 0 ? 'moveTo' : 'lineTo'; | ||
| ctx[method]( | ||
| p.x + Math.cos(angle) * s, | ||
| p.y + Math.sin(angle) * s | ||
| ); | ||
| } | ||
| ctx.closePath(); | ||
| ctx.fill(); | ||
| ctx.restore(); | ||
| } | ||
| }); | ||
|
|
||
| if (alive) { | ||
| animationId = requestAnimationFrame(animate); | ||
| } | ||
| }; | ||
|
|
||
| animate(); | ||
| return () => cancelAnimationFrame(animationId); | ||
| }, [active]); |
There was a problem hiding this comment.
animationId is declared without an initializer and is always passed to cancelAnimationFrame in cleanup. Initializing it (e.g., to 0) and/or guarding the cleanup avoids any chance of calling cancelAnimationFrame with an unassigned value if the animation never schedules a frame.
| <canvas | ||
| ref={canvasRef} | ||
| className="absolute inset-0 pointer-events-none z-50" | ||
| style={{ width: '100%', height: '100%' }} |
There was a problem hiding this comment.
The canvas is purely decorative, but it isn't hidden from assistive tech. Consider adding aria-hidden="true" (or an appropriate accessible name if it conveys meaning) so screen readers don't announce an unlabeled canvas element.
| style={{ width: '100%', height: '100%' }} | |
| style={{ width: '100%', height: '100%' }} | |
| aria-hidden="true" |
| canvas.width = canvas.offsetWidth; | ||
| canvas.height = canvas.offsetHeight; |
There was a problem hiding this comment.
Canvas is sized using CSS pixels (offsetWidth/offsetHeight) without accounting for devicePixelRatio, which will render blurry on HiDPI/retina displays. Consider scaling the backing store by DPR (set width/height to cssSize * dpr and ctx.scale(dpr, dpr)) while keeping the CSS size unchanged.
| canvas.width = canvas.offsetWidth; | |
| canvas.height = canvas.offsetHeight; | |
| const dpr = (typeof window !== 'undefined' && window.devicePixelRatio) ? window.devicePixelRatio : 1; | |
| const rect = canvas.getBoundingClientRect(); | |
| canvas.width = rect.width * dpr; | |
| canvas.height = rect.height * dpr; | |
| ctx.setTransform(dpr, 0, 0, dpr, 0, 0); |
codewithaman07
deleted the
refactor/extract-sparkle-effect-10973287183591162512
branch
Extracted the SparkleEffect component from app/affirmations/page.tsx to app/components/SparkleEffect.tsx to improve code reusability and maintainability.
Verified that the component renders correctly using a temporary test page and confirmed with a screenshot.
Verified that the build passes with no errors.
PR created automatically by Jules for task 10973287183591162512 started by @codewithaman07