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

the animation state _running sometimes stay false every time I call it #49

Closed
tiye opened this issue Aug 15, 2014 · 6 comments · Fixed by #50
Closed

the animation state _running sometimes stay false every time I call it #49

tiye opened this issue Aug 15, 2014 · 6 comments · Fixed by #50
Assignees
Labels

Comments

@tiye
Copy link
Contributor

tiye commented Aug 15, 2014

I initialized with Favico like this:

favicon = new Favico
  animation: 'none'

but to find the badge not appearing when I call favicon.badge(3). I debugged into favico.js but found the state __running stay false just like that finished function was never called.

/**
 * Start animation
 */
icon.start = function() {
    if (!_ready || _running) { // <-- I found this _running false
        return;
    }
    var finished = function() {
        _lastBadge = _queue[0];
        _running = false;
        console.log('running -> false')
        if (_queue.length > 0) {
            _queue.shift();
            icon.start();
        } else {

        }
    };
    if (_queue.length > 0) {
        _running = true;
        console.log('running -> true')
        var run = function() {
            // apply options for this animation
            ['type', 'animation', 'bgColor', 'textColor',
             'fontFamily', 'fontStyle'].forEach(function(a) {
                if (a in _queue[0].options) {
                    _opt[a] = _queue[0].options[a];
                }
            });
            animation.run(_queue[0].options, function() {
                finished();
            }, false);
        };
        if (_lastBadge) {
            animation.run(_lastBadge.options, function() {
                run();
            }, true);
        } else {
            run();
        }
    }
};

But I failed to locate the problem. It happens in Chrome and Firefox but does not happen every time.
Looks like when I called it too frequenly and it breaks. Anyone run into this?

@ejci ejci added the bug label Aug 15, 2014
@ejci ejci self-assigned this Aug 15, 2014
@ejci
Copy link
Owner

ejci commented Aug 15, 2014

Does it happen only for animation type 'none' or also for others?

@tiye
Copy link
Contributor Author

tiye commented Aug 15, 2014

I don't think so. I justed tried but animations with none slide fade all run into this problem.
And I got some logs, which made me think it breaks every time I set badge 0 after _running was set to true.

2014-08-15T08:23:26.108Z badge -> 0
2014-08-15T08:23:26.555Z badge -> 0
2014-08-15T08:23:26.659Z badge -> 1
2014-08-15T08:23:26.660Z running -> true favico.js:173
2014-08-15T08:23:26.669Z badge -> 0 <-- I think the problem is here.
2014-08-15T08:23:32.275Z badge -> 1
2014-08-15T08:23:32.276Z badge -> 0
2014-08-15T08:23:32.672Z badge -> 1
2014-08-15T08:23:32.939Z badge -> 2 

@tiye
Copy link
Contributor Author

tiye commented Aug 15, 2014

I think my issue can be fixed by resetting _running to false when I reset the badge. Global state makes me anxious, and I'm not sure if it would affect other parts of you code.
I want to send a PR if you think that line is OK.

/**
 * Set badge
 */
var badge = function(number, opts) {
    opts = ((typeof opts)==='string' ? {animation:opts} : opts) || {};
    _readyCb = function() {
        try {
            if (typeof(number)==='number' ? (number > 0) : (number !== '')) {
                //
                // too long...
                //
                _queue.push(q);
                icon.start();
            } else {
                icon.reset();
                _running = false; // <-- I added two lines here
                console.log((new Date()).toISOString(), 'running -> false, forced');
            }
        } catch(e) {
            throw 'Error setting badge. Message: ' + e.message;
        }
    };
    if (_ready) {
        _readyCb();
    }
};

@ejci
Copy link
Owner

ejci commented Aug 15, 2014

Good catch!
Send a pull request I will test it and merge it to next version. And I will also make the repo smaller for bower.
Thanks

tiye added a commit to alibaba-archive/favico.js that referenced this issue Aug 15, 2014
tiye added a commit to alibaba-archive/favico.js that referenced this issue Sep 22, 2014
@amdprophet
Copy link

Any status updates on getting this merged?

@ejci ejci closed this as completed in #50 Dec 20, 2014
ejci added a commit that referenced this issue Dec 20, 2014
reset _running state when reset icon; fixes #49
@ejci
Copy link
Owner

ejci commented Dec 20, 2014

Merged. I will do few more fixes this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants