Fix buffer overflow issue: Clear previous canvas object on window resize #28
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughOn window resize, the module now performs a full teardown and reinitialization: it calls destroy(), clears particles, removes/recreates the canvas, rebinds context, reapplies styling, and recalculates layout and particle density. destroy() explicitly resets the particle array. Public API signatures remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Window
participant D as Daisy Module
participant DOM as Canvas DOM
participant P as Particles
User->>D: resize event
activate D
D->>D: destroy()
note right of D: Log teardown
D->>P: particles = []
D->>DOM: remove existing canvas (if present)
D->>DOM: create new canvas (.pg-canvas, display:block)
D->>D: ctx = canvas.getContext("2d")
D->>DOM: insert canvas into container
D->>D: apply styling and layout calculations
D->>P: recalc density, reindex/reseed particles
deactivate D
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
daisy.js (1)
365-373: Make destroy() safe for internal use and null-guard canvas removalAdd an internal flag so resize can reuse destroy without nuking hooks or $.data. Also guard canvas removal to avoid NPEs.
-function destroy() { - console.log('destroy'); - particles = []; - canvas.parentNode.removeChild(canvas); - hook('onDestroy'); - if ($) { - $(element).removeData('plugin_' + pluginName); - } - } +function destroy(internal) { + if (!internal) { console.log('destroy'); } + particles = []; + if (canvas && canvas.parentNode) { + canvas.parentNode.removeChild(canvas); + } + if (!internal) { + hook('onDestroy'); + if ($) { + $(element).removeData('plugin_' + pluginName); + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
daisy.js(2 hunks)
🔇 Additional comments (1)
daisy.js (1)
367-367: Clearing particle state on teardown is correctResetting particles prevents stale references and ensures density is recalculated cleanly after resize.
| destroy(); | ||
| //Create canvas | ||
| canvas = document.createElement('canvas'); | ||
| canvas.className = 'pg-canvas'; | ||
| canvas.style.display = 'block'; | ||
| element.insertBefore(canvas, element.firstChild); | ||
| ctx = canvas.getContext('2d'); | ||
|
|
There was a problem hiding this comment.
Avoid full destroy() on resize — it breaks the jQuery plugin handle and fires onDestroy on every resize
Calling destroy() here removes
Apply:
- destroy();
+ // Internal teardown: keep plugin handle and skip hooks
+ destroy(true);Optional: if you see flicker or transient errors while the canvas is detached, pause around the rebuild (cancelAnimationFrame/resume) or debounce resize with rAF.
📝 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.
| destroy(); | |
| //Create canvas | |
| canvas = document.createElement('canvas'); | |
| canvas.className = 'pg-canvas'; | |
| canvas.style.display = 'block'; | |
| element.insertBefore(canvas, element.firstChild); | |
| ctx = canvas.getContext('2d'); | |
| // Internal teardown: keep plugin handle and skip hooks | |
| destroy(true); | |
| //Create canvas | |
| canvas = document.createElement('canvas'); | |
| canvas.className = 'pg-canvas'; | |
| canvas.style.display = 'block'; | |
| element.insertBefore(canvas, element.firstChild); | |
| ctx = canvas.getContext('2d'); |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Previous unmerged PR: jnicol/particleground#34
On window resize, the previous canvas object was not being cleared
Summary by CodeRabbit
Bug Fixes
Refactor