perf(): Remove inside onChange event

[elifer5000]

When the callback run while changing the slider takes a certain amount of time (~50 ms or more), the
inside onChange can stall resulting in many changes lost along the way while it catches up.
Removing seems to have no visible side effects. It might be possible that this is due to
scope.() being called right before callOnChange(), so the model is updated anyway (see in
applyModel)

See:
http://jsfiddle.net/c8po9y58/3/

Move the slider like crazy and watch the console output, you will see no stalling. To compare use the master version:
http://jsfiddle.net/3bfnocrb/1/

[codecov-io]

Current coverage is 100.00%

Merging #229 into master will not affect coverage as of e4d660d

@@            master    #229   diff @@
======================================
  Files            1       1       
  Stmts          622     620     -2
  Branches         0       0       
  Methods          0       0       
======================================
- Hit            622     620     -2
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of e4d660d

Powered by Codecov. Updated on successful CI builds.

[ValentinH]

OK now I understand the issue. :)
Indeed, in the applyModel method we already call scope.$apply() so this $timeout is useless and bring overhead.

I think I'll also remove the $timeout call for the onStart since it should be called right away. However, I'll keep it for the onEnd since we need to ensure that the model is synchronized.

[ValentinH]

Just to make it perfect, could you remove this line and call onChange directly on this? (When it's done, don't forget to rebase so there is one commit) ;)

[elifer5000]

Ok, done!

[ValentinH]

Great thanks.

[ValentinH]

I just rollback your changes to the callback logic because I found the reason why we needed to wrap them in a $timeout call: if the user was changing one of its scope values in the callback function, then its changed wouldn't be applied.

In the new version, I replaced the previous $timeout calls by $evalAsync one.

See you example with the latest version: http://jsfiddle.net/zb33necv/ . Everything seems OK to me.

[elifer5000]

I see, will the performance be the same, though?

On Sun, Jan 24, 2016 at 1:00 PM, Valentin Hervieu [email protected]
wrote:

I just rollback your changes to the callback logic because I found the
reason why we needed to wrap them in a $timeout call: if the user was
changing one of its scope values in the callback function, then its changed
wouldn't be applied.

In the new version, I replaced the previous $timeout calls by $evalAsync
one.

—
Reply to this email directly or view it on GitHub
#229 (comment)
.

[ValentinH]

If I understand correctly how it works, with $evalAsync, if there is already a digest in progress then it will be executed in it, otherwise it will program a new one.

[elifer5000]

Indeed it seems to be ok. I tried again with my same sample and there's no
dropped events. You can try it here:
http://jsfiddle.net/3bfnocrb/2/

On Tue, Jan 26, 2016 at 9:07 AM, Valentin Hervieu [email protected]
wrote:

If I understand correctly how it works, with $evalAsync, if there is
already a digest in progress then it will be executed in it, otherwise it
will program a new one.

—
Reply to this email directly or view it on GitHub
#229 (comment)
.