Removed the use of remove() method

[kasparasg]

When hiding and showing the popover the bound events were removed from the element, because of the use of remove(). That means that if you have custom popover template which has some clickable dynamic content it only works on the first show of the popover. Therefore I put in place a simple fix. Let me know if this can be merged in or if not, how would you do it differently?

Thanks a lot.

[jbruni]

Hi, @foreachlt - let me tell you how I got that code for #1391...

When using jQuery, then instead of our workarounds, using detach just works: http://api.jquery.com/detach/ - in other words, what we need is detach, not remove...

But Angular's jqLite does not have detach!

I searched for jQuery's detach source code, and for my surprise here is what I've found:

        detach: function( selector ) {
                return this.remove( selector, true );
        },

source: https://github.com/jquery/jquery/blob/master/src/manipulation.js#L464

You see? There is an undocumented second parameter for remove. So, when using jQuery, both detach() or adding a true boolean as second parameter for remove( null, true ) work!

But, again, we are at AngularJS and should rely on jqLite... so, I went further on and checked out the remove code for both jQuery and jqLite.

Below we have jQuery's remove - look how the second parameter (which triggers "detach" behaviour) is used:

        remove: function( selector, keepData /* Internal Use Only */ ) {
                var elem,
                        elems = selector ? jQuery.filter( selector, this ) : this,
                        i = 0;

                for ( ; (elem = elems[i]) != null; i++ ) {
                        if ( !keepData && elem.nodeType === 1 ) {
                                jQuery.cleanData( getAll( elem ) );
                        }

                        if ( elem.parentNode ) {
                                if ( keepData && jQuery.contains( elem.ownerDocument, elem ) ) {
                                        setGlobalEval( getAll( elem, "script" ) );
                                }
                                elem.parentNode.removeChild( elem );
                        }
                }

                return this;
        },

source: https://github.com/jquery/jquery/blob/master/src/manipulation.js#L359

And below we have jqLite's remove:

  remove: function(element) {
    jqLiteDealoc(element);
    var parent = element.parentNode;
    if (parent) parent.removeChild(element);
  },

source: https://github.com/angular/angular.js/blob/master/src/jqLite.js#L799

Further investigation will reveal that jqLiteDealoc is performing similar functionality as of jQuery.cleanData, and what really does the "dettachment" is simply the if ( elem.parentNode ) { elem.parentNode.removeChild( elem ); } in both jQuery and jqLite.

That's why I think jqLite should add a detach method (just like jQuery already has). This way, we could keep the tooltip code cleaner, by simply calling detach instead of this "dirtier" removeChild code...

I wanted to create a PR for a jqLite's detach, and now your interaction gave me the motivation. I think I'll do it. After all, it is quite simple.

[kasparasg]

@jbruni thanks for the explanation, it makes sense 👍 I hope your PR gets merged in Angular core.

[chrisirhc]

This should be fixed soon via #1455 .

[mgol]

The Angular PR (angular/angular.js#5461) implementing the jqLite detach method has just landed (angular/angular.js@1a05daf).