search

loonywizard / js-confetti

JS Confetti library that supports emojis 🦄 🎉 ⚡️
https://loonywizard.github.io/js-confetti/
MIT License
1.08k stars 45 forks source link

Correctly position confetti source when confetti added immediately af… #38

Closed grork closed 2 years ago

grork commented 2 years ago

…ter instantiation

With the previous usage of canvas.width|.height, if confetti was added immediately after the canvas was created, the position of the confetti source was incorrect. This is because no layout pass had completed, and the defaults of 300x150 for the height/width of the canvas were applied (see: https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement).

Switching to getBoundingClientRect forces a layout pass in the browser, and returns the correct sizes immediately on creation.

grork commented 2 years ago

Note, I did not include a change to index.tsx that makes the original issue easy to repro, since it has a 1000 ms delay -- I wasn't sure if that was to aid in debugging/testing (e.g. you switch to the page after a reload), and thus changing it would harm 'quality of life'. I'm happy to update to requestAnimationFrame or remove the cancel-ability of the operation if the 1000ms delay wasn't intentional.

loonywizard commented 2 years ago

@grork thank you so much! I haven't seen this bug behaviour before you submitted this pull request and I tested the library without 1000ms delay

I will merge this and publish new version with other your contributions (after I will review them)

Speaking about 1000ms delay in site demo - it was added intentionally: users can see a blank page for a second, and only then see the confetti effect, so effect is kind of less aggressive