From 676713edbdb6c79855c274660b48f28b9bec6904 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Mon, 23 Dec 2013 14:49:26 -0800 Subject: [PATCH] fix(tooltip): link on demand - Calling $digest is enough as we only need to digest the watchers in this scope and its children. No need to call $apply. - Set invokeApply to false on $timeout for popUpDelay - No need to test for cached reference when tooltip isn't visible as the tooltip has no scope. Fixes #1450 and #1191 --- src/tooltip/test/tooltip.spec.js | 8 +------ src/tooltip/tooltip.js | 41 ++++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/tooltip/test/tooltip.spec.js b/src/tooltip/test/tooltip.spec.js index b2aeba9a92..90be4f6907 100644 --- a/src/tooltip/test/tooltip.spec.js +++ b/src/tooltip/test/tooltip.spec.js @@ -348,18 +348,12 @@ describe('tooltip', function() { elm = elmBody.find('input'); elmScope = elm.scope(); + elm.trigger('fooTrigger'); tooltipScope = elmScope.$$childTail; })); - it( 'should not contain a cached reference', function() { - expect( inCache() ).toBeTruthy(); - elmScope.$destroy(); - expect( inCache() ).toBeFalsy(); - }); - it( 'should not contain a cached reference when visible', inject( function( $timeout ) { expect( inCache() ).toBeTruthy(); - elm.trigger('fooTrigger'); elmScope.$destroy(); expect( inCache() ).toBeFalsy(); })); diff --git a/src/tooltip/tooltip.js b/src/tooltip/tooltip.js index f6d116647f..9766f0ab35 100644 --- a/src/tooltip/tooltip.js +++ b/src/tooltip/tooltip.js @@ -108,8 +108,11 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap return { restrict: 'EA', scope: true, - link: function link ( scope, element, attrs ) { - var tooltip = $compile( template )( scope ); + compile: function (tElem, tAttrs) { + var tooltipLinker = $compile( template ); + + return function link ( scope, element, attrs ) { + var tooltip; var transitionTimeout; var popupTimeout; var appendToBody = angular.isDefined( options.appendToBody ) ? options.appendToBody : false; @@ -184,10 +187,10 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap return; } if ( scope.tt_popupDelay ) { - popupTimeout = $timeout( show, scope.tt_popupDelay ); + popupTimeout = $timeout( show, scope.tt_popupDelay, false ); popupTimeout.then(function(reposition){reposition();}); } else { - scope.$apply( show )(); + show(); } } @@ -206,6 +209,8 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap return angular.noop; } + createTooltip(); + // If there is a pending remove transition, we must cancel it, lest the // tooltip be mysteriously removed. if ( transitionTimeout ) { @@ -227,6 +232,7 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap // And show the tooltip. scope.tt_isOpen = true; + scope.$digest(); // digest required as $apply is not called // Return positioning function as promise callback for correct // positioning after draw. @@ -245,11 +251,27 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap // need to wait for it to expire beforehand. // FIXME: this is a placeholder for a port of the transitions library. if ( scope.tt_animation ) { - transitionTimeout = $timeout(function () { - tooltip.remove(); - }, 500); + transitionTimeout = $timeout(removeTooltip, 500); } else { + removeTooltip(); + } + } + + function createTooltip() { + // There can only be one tooltip element per directive shown at once. + if (tooltip) { + removeTooltip(); + } + tooltip = tooltipLinker(scope, function () {}); + + // Get contents rendered into the tooltip + scope.$digest(); + } + + function removeTooltip() { + if (tooltip) { tooltip.remove(); + tooltip = null; } } @@ -322,10 +344,9 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap $timeout.cancel( transitionTimeout ); $timeout.cancel( popupTimeout ); unregisterTriggers(); - tooltip.remove(); - tooltip.unbind(); - tooltip = null; + removeTooltip(); }); + }; } }; };