Snowfall v2 #62
Snowfall v2 #62
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an animated snowfall effect to the application as a visual enhancement. The implementation uses an HTML5 canvas with optimized rendering for a winter-themed aesthetic.
- Introduces a new
Snowfall.sveltecomponent with canvas-based particle animation - Integrates the snowfall effect into the root layout with proper z-index layering
- Includes accessibility support by hiding the animation when users prefer reduced motion
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/routes/+layout.svelte |
Imports and renders the Snowfall component, adds wrapper div with positioning for proper layering |
src/lib/components/Snowfall.svelte |
New component implementing canvas-based snowfall animation with lifecycle management and responsive behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return () => { | ||
| if (typeof cancelAnimationFrame === 'function') cancelAnimationFrame(raf); | ||
| window.removeEventListener('resize', handleResize); | ||
| }; |
There was a problem hiding this comment.
The cleanup function returned from onMount will not be executed. In Svelte, onMount does not support returning a cleanup function. The cleanup logic should be moved to the onDestroy hook instead, which already exists at lines 106-109. The current implementation has duplication between the onMount return statement and the onDestroy hook.
| return () => { | |
| if (typeof cancelAnimationFrame === 'function') cancelAnimationFrame(raf); | |
| window.removeEventListener('resize', handleResize); | |
| }; |
| dpr = Math.min(2, window.devicePixelRatio || 1); | ||
| ctx = canvas.getContext('2d'); | ||
| if (!ctx) return; | ||
| setCanvasSize(); |
There was a problem hiding this comment.
The handleResize function is called twice during initialization - once directly at line 97 and once again at line 96 via setCanvasSize (which sets dimensions), making the line 97 call redundant since handleResize already calls setCanvasSize at line 86. This causes unnecessary duplication of work during component initialization.
| setCanvasSize(); |
| } | ||
|
|
||
| function draw() { | ||
| if (!ctx) return; |
There was a problem hiding this comment.
The draw function is called unconditionally in every animation frame, even when the canvas size is zero or the context is invalid. While there's an early return if ctx is null, the function should also check if width or height are zero to avoid unnecessary work.
| if (!ctx) return; | |
| if (!ctx || width <= 0 || height <= 0) return; |
| setCanvasSize(); | ||
| handleResize(); | ||
| window.addEventListener('resize', handleResize, { passive: true }); | ||
| raf = requestAnimationFrame(loop); |
There was a problem hiding this comment.
The animation loop continues running even when the canvas is hidden due to prefers-reduced-motion. The animation should be paused or stopped when this media query is active to avoid unnecessary CPU usage and respect user preferences for reduced motion.
|
|
||
| function handleResize() { | ||
| setCanvasSize(); | ||
| const density = Math.min(220, Math.max(60, Math.floor((width * height) / 15000))); |
There was a problem hiding this comment.
The density calculation uses magic numbers (220, 60, 15000) without explanation. These values should be extracted as named constants with comments explaining their purpose and how they were chosen, improving code maintainability.
| function makeFlake(): Flake { | ||
| const r = Math.random() * 2.2 + 0.8; // 0.8 - 3.0 px | ||
| const speed = 0.4 + r * 0.35; // tie speed to size | ||
| return { | ||
| x: Math.random() * width, | ||
| y: -10 - Math.random() * height, | ||
| r, | ||
| vx: (Math.random() - 0.5) * 0.4, | ||
| vy: speed, | ||
| phase: Math.random() * Math.PI * 2, | ||
| opacity: 0.6 + Math.random() * 0.4 | ||
| }; |
There was a problem hiding this comment.
The makeFlake function uses magic numbers (2.2, 0.8, 0.4, 0.35, etc.) for snowflake properties without explanation. These should be extracted as named constants to improve code readability and make it easier to adjust the visual effect.
1 participant
No description provided.