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

fix(tooltip): popup close delay not respected #4597

Closed
wants to merge 1 commit into from
Closed

fix(tooltip): popup close delay not respected #4597

wants to merge 1 commit into from

Conversation

RobJacobs
Copy link
Contributor

Under PR #4455 the timeout for the popup close delay
was impacted by the evalAsync for setting the isOpen
value. Moved the popup close delay timeout to the
hideTooltipBind method so the hide mehtod will now
get called after the close popup delay. This will
also ensure if the show method is called and the
close timeout is cancelled, the isOpen property
will not get toggled.

@wesleycho
Copy link
Contributor

Can you explain what is going on here - how come it is not enough that all of the logic is in the hide function and why move it up to the hideTooltipBind function?

@RobJacobs
Copy link
Contributor Author

The hide function is called in the disabled and empty content watchers, by-passing the close delay. Maybe they shouldn't?

@RobJacobs
Copy link
Contributor Author

@tmcgee123 @wesleycho After looking into this further, I think the original implementation of this feature was wrong. The transitionTimeout should be reserved for allowing the tooltip animation to run before removing the tooltip element from the dom. The tooltip templates use ng-class="{ in: isOpen() }" to add and remove the 'in' class which sets the opacity (0 and .9). The fade class has the opactiy transition with a time of .15s.

A close delay should be a different timeout that completes before the transitionTimeout. I'm going to do some more work on this PR and get this working as it should.

@tmcgee123
Copy link
Contributor

@RobJacobs I agree. Originally in 13.x, the functionality was as you said. But, with the updates that were put through, it looks as though the timeout is no longer completing before the animation starts.

Under PR #4455 the timeout for the popup close delay
was impacted by the evalAsync for setting the isOpen
value.  Moved the popup close delay timeout to the
hideTooltipBind method so the hide method will now
get called after the close popup delay.  This will
also ensure if the show method is called and the
close timeout is cancelled, the isOpen property
will not get toggled.

Closes #4597

Fixes #4567
@wesleycho
Copy link
Contributor

This PR LGTM - that was a good find that the implementation was incorrect. We really need to port the timeout to use ngAnimate, but that's future work.

@RobJacobs RobJacobs closed this in 6daf871 Oct 14, 2015
@RobJacobs RobJacobs deleted the fix-tooltip-delay branch October 14, 2015 13:13
aroop pushed a commit to aroop/bootstrap that referenced this pull request Oct 16, 2015
Under PR angular-ui#4455 the timeout for the popup close delay
was impacted by the evalAsync for setting the isOpen
value. Moved the popup close delay timeout to the
hideTooltipBind method so the hide method will now
get called after the close popup delay. This will
also ensure if the show method is called and the
close timeout is cancelled, the isOpen property
will not get toggled.

Closes angular-ui#4597
Fixes angular-ui#4567
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants