Skip to content

Added listener to be able to set countdown value #44

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

Merged
merged 5 commits into from
Feb 8, 2014

Conversation

johnparn
Copy link
Contributor

@johnparn johnparn commented Feb 5, 2014

With the listener one can set the countdown value using
$broadcast('countdown-set', value). This might be an edge case, but I
need to set a new countdown. Issue #23 describes how to set the value on initialization but using
this one can change the value whenever necessary.

Also changed the order of execution of some code to make 0 the last emitted millis value
for a countdown. In the original code the last emit message was the value just
before 0. For example with a countdown with an interval of 1000 the last
emitted millis value was 1000.

With the listener one can set the countdown value using
$broadcast('countdown-set', value). This might be an edge case, but I
need to set a new countdown. This makes the countdown value dynamic set.
Issue siddii#23 describes how to set the value on initialization but using
this one can change the value whenever necessary.
Changed the order of execution to make 0 the last emitted millis value
for a countdown. Previously the last emit message was the value just
before 0. For example with a countdown with an interval of 1000 the last
emitted millis value was 1000.
@@ -143,14 +147,6 @@ angular.module('timer', [])
return;
}
calculateTimeUnits();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this section further down so that the last millis value emitted is 0. In the original code the last value emitted is the value right before the countdown reaches 0, i e with an interval of 1000 the last value emitted is 1000.

@siddii
Copy link
Owner

siddii commented Feb 7, 2014

@johnparn - Thanks for the PR! Sorry, I don't see any changes to the source file app/js/timer.js whereas I see some changes on the built file (angular-timer.js).

And also, could you please change the event to timer-set-countdown complying with the other event names?

@johnparn
Copy link
Contributor Author

johnparn commented Feb 7, 2014

My bad, I'll change the proper file and change the event name.

// John

On 20140207,, at 15:55 , Siddique Hameed wrote:

@johnparn - Thanks for the PR! Sorry, I don't see any changes to the source file app/js/timer.js whereas I see some changes on the built file (angular-timer.js).

And also, could you please change the event to timer-set-countdown complying with the other event names?


Reply to this email directly or view it on GitHub.

With the listener one can set the countdown value using
$broadcast('countdown-set', value). This might be an edge case, but I
need to set a new countdown. Issue siddii#23 describes how to set the value on
initialization but using this one can change the value whenever
necessary.

Also changed the order of execution of some code to make 0 the last
emitted millis value for a countdown. In the original code the last emit
message was the value just
before 0. For example with a countdown with an interval of 1000 the last
emitted millis value was 1000.
@johnparn
Copy link
Contributor Author

johnparn commented Feb 7, 2014

Hi!

I have committed the change of timer.js and reverted the previous commits.

@siddii
Copy link
Owner

siddii commented Feb 7, 2014

I am looking at this PR. Does this mean we can merge or remove addCDSeconds method?

Any thoughts?

@johnparn
Copy link
Contributor Author

johnparn commented Feb 8, 2014

There might be use cases for appending seconds to countdown. Either merge or create a new listener with event name timer-append-countdown.

If merging, did you think about passing the type of operation via an argument, ie append, add or subtract?

@johnparn
Copy link
Contributor Author

johnparn commented Feb 8, 2014

Well, append and add would be the same. Rather add, subtract and set.

@siddii
Copy link
Owner

siddii commented Feb 8, 2014

I thought we could remove addCDSeconds method if there is a way to set a specific countdown value.
No biggie, we can circle back on this later.

I am gonna merge this PR for now. Thanks for the PR 👍

siddii added a commit that referenced this pull request Feb 8, 2014
Added listener to be able to set countdown value
@siddii siddii merged commit 42d932b into siddii:master Feb 8, 2014
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.

2 participants