search

monterail / angular-date-range-picker

Pure Angular date range picker, no jQuery
MIT License
70 stars 50 forks source link

Unbind Document Click Event #8

Closed mjquinlan2000 closed 10 years ago

mjquinlan2000 commented 10 years ago

The bound document click event created in this directive was interfering with other click events after I had navigated away from the page I was using it on. Regardless, the bound function should be unbound upon destruction of the scope so that we don't have rogue click events running when they no longer apply. Please feel free to contact me with any questions or concerns.

teamon commented 10 years ago

Sounds pretty reasonable :) Could this be fix for #6 ?

mjquinlan2000 commented 10 years ago

I doubt it's a fix as much as it's some insight into the whole documentClickFn in general. The function is still bound to document, while the directive is on the page. My fix just destroys the click binding when angular destroys the scope. I'm not using any checkboxes on the page and cannot vouch that it is an issue, but based on what I've seen, the claim seems pretty reasonable.

Part of the problem may be that you're calling scope.hide() within the $apply wrapper, which kicks off the digest loop. Is that entirely necessary?

teamon commented 10 years ago

angular.element(document).bind does not wrap the function in $scope.$apply so we have to do this manually.

Either way I'm going to test this PR, hopefully merge and release new version tomorrow, it's already midnight in Poland ;)

mjquinlan2000 commented 10 years ago

Right. I thought that might be the case. It seems to me that making the element hidden or shown isn't really relevant to the digest cycle except for the fact that you have an ng-show directive hanging onto the element. Have you considered styling the element to have display: none; and calling element.show() or element.hide() in the $scope.show and $scope.hide functions respectively?