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

fix(tooltip): performance and scope fixes #1455

Closed
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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
chrisirhc committed Dec 30, 2013
commit d7896f64991fd851eb868fd00e289a851c1f3ab3
8 changes: 1 addition & 7 deletions src/tooltip/test/tooltip.spec.js
Original file line number Diff line number Diff line change
@@ -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();
}));
41 changes: 31 additions & 10 deletions src/tooltip/tooltip.js
Original file line number Diff line number Diff line change
@@ -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();
});
};
}
};
};