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

$document event listener causes memory leak #84

Open
acidaris opened this issue Aug 4, 2016 · 2 comments
Open

$document event listener causes memory leak #84

acidaris opened this issue Aug 4, 2016 · 2 comments

Comments

@acidaris
Copy link

acidaris commented Aug 4, 2016

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
Copy link

I think you should submit pull request for that

@baj84
Copy link

baj84 commented Oct 27, 2016

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);
        });
      };

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

No branches or pull requests

3 participants