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

Fix angular-busy promise-compare issues and added UT #85

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix angular-busy promise-compare issues and added UT #85

wants to merge 3 commits into from

Conversation

benbracha
Copy link

Related also to issue #68

In case I change cg-busy unresolved promise with a new unresolved promise - the tracker.reset is not begin called.
This is due the use of angular.equals, which doesn't look on "$" fields - so the two above promises looks the same.
Because reset is not being called, when the first promise is resolved - the cg-busy overlay is removed, which causes unwanted behaviour (the user still waits for an action, but no overlay).

Therefore, we add to the promise object a random ID, so comparison works correctly.
Added also UT that catch this bug, and works after our fix.

Thanks to @barnash for the help in debugging and finding a solution for it.

@benbracha
Copy link
Author

Looking at the angular-busy code again, actually - I'm not sure what is the need for angular.compare on the promises at all. Meaning - removing my fix and the compare check together, will have the same result (and fix the issue I originally tried to fix).

All the logic is triggered under $watchCollection on the options parameter.
Two scenarios I see here:

  1. The options is a promise, or array of promises. In this case - if the $watchCollection was triggered, the angular.equal will always return true because there is an actual change of the promises.
  2. The options is an object, with promise or array of promises supplied. In this case - if the $watchCollection was triggered it is either the promise was changed or other key in the object (like the min-duration or templateUrl key).
    If it was a promise change - we need to call reset anyway.
    If not - calling reset again seems not to have any effect. It may create more callbacks for delay/min-duration which is a waste, but it doesn't seem to have any effect.

To be on the safe side - I'll keep my fix because if fixes the author original intention - trigger reset only on promise changes..

@faceleg
Copy link

faceleg commented Mar 2, 2016

I've forked this to https://github.com/faceleg/angular-busy-plus. Recommend using that as the development focus until / unless the owner becomes active or brings help on board.

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.

3 participants