feature idleDelay/delayOnVisibilityChange and stopOnVisibilityChange

[gugiserman]

I needed to increase the delay of some of my pollers when the user leaves the page/tab, and switch it back once visibility changes again.

[emmaguo]

@gugiserman Thanks for the pull request! Is there any reason why you want to increase the interval when user leaves the tab instead of completely pausing all pollers by using poller.stopAll()?

[gugiserman]

@emmaguo I was stopping all pollers at first, but then we had the need to display the progress (%) of a progress-bar in the favicon (using favicon.js for ex). With the poller completely frozen I couldn't update the progress there.

Now I can continue to display it (and take it easy on the servers as well) by increasing the delay when visibility changes.

Also, if you set idleDelay to 0 it disables the poller without removing it, just preventing the request to start until the user comes back to the page. Which is also in use in my application, because for some pollers I could just disable it while idle.
I forgot to mention this in the README tho.

We were actually doing the polling thing with $interval everywhere in applications services, and then intercepting/delaying the requests using angular interceptor I built that watches page visibility. But it was just complicating things, so we found your project and I already switched all old poller services with yours. Awesome job, btw!

[emmaguo]

@gugiserman I see! That makes total sense. Also I am so happy that you guys find this library useful!
I will review your code soon :)

[emmaguo]

@gugiserman Let me know if those comments make sense to you. :-)

[gugiserman]

@emmaguo I'll make some changes right away. I think I put $rootScope.$broadcast there for some reason that got lost in the way :P
Some test is using it too, I'll fix that.

I actually implemented stopOnVisibilityChange and removed it later on because I found it unnecessary. It conflicts with delayOnVisibilityChange and makes things less flexible. For example, what if I set two pollers, and I want one of them to stop on visibilitychange and the other one to delay on visibilitychange? Right now as it is I can easily set idleDelay to 0 and 10000 in this case.

So, I would enable both configurations and decide the priority somehow? Or throw some error because there can't be the two features enabled at the same time.

Totally agree on setting default idleDelay to 10000.

edit: the example above, about the two pollers, is actually a real scenario in one of the SPAs I'm working on

edit2: I think I put isHidden optinal statement to simulate page visibility changing in unit test

[emmaguo]

I was thinking stopOnVisibilityChange could be useful for those who want to stop all their pollers on page visibility change. In your case where you want to stop one and delay the other one, you can simply set idleDelay to a very large number to act like a stopped poller. And I would assume that user would only use stopOnVisibilityChange or delayOnVisibilityChange. (Something we can add to readme).

I do not like using idleDelay 0 for stopping poller simply because it looks like 0 delay instead of infinite delay. Make sense?

[gugiserman]

@emmaguo just made the changes we talked about around idleDelay

If things look good, I can implement stopOnVisibilityChange later on

[emmaguo]

@gugiserman I see you already implemented stopOnVisibilityChange! Cool!

[emmaguo]

Why do you call handleDelayOnVisibilityChange here? Is this necessary?

[gugiserman]

From what I've tested in current use in my application, if you leave the tab before angular fully started, it won't catch visibilitychange event, so you are actually idle but never triggered stopAll/delayAll. So I just check right away for document.hidden

[emmaguo]

I see. Ok. Then let's keep it there:)

[gugiserman]

@emmaguo yeah, got a little excited lol

Missing tests, though. I don't know how to trigger visibilitychange in unit tests

[emmaguo]

@gugiserman I'm not sure how to trigger visibilitychange in unit tests either. Need some googling. I will merge it once we add unit test. :-)

[gugiserman]

@emmaguo awesome! I googled for a while and couldn't find much, but tomorrow I'll dig some more.

Thanks a lot!