Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

When removing a datepicker element from the dom, the element is not cleaned up properly #5780

Closed
ckosloski opened this issue Apr 11, 2016 · 6 comments

Comments

@ckosloski
Copy link

Bug description:

When removing a datepicker element from the dom, the element is not cleaned up properly and causes errors when the datepicker is added back to the dom.

Link to minimally-working plunker that reproduces the issue:

https://plnkr.co/edit/2zfVptOELXPeslM2EIVq?p=preview
Click on the date link, open and select a date from the datepicker and click the save button.
Click on the date link a second time and open the date picker. The following error is thrown in the console.

Error: this.activeDate is undefined
this._refreshView@https://cdnjs.cloudflare.com/ajax/libs/angular-ui-bootstrap/1.3.1/ui-bootstrap-tpls.js:1671:9
this.refreshView@https://cdnjs.cloudflare.com/ajax/libs/angular-ui-bootstrap/1.3.1/ui-bootstrap-tpls.js:1501:7
this.init@https://cdnjs.cloudflare.com/ajax/libs/angular-ui-bootstrap/1.3.1/ui-bootstrap-tpls.js:1657:5
.link@https://cdnjs.cloudflare.com/ajax/libs/angular-ui-bootstrap/1.3.1/ui-bootstrap-tpls.js:1903:7
ja@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:80:35
n@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:65:220
aa/<@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:76:125
g@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:58:140
ba/<@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:57:281
d@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:59:122
l@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:63:157
Ke</<.link/</<@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:298:363
q@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:7:353
Ke</<.link/<@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:298:1
sf/this.$get</m.prototype.$digest@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:140:306
sf/this.$get</m.prototype.$apply@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:143:247
Ic[b]</<.compile/</<@https://ajax.googleapis.com/ajax/libs/angularjs/1.5.3/angular.min.js:268:444
n.event.dispatch@https://code.jquery.com/jquery-2.2.2.min.js:3:7485
n.event.add/r.handle@https://code.jquery.com/jquery-2.2.2.min.js:3:5603

### Version of Angular, UIBS, and Bootstrap

Angular: 1.5.3

UIBS: 1.3.1

Bootstrap: 3.3.6

I think this can be fixed by adding the following to the UibDatepickerPopupController:

    $element.on('$destroy', function() {
      $scope.$destroy();
     });

You can view the fix working in this plunker:
https://plnkr.co/edit/oTtDgtmdKFQnMDaA9PhU?p=preview

@wesleycho
Copy link
Contributor

This sounds like xeditable's job - it should be garbage collecting by running $scope.$destroy() on element destruction if it is going to $compile it.

I'm not convinced this is something UI Bootstrap should have to do.

@wesleycho
Copy link
Contributor

@Foxandxss what is your thoughts on this?

@Foxandxss
Copy link
Contributor

To be honest, there seems to be two kind of $destroy, element and scope one and I am still confused. Not sure if you need both or just one.

If that change fixes it, I don't think it is gonna hurt us, so we can add it.

@ckosloski
Copy link
Author

Any update on this?

@Sampath-Lokuge
Copy link

Hi @wesleycho and @Foxandxss

Can we have any update for this ? Thanks.

@wesleycho
Copy link
Contributor

Please respect the fact that we are unpaid volunteers - in particular, many of us have been extraordinarily busy the past few months (one with prepping for ng-conf on the Angular team, myself flying to 5 different cities in the past month), and posting questions intended to badger is in bad form.

I'm still not convinced this is really something we should do in the library itself - this really is the job of anything being used to wire up the third party library into Angular. The implication would mean that we would have to make similar changes to every other component. In addition, if the $destroy is triggered and the element is destroyed in the process, this has the potential to trigger a double $destroy. This does not pass the smell test to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants