search

leocaseiro / angular-chosen

AngularJS Chosen directive is an AngularJS Directive that brings the Chosen jQuery in a AngularJS way
http://leocaseiro.github.io/angular-chosen/
MIT License
682 stars 248 forks source link

Fix a potential $digest in progress error #249

Closed VanTanev closed 5 years ago

VanTanev commented 6 years ago

When the chosen:hiding_dropdown event triggers during a digest (ie, an angular operation turns an open chosen into disabled with ng-disable, which closes the dropdown) calling $apply() in the event call back triggers a $digest already in progress error.

To fix that, we replace the call with $applyAsync().

Additionally, we replace all instances of $digest() in the tests with $apply(), because $apply() will also clear the async queue, and is the preferred method of doing this over $digest(): https://docs.angularjs.org/guide/unit-testing#testing-promises

leocaseiro commented 6 years ago

Hi @VanTanev, thank you for this PR.

LGTM!

Would you like to try run a release and publish into npm or do you prefer I do it?

VanTanev commented 6 years ago

Could you please?

I will also try to get the peer deps change up this week, for a 2.0 release :)

On Dec 4, 2017 6:06 AM, "Leo Caseiro" notifications@github.com wrote:

Hi @VanTanev https://github.com/vantanev, thank you for this PR.

LGTM!

Would you like to try run a release and publish into npm or do you prefer I do it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/leocaseiro/angular-chosen/pull/249#issuecomment-348855773, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMXHyycAz_yWkAmXpIXiigsxb_T1Ec_ks5s82_LgaJpZM4QwoS0 .

VanTanev commented 5 years ago

@leocaseiro if we're doing 1.9, let's get this in too, as well as a couple of other small ones

leocaseiro commented 5 years ago

Happy with merging this PR, however I’ve pushed a release 1.9.0 for npm and bower without this commit. We would need to release a 1.10.0 to introduce those changes.

VanTanev commented 5 years ago

This is minor enough to make it into 1.9.1 (and even a 1.8.1 if we cared to release that). It's a straight bugfix for an edgecase.

leocaseiro commented 5 years ago

If it’s a fix I would prefer 1.9.1 than. Would like to run the build?

It just need to replace the package.json to 1.9.1 and run gulp build.

(I know I haven’t automatized that yet, sorry LOL)

After that, we need to publish to npm via command line: npm login: npm publish ./; Via bower, just create a release on github.

I won’t be able to do today. But can do tomorrow.

VanTanev commented 5 years ago

@leocaseiro I replied to you last night through email, but github didn't pick it up it seems.

I don't have access on npm, you need to add me there for me to be able to do releases :) This is my account: https://www.npmjs.com/~van.tanev

leocaseiro commented 5 years ago

Hi @VanTanev I've added you at npm.