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

Commit

Permalink
fix(tooltip): performance and scope fixes
Browse files Browse the repository at this point in the history
Isolate scope contents should be the same after hiding and showing the
tooltip. The isolate scope's parent should also always be set to the
directive's scope correctly.

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.

Closes #1450
Closes #1191
Closes #1455
  • Loading branch information
chrisirhc authored and pkozlowski-opensource committed Dec 31, 2013
1 parent c4d0e2a commit c0df320
Show file tree
Hide file tree
Showing 2 changed files with 235 additions and 195 deletions.
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
Loading

0 comments on commit c0df320

Please sign in to comment.