search

ConnorAtherton / walkway

An easy way to animate SVG elements.
http://connoratherton.com/walkway
MIT License
4.37k stars 229 forks source link

feat(path): add viewbox scaling #52

Closed brycedorn closed 6 years ago

brycedorn commented 6 years ago

hi! 👋 love the lib @ConnorAtherton - makes for some really cool animations of existing svg's with minimal effort.

one issue that I came across was that when using a svg with a predefined viewbox & resizing larger than that viewbox, the path length remained at the viewbox scale leading to incomplete drawings. to remedy this, I added a check for a viewbox definition in the the Path constructor, which then can get this scale & use it to calculate length.

let me know if this looks ok! I didn't test any line/polylines so only added it to path for now. also bumped the version & rebuilt (still on 0.0.6 on npm, would be great if you could bump it afterward 🙂).

ConnorAtherton commented 6 years ago

@brycedorn Thanks for the contribution. I remember someone mentioning this to me in passing a few months after I first released this, but I forgot all about it until seeing this. Let me know what you think of the comment I added.

Also, oversight on 0.0.7 not being published yet. I just published that version now ready for these changes to be merged and published.

brycedorn commented 6 years ago

hey, thanks for the quick reply!

I pulled it out into a function per your suggestion - just didn't have any viewbox'd polyline or lines so didn't think to update those as well. but it's better to keep things consistent 👍 let me know if there's anything else!

ConnorAtherton commented 6 years ago

This looks great, thanks for taking the extra time to make this consistent across all the constructors. I'll comment back here after I have published this to npm for you.

ConnorAtherton commented 6 years ago

walkway.js@0.0.8 is up 👍

brycedorn commented 6 years ago

thanks for much for the fast turnaround! really appreciated 🐝

idlesHand commented 6 years ago

The new update has broken animation for me and on both the demo pages. Already opened issue just thought it might help to get @brycedorn attention too. Don't know how you tested but something key has been missed. Nothing stands out in the code commited.