Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing undefined as second arg causes an exception #275

Closed
Rycochet opened this issue Aug 29, 2014 · 4 comments
Closed

Passing undefined as second arg causes an exception #275

Rycochet opened this issue Aug 29, 2014 · 4 comments

Comments

@Rycochet
Copy link
Collaborator

If you pass undefined as the second argument to velocity it throws an error related to line 2060 -

if (!Type.isArray(arguments[i]) && (/fast|normal|slow/i.test(arguments[i].toString()) || /^\d/.test(arguments[i]))) {

Basically this means that using a wrapper function with an optional second arg can't pass that second arg straight through without processing it in some way.

It might be somewhat safer (and possibly quicker) to do something like this (forcing the formatting, rather than matching any string with the correct words etc too) -

if (arguments[i] !== undefined && !Type.isArray(arguments[i]) && /^(fast|normal|slow|\d+)$/i.test(arguments[i].toString())) {

Though I'd be tempted to manually check if it's a number or matching string specifically, rather than simply excluding an array.

jsFiddle - http://jsfiddle.net/Rycochet/65xCP/6/ (since I seem to always forget the things :-P )

@julianshapiro
Copy link
Owner

Thanks, Robyn. I'll have this fixed in the next version. Might be a bit before that's released, though, as my update frequency will henceforth depend on the reporting of critical bugs.

P.S. I was unnecessarily calling .toString() for IE8 error prevention. Turns out it's only .match()/.replace() that throw errors on non-strings -- not .test().

@Rycochet
Copy link
Collaborator Author

I did wonder about the .toString, but couldn't remember where implicit conversion happened so just left it in - fortunately I only need IE9+ at work, so can ignore browser bugs like that ;-)

You could always use a separate branch for working on, then only update master when you want a new release - I keep meaning to do things like that myself, but still too many changes (and not publicly launched either for that matter) ;-)

Btw, in Scotland, Robin is the boys name and Robyn is the girls name, it's only been Americanised in the last couple of decades... Gah, now I feel old(er) :-P

@julianshapiro
Copy link
Owner

Please check, make sure it works, and get back to me. Thanks!

@Rycochet
Copy link
Collaborator Author

Doh, just realised I hadn't replied - yep, fixed and working :-)

Rycochet pushed a commit that referenced this issue Aug 3, 2020
Internal “Sequences” have been renamed to “Redirects”. You shouldn’t
have been using them (they were deprecated), so this is not being
considered backwards-incompatible.

`Velocity.mock` now accepts a multiplier value in addition to `true`.
Use it for slowing down/speeding up animations globally when testing.
`true` mock has also been fixed to also reduce `delay`s to 1ms.

You can now animate `filter: blur(value);` for -webkit- browsers. Note
that animating this property will strip out all other `filter`
components. Note that, at the time of this update, Chrome incorrectly
returns blur values that were originally set with the `px` unit.

`color` RGB defaults to black instead of white, which is closer to
text’s typical color value.

`opts.progress` now gets repeatedly fired during looped calls. Closes
#292.

Fix jQuery-free `position: absolute` bug with left/top properties.
Closes #285.

Allow passing undefined as a second arg into Velocity. Closes #275.

Performance improvement: Individual property tweens don’t update the
DOM if there’s no change in value between loop ticks.

Fixes bug where `loop`ing on iOS devices (due to mobileHA) would cause
flickering. Closes #299.

Fixes bug where you couldn’t force-feed one hex value to another.

Can now set `visibility: “”`, just like you can with display.

Fixed bug where using `backwards: true:` with the UI pack mutated the
source array.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants