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

feat(tooltip): now can take the option for the close delay timeout #3576

Closed
wants to merge 1 commit into from
Closed

feat(tooltip): now can take the option for the close delay timeout #3576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions src/tooltip/docs/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ will display:
- `tooltip-animation`: Should it fade in and out? Defaults to "true".
- `tooltip-popup-delay`: For how long should the user have to have the mouse
over the element before the tooltip shows (in milliseconds)? Defaults to 0.
- `tooltip--close-popup-delay`: For how long should the tooltip remained open?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One dash

- `tooltip-trigger`: What should trigger a show of the tooltip? Supports a space separated list of event names.
Note: this attribute is no longer observable. See `tooltip-enable`.
- `tooltip-enable`: Is it enabled? It will enable or disable the configured
Expand Down Expand Up @@ -63,6 +64,7 @@ methods are available:
placement: 'top',
animation: true,
popupDelay: 0,
popupCloseDelay: 500,
appendToBody: false
</pre>

Expand Down
54 changes: 54 additions & 0 deletions src/tooltip/test/tooltip.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,60 @@ describe('tooltip', function() {
}));
});

describe('with specified popup close delay', function() {
var $timeout;
beforeEach(inject(function($compile, _$timeout_) {
$timeout = _$timeout_;
scope.delay = '1000';
elm = $compile(angular.element(
'<span uib-tooltip="tooltip text" tooltip-popup-close-delay="{{delay}}" ng-disabled="disabled">Selector Text</span>'
))(scope);
elmScope = elm.scope();
tooltipScope = elmScope.$$childTail;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't using Angular's internal method $$childTail creating a potential for future bugs, should its implementation change in the future?

If I understand it correctly, this is to access child scope, which we should try to avoid to.
Here it is suggested instead to

move the model up into the parent...
http://stackoverflow.com/a/14006165/1614973

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used the same setup as the test for the delay of the opening of the tooltip in the file, if you were to look at the rest of the spec it is doing the same. In a test, this is standard practice to access the directive's scope in order to check a value.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a concern that came up, I know it works for now, but might be helpful for future reference. There might be a better way without depending so deeply on Angular's mechanics.
I didn't want to add to already huge issue queue, so put it here.

scope.$digest();
}));

it('should close after timeout', function() {
trigger(elm, 'mouseenter');
expect(tooltipScope.isOpen).toBe(true);
trigger(elm, 'mouseleave');
$timeout.flush();
expect(tooltipScope.isOpen).toBe(false);
});
//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete comment

it('should use default popup close delay if specified delay is not a number and close after delay', function() {
scope.delay = 'text1000';
scope.$digest();
trigger(elm, 'mouseenter');
expect(tooltipScope.popupCloseDelay).toBe(500);
expect(tooltipScope.isOpen).toBe(true);
trigger(elm, 'mouseleave');
$timeout.flush();
expect(tooltipScope.isOpen).toBe(false);
});

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

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

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

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

trigger(elm, 'mouseenter');

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

describe('with an is-open attribute', function() {
beforeEach(inject(function ($compile) {
scope.isOpen = false;
Expand Down
5 changes: 4 additions & 1 deletion src/tooltip/tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.s
placement: 'top',
animation: true,
popupDelay: 0,
popupCloseDelay: 500,
useContentExp: false
};

Expand Down Expand Up @@ -265,7 +266,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.s
// FIXME: this is a placeholder for a port of the transitions library.
if (ttScope.animation) {
if (!transitionTimeout) {
transitionTimeout = $timeout(removeTooltip, 500);
transitionTimeout = $timeout(removeTooltip, ttScope.popupCloseDelay);
}
} else {
removeTooltip();
Expand Down Expand Up @@ -321,7 +322,9 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.s
ttScope.placement = angular.isDefined(attrs[prefix + 'Placement']) ? attrs[prefix + 'Placement'] : options.placement;

var delay = parseInt(attrs[prefix + 'PopupDelay'], 10);
var closeDelay = parseInt(attrs[prefix + 'PopupCloseDelay'], 10);
ttScope.popupDelay = !isNaN(delay) ? delay : options.popupDelay;
ttScope.popupCloseDelay = !isNaN(closeDelay) ? closeDelay : options.popupCloseDelay;
}

ttScope.contentExp = function() {
Expand Down