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

Add support for floating point seconds as a duration #7

Closed
wants to merge 1 commit into from

Conversation

mduvall
Copy link
Contributor

@mduvall mduvall commented Sep 19, 2013

Fixes #4.

@kn
Copy link
Contributor

kn commented Sep 19, 2013

Assuming this is for just accepting floating point seconds as valid duration, I think it's better to just convert float to int at the beginning so that we can keep our code simple.

What do you think?

@mduvall
Copy link
Contributor Author

mduvall commented Sep 19, 2013

@kn This commit adds the actual countdown from a float, so doing new Countdown(2.5...) will trigger onComplete after two full ticks on the interval and sets a timeout for 500ms (the majority of the added code). I thought this abstraction would be useful as a basis to use other units of time that are more granular, ms specifically.

@kn
Copy link
Contributor

kn commented Sep 19, 2013

I see. I think this change makes countdown's behavior harder to understand. e.g. passing 2.5s as a duration makes two ticks and calls onComplete after 0.5s from the last tick, where last interval is now 0.5s instead of 1s (which is not usual behavior of counting down.)

Unless there is strong use case for this, I rather keep it simple.

Instead, I think we should force 1s interval by validating/converting duration argument.

Floating point duration makes more sense if we accept variable interval duration I think (not that I think we should do it).

@mduvall
Copy link
Contributor Author

mduvall commented Sep 19, 2013

@kn That's fair, it does add a fair amount of complexity for a pretty orthogonal concern. I wanted to use Countdown.js to animate the buttons (similar to a progress bar with the ticker) smoothly and was going to add intervals for ms as an animation hook with the tickCallback after this commit. CSS3 transitions could be used as well when I think about it! I'll close for now and we can open this later if somebody else requests it 👍

Here's a crappy drawing of what I kind of mean:
transition countdown

@mduvall mduvall closed this Sep 19, 2013
@kn
Copy link
Contributor

kn commented Sep 19, 2013

Awesome. Great idea tho! 👍

@mduvall mduvall mentioned this pull request Sep 20, 2013
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

Successfully merging this pull request may close these issues.

Support for float as a duration
2 participants