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

fix(tooltip): performance and scope fixes #1455

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
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
35 changes: 27 additions & 8 deletions src/tooltip/test/tooltip.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe('tooltip', function() {
scope.alt = "Alt Message";

elmBody = $compile( angular.element(
'<div><span alt={{alt}} tooltip="{{tooltipMsg}}">Selector Text</span></div>'
'<div><span alt={{alt}} tooltip="{{tooltipMsg}}" tooltip-animation="false">Selector Text</span></div>'
) )( scope );

$compile( elmBody )( scope );
Expand All @@ -114,6 +114,13 @@ describe('tooltip', function() {
expect( ttScope.content ).toBe( scope.tooltipMsg );

elm.trigger( 'mouseleave' );

//Isolate scope contents should be the same after hiding and showing again (issue 1191)
elm.trigger( 'mouseenter' );

ttScope = angular.element( elmBody.children()[1] ).scope();
expect( ttScope.placement ).toBe( 'top' );
expect( ttScope.content ).toBe( scope.tooltipMsg );
}));

it('should not show tooltips if there is nothing to show - issue #129', inject(function ($compile) {
Expand All @@ -136,6 +143,24 @@ describe('tooltip', function() {
expect( elmBody.children().length ).toBe( 0 );
}));

it('issue 1191 - isolate scope on the popup should always be child of correct element scope', inject( function ( $compile ) {
var ttScope;
elm.trigger( 'mouseenter' );

ttScope = angular.element( elmBody.children()[1] ).scope();
expect( ttScope.$parent ).toBe( elmScope );

elm.trigger( 'mouseleave' );

// After leaving and coming back, the scope's parent should be the same
elm.trigger( 'mouseenter' );

ttScope = angular.element( elmBody.children()[1] ).scope();
expect( ttScope.$parent ).toBe( elmScope );

elm.trigger( 'mouseleave' );
}));

describe('with specified enable expression', function() {

beforeEach(inject(function ($compile) {
Expand Down Expand Up @@ -323,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();
}));
Expand Down
41 changes: 31 additions & 10 deletions src/tooltip/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()();
}
}

Expand All @@ -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 ) {
Expand All @@ -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.
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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();
});
};
}
};
};
Expand Down