Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Popover with no animation doesn't hide (after toggle) #10568

Closed
fkling opened this issue Sep 11, 2013 · 7 comments
Closed

Popover with no animation doesn't hide (after toggle) #10568

fkling opened this issue Sep 11, 2013 · 7 comments
Labels
js
Milestone
v3.1.0

Comments

@fkling
Copy link

fkling commented Sep 11, 2013

If you use $().popover with no animation and show the popover with $().popover('toggle') (i.e. trigger: 'manual'), then it doesn't hide after $().popover('hide') is called.

Demo: http://jsfiddle.net/sJtrg/

The problem seems to be that inside the Tooltip.prototype.toggle function, enter and leave are called instead of show and hide:

self.tip().hasClass('in') ? self.leave(self) : self.enter(self)

This was changed as part #7163 (9257bdc).

Inside enter, the following property is set:

self.hoverState = 'in'

and inside hide, the popover is only removed if this property is not 'in':

function complete() {
  if (that.hoverState != 'in') $tip.detach()
}

The same problem exists with animation of course, but since element was faded out, it's not visible (even though it doesn't get detached).

I would have changed the calls from leave and enter to hide and show again, but I didn't go through the other issue completely, so I don't know if that would reintroduce the other bug. There might be a different way to solve this problem.
@mauvm
Copy link

mauvm commented Sep 16, 2013

The enter and leave functions in their turn call show and hide (after the given delay). So that's not the problem. After some digging I also found this function:

function complete() {
    if (that.hoverState != 'in') $tip.detach()
}

Which checks the hoverState variable, that is set by the enter and leave methods. I believe this hover state is only necessary for trigger: 'hover'. So I added a variable hoverActivated to ignore the hover state in other cases, which worked for me.

Here are the changes I made: http://pastebin.com/Jq6ebg05

When I'll find the time, I will test it more thoroughly and make a PR out if this (if nobody else already beat me to it).

I hope this fixes your problem for now.

@mauvm mauvm mentioned this issue Sep 22, 2013
Closed
@BenBish
Copy link

BenBish commented Sep 26, 2013

@mauvm Thanks very much, your fix appears to solve my issue that I was having.

@sorahn
Copy link

sorahn commented Nov 13, 2013

Since bootstrap has so nicely evented all of their javascript, it's easy to do this with out hacking core.

$('body').on('hidden.bs.popover', function() {
  $('.popover:not(.in)').hide().detach();
});

That being said, still a bug.

@thephw
Copy link

thephw commented Nov 13, 2013

This issue also is present by default in IE8, though the aforementioned patch fixes it brilliantly thanks @mauvm !

thephw added a commit to thephw/bootstrap that referenced this issue Nov 13, 2013
@thephw
Fix issue twbs#10568 - hides tooltip/popovers when not using CSS anim…
3870373 
…ations
@thephw
Copy link

thephw commented Nov 13, 2013

Pull Request #11463

@mauvm
Copy link

mauvm commented Nov 13, 2013

@patrickwiseman The PR you proposed does not integrate the full fix and will now always detach the tip. Would you mind adding all rules that contain "hoverActivated" of http://pastebin.com/Jq6ebg05 to tooltip.js?

Oh and thanks for making a PR. :)

thephw added a commit to thephw/bootstrap that referenced this issue Nov 13, 2013
@thephw
Fix issue twbs#10568 - hides tooltip/popovers when not using CSS anim…
5db391d 
…ations
@thephw
Copy link

thephw commented Nov 13, 2013

Good catch @mauvm I was like half asleep last night, it's updated now.

@fat fat closed this as completed in 1fc08c1 Dec 24, 2013
@mdo mdo mentioned this issue Dec 25, 2013
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
js
Projects
None yet
Milestone
v3.1.0
Development

No branches or pull requests
5 participants
@fkling @sorahn @thephw @mauvm @BenBish