Updated timer to support continuous run (#1) #92

mohitYogi · 2019-07-26T09:42:36Z

Updated intervalHandler to support continuous run

intervalHandler is updated in order to support continuous timer run, even after duration is completed

Flag for single call to callback

Flag for single call to callback when timer completes in continuous run.

Update utils.js
support continuous Timer run

Added stopTimerOnDuration in config to run timer continuously.
Updated intervalHandler to support continuous run
Added Flag isCallbackCalled to stop multiple calls to callback after reaching the duration in continuous run.

* Updated intervalHandler to support continuous run intervalHandler is updated in order to support continuous timer run, even after duration is completed * Flag for single call to callback Flag for single call to callback when timer completes in continuous run. * Update utils.js * support continuous Timer run 1. Added stopTimerOnDuration in config to run timer continuously. 2. Updated intervalHandler to support continuous run 3. Added Flag isCallbackCalled to stop multiple calls to callback after reaching the duration in continuous run.

walmik

Please bump patch version in package.json as part of this PR. Make it 0.9.1.

walmik · 2019-07-26T14:50:01Z

src/utils.js

@@ -52,7 +52,8 @@ function getDefaultConfig() {
 		repeat: false,				// This will repeat callback every n times duration is elapsed
 		countdown: false,			// If true, this will render the timer as a countdown (must have duration)
 		format: null,				// This sets the format in which the time will be printed
-		updateFrequency: 500		// How often should timer display update
+		updateFrequency: 500,		// How often should timer display update
+            	stopTimerOnDuration: true,	// If set to false than timer will keep on running even after duration is completed.


Is this prop used anywhere? By the way, we do have a prop called repeat to do this. Hence we might not need this.

removed the prop stopTimerOnDuration.

walmik · 2019-07-26T14:55:05Z

src/utils.js

-		if (timerInstance.totalSeconds === 0) {
+
+		//Added isCallbackCalled flag, to avoid multiple calls to the callback.
+		if (!timerInstance.isCallbackCalled && timerInstance.totalSeconds === 0) {


Do we really need the check for isCallbackCalled? We are clearing the interval on the next line. Once the interval is cleared the callback should not be called. Here s an example usage of the countdown with duration https://jsbin.com/noyijuqice/edit?js,console,output Lemme know if I m missing something.

Removed the flag.

walmik · 2019-07-26T14:57:43Z

src/utils.js

 	if (timerInstance.totalSeconds > 0 &&
-		timerInstance.totalSeconds % timerInstance.config.duration === 0) {
+		(timerInstance.totalSeconds % timerInstance.config.duration === 0 || timerInstance.totalSeconds > timerInstance.config.duration )) {


This is a good check to add :)

1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review.

Removed flag isCallbackCalled. (Not required).

1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review.

New rule for checking timer duration completed.

mohitYogi · 2019-07-27T05:38:37Z

Updated the code.

walmik

We are almost there, left a coupla minor comments. Once you address those I ll merge the changes.

walmik · 2019-07-28T02:40:32Z

dist/timer.jquery.js

@@ -241,7 +241,7 @@ function intervalHandler(timerInstance) {
 		if (timerInstance.totalSeconds === 0) {
 			clearInterval(timerInstance.intervalId);
 			setState(timerInstance, Constants.TIMER_STOPPED);
-			timerInstance.config.callback();
+			timerInstance.config.callback()


Looks like the semi-colon got removed here in the dist file.

walmik · 2019-07-28T02:41:59Z

src/utils.js

@@ -227,7 +227,8 @@ function intervalHandler(timerInstance) {

 	if (timerInstance.config.countdown) {
 		timerInstance.totalSeconds = timerInstance.config.duration - timerInstance.totalSeconds;
-
+
+		//Added isCallbackCalled flag, to avoid multiple calls to the callback.


We can remove this comment as well.

walmik · 2019-07-28T02:43:48Z

src/utils.js

 	if (!timerInstance.config.duration) {
 		return;
 	}

 	// If the timer was called with a duration parameter,
 	// run the callback if duration is complete
 	// and remove the duration if `repeat` is not requested
+
+	// Added new Condition of checking the timer completed or not


Remove this new comment and update the comment on line 250. Change from,
// run the callback if duration is complete
to,
// run the callback if duration is complete or total seconds is more than duration

Added missing semi colon. Updated Comment.

mohitYogi · 2019-07-30T05:15:45Z

Updated files as per required.

walmik

Thank you very much for diligent effort in getting this in. I m merging it now, it should be available on CDNjs. Currently it shows the existing version, but in a few hours it should update to the new version and will have your changes in it https://cdnjs.com/libraries/timer.jquery

* Updated timer to support continuous run (#1) (walmik#92) * Updated timer to support continuous run (#1) * Updated intervalHandler to support continuous run intervalHandler is updated in order to support continuous timer run, even after duration is completed * Flag for single call to callback Flag for single call to callback when timer completes in continuous run. * Update utils.js * support continuous Timer run 1. Added stopTimerOnDuration in config to run timer continuously. 2. Updated intervalHandler to support continuous run 3. Added Flag isCallbackCalled to stop multiple calls to callback after reaching the duration in continuous run. * Updates after review. 1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review. * Update after Review Removed flag isCallbackCalled. (Not required). * Updates after review 1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review. * Updates after Review 1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review. * Patch version New rule for checking timer duration completed. * Update timer.jquery.js * Semicolon & Comment update Added missing semi colon. Updated Comment. * Removed & Updated Comment * Update README.md (walmik#99) Let's not send people to an ad-infested abandoned domain name :) Co-authored-by: Alan <my@working.name>

mohitYogi mentioned this pull request Jul 26, 2019

Timer Keeps on running if provided seconds are already greater than duration #90

Closed

walmik reviewed Jul 26, 2019

View reviewed changes

mohitYogi added 6 commits July 27, 2019 10:47

Updates after review.

5da095b

1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review.

Update after Review

076c1af

Removed flag isCallbackCalled. (Not required).

Updates after review

408fbef

1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review.

Updates after Review

9c328ef

1. Removed stopTimerOnDuration flag (repeat flag do the same.) 2. Removed isCallbackCalled flag, as suggested after commit review.

Patch version

77a7ac9

New rule for checking timer duration completed.

Update timer.jquery.js

84baf37

walmik reviewed Jul 28, 2019

View reviewed changes

mohitYogi added 2 commits July 30, 2019 10:41

Semicolon & Comment update

fd614b9

Added missing semi colon. Updated Comment.

Removed & Updated Comment

c3e5986

mohitYogi closed this Jul 31, 2019

mohitYogi reopened this Jul 31, 2019

walmik approved these changes Jul 31, 2019

View reviewed changes

walmik merged commit 9b7c862 into walmik:master Jul 31, 2019

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated timer to support continuous run (#1) #92

Updated timer to support continuous run (#1) #92

mohitYogi commented Jul 26, 2019

walmik left a comment

walmik Jul 26, 2019

mohitYogi Jul 27, 2019

walmik Jul 26, 2019

mohitYogi Jul 27, 2019

walmik Jul 26, 2019

mohitYogi commented Jul 27, 2019

walmik left a comment

walmik Jul 28, 2019

walmik Jul 28, 2019

walmik Jul 28, 2019

mohitYogi commented Jul 30, 2019

walmik left a comment

Updated timer to support continuous run (#1) #92

Updated timer to support continuous run (#1) #92

Conversation

mohitYogi commented Jul 26, 2019

walmik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohitYogi commented Jul 27, 2019

walmik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohitYogi commented Jul 30, 2019

walmik left a comment

Choose a reason for hiding this comment