restorando / angular-pickadate

A simple and fluid inline datepicker for AngularJS with no extra dependencies.
MIT License
273 stars 91 forks source link

$document event listener causes memory leak #84

Open acidaris opened 8 years ago

acidaris commented 8 years ago

Found this when attempting to debug crashes of javascript tests.

This click event added to the $document is never cleaned up. This means any page that renders a angular-pickadate component will leak memory, especially if the application uses ui router that might render many pickadate components.

see https://github.com/restorando/angular-pickadate/blob/master/src/angular-pickadate.js#L82

Should instead be something like this

return function(scope, element, rootNode) {
        var togglePicker = function(toggle) {
          scope.displayPicker = toggle;
          scope.$apply();
        };

        var handleDocumentClick = function(e) function(e) {
          if (isDescendant(rootNode, e.target) || e.target === element[0]) return;
          togglePicker(false);
        }

        element.on('focus', function() {
          scope.modalStyles = computeStyles(element[0]);
          togglePicker(true);
        });

        element.on('keydown', function(e) {
          if (indexOf.call([9, 13, 27], e.keyCode) >= 0) togglePicker(false);
        });

        $document.on('click', handleDocumentClick);

        // Remove click event when element is destroyed.
        element.on('$destroy', function(){
             $document.off('click, handleDocumentClick);
       });

      };

    }])
mpospelov commented 8 years ago

I think you should submit pull request for that

baj84 commented 7 years ago

Thanks for that fix, although your code has a few small syntax errors. I've fixed them below:

        var togglePicker = function(toggle) {
          scope.displayPicker = toggle;
          scope.$apply();
        };

        var handleDocumentClick = function(e) {
          if (isDescendant(rootNode, e.target) || e.target === element[0]) return;
          togglePicker(false);
        };

        element.on('focus', function() {
          scope.modalStyles = computeStyles(element[0]);
          togglePicker(true);
        });

        element.on('keydown', function(e) {
          if (indexOf.call([9, 13, 27], e.keyCode) >= 0) togglePicker(false);
        });

        $document.on('click', handleDocumentClick);

        // Remove click event when element is destroyed.
        element.on('$destroy', function() {
          $document.off('click', handleDocumentClick);
        });
      };