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

fix(tooltip): properly gc timeout on toggle of disabled #4210

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
48 changes: 35 additions & 13 deletions src/tooltip/test/tooltip.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,10 @@ describe('tooltip', function() {
});

describe('with specified popup delay', function() {
beforeEach(inject(function($compile) {
scope.delay='1000';
var $timeout;
beforeEach(inject(function($compile, _$timeout_) {
$timeout = _$timeout_;
scope.delay = '1000';
elm = $compile(angular.element(
'<span tooltip="tooltip text" tooltip-popup-delay="{{delay}}" ng-disabled="disabled">Selector Text</span>'
))(scope);
Expand All @@ -256,22 +258,22 @@ describe('tooltip', function() {
scope.$digest();
}));

it('should open after timeout', inject(function($timeout) {
it('should open after timeout', function() {
elm.trigger('mouseenter');
expect(tooltipScope.isOpen).toBe(false);

$timeout.flush();
expect(tooltipScope.isOpen).toBe(true);
}));
});

it('should not open if mouseleave before timeout', inject(function($timeout) {
it('should not open if mouseleave before timeout', function() {
elm.trigger('mouseenter');
expect(tooltipScope.isOpen).toBe(false);

elm.trigger('mouseleave');
$timeout.flush();
expect(tooltipScope.isOpen).toBe(false);
}));
});

it('should use default popup delay if specified delay is not a number', function() {
scope.delay='text1000';
Expand All @@ -280,7 +282,7 @@ describe('tooltip', function() {
expect(tooltipScope.isOpen).toBe(true);
});

it('should not open if disabled is present', inject(function($timeout) {
it('should not open if disabled is present', function() {
elm.trigger('mouseenter');
expect(tooltipScope.isOpen).toBe(false);

Expand All @@ -291,10 +293,30 @@ describe('tooltip', function() {

$timeout.flush();
expect(tooltipScope.isOpen).toBe(false);
}));
});

it('should open when not disabled after being disabled - issue #4204', function() {
elm.trigger('mouseenter');
expect(tooltipScope.isOpen).toBe(false);

$timeout.flush(500);
elmScope.disabled = true;
elmScope.$digest();

$timeout.flush(500);
expect(tooltipScope.isOpen).toBe(false);

elmScope.disabled = false;
elmScope.$digest();

elm.trigger('mouseenter');
$timeout.flush();

expect(tooltipScope.isOpen).toBe(true);
});
});
describe( 'with an is-open attribute', function() {

describe('with an is-open attribute', function() {
beforeEach(inject(function ($compile) {
scope.isOpen = false;
elm = $compile(angular.element(
Expand All @@ -304,7 +326,7 @@ describe('tooltip', function() {
tooltipScope = elmScope.$$childTail;
scope.$digest();
}));

it( 'should show and hide with the controller value', function() {
expect(tooltipScope.isOpen).toBe(false);
elmScope.isOpen = true;
Expand All @@ -314,7 +336,7 @@ describe('tooltip', function() {
elmScope.$digest();
expect(tooltipScope.isOpen).toBe(false);
});

it( 'should update the controller value', function() {
elm.trigger('mouseenter');
expect(elmScope.isOpen).toBe(true);
Expand Down Expand Up @@ -412,7 +434,7 @@ describe('tooltip', function() {
elm.trigger('fakeTriggerAttr');
expect( tooltipScope.isOpen ).toBeFalsy();
}));

it( 'should not show when trigger is set to "none"', inject(function($compile) {
elmBody = angular.element(
'<div><input tooltip="Hello!" tooltip-trigger="none" /></div>'
Expand Down
11 changes: 6 additions & 5 deletions src/tooltip/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
if (isOpenExp) {
isOpenExp.assign(ttScope.origScope, ttScope.isOpen);
}

if (!$rootScope.$$phase) {
ttScope.$apply(); // digest required as $apply is not called
}
Expand All @@ -232,7 +232,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
if (isOpenExp) {
isOpenExp.assign(ttScope.origScope, ttScope.isOpen);
}

//if tooltip is going to be shown after delay, we must cancel this
$timeout.cancel(popupTimeout);
popupTimeout = null;
Expand Down Expand Up @@ -269,7 +269,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
hide();
}
});

tooltipLinkedScope.$watch(function() {
if (!repositionScheduled) {
repositionScheduled = true;
Expand All @@ -281,7 +281,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
});
}
});

}
}

Expand Down Expand Up @@ -325,6 +325,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
attrs.$observe('disabled', function(val) {
if (popupTimeout && val) {
$timeout.cancel(popupTimeout);
popupTimeout = null;
}

if (val && ttScope.isOpen) {
Expand All @@ -345,7 +346,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
}, 0, false);
}
});

if (isOpenExp) {
scope.$watch(isOpenExp, function(val) {
if (val !== ttScope.isOpen) {
Expand Down