Skip to content

UI Pack: Incorrect handling of defaults values #239

Closed
@jods4

Description

@jods4

Velocity consistently uses the || operator to set default values, like this: opt.easing = param.easing || 'ease';.
This is a bad JS habit, because it doesn't work when the value passed is falsy but not undefined (e.g. false or 0).

I got biten by this bug here: https://github.com/julianshapiro/velocity/blob/master/velocity.ui.js#L83
opts.queue = sequenceOptions.queue || "";
This is wrong because the documentation says that passing the option queue: false executes the animation immediately. The code above in the UI pack silently ignores a passed false option.

A suggested fix is to use $.extend to set default values. Or in this case -- because you are white-listing the options you copy -- the longer opts.queue = sequenceOptions.queue !== undefined ? sequenceOptions.queue : "";

BTW I stumbled on two related bugs.
(1) The UI pack doesn't white-list (i.e. copy) the option _cacheValues any reason for that? It prevents using transitions on an element that you may have modified directly.

(2) It is highly annoying that these options don't work: { stagger: 75, queue: false }. The problem seems to be that not queuing bypasses everything in the queue including the delays created by the animate call itself! It would be a lot more useful if queue: false bypasses everything else, with the exception of its own stuff!

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions