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.

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

Tooltip occasionally won't hide when using a hide delay #14519

Closed
wants to merge 1 commit into from
Closed

Tooltip occasionally won't hide when using a hide delay #14519

wants to merge 1 commit into from

Conversation

iamphill
Copy link

@iamphill iamphill commented Sep 3, 2014

Fix for issue #14375

Error demo can be found on http://jsfiddle.net/d45cc0rs/
Fixed demo can be found on http://jsfiddle.net/7ga1mz6r/

@iamphill iamphill changed the title Fix for issue 14375 Tooltip occasionally won't hide when using a hide delay Sep 3, 2014
that.$element.trigger('shown.bs.' + that.type)
that.hoverState = null

if (oldHoverState === 'out') that.leave(that)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non strict comparison here, please.

Copy link
Author

Choose a reason for hiding this comment

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

Ah flip! Thought I got them things! 😎

Fixed in 54ee72470bad0eb72468fa06db39eb084c9a3f44

@cvrebert cvrebert added the js label Sep 3, 2014
@cvrebert cvrebert added this to the v3.2.1 milestone Sep 3, 2014
@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 8, 2014

/to @fat for review

@cvrebert
Copy link
Collaborator

cvrebert commented Sep 8, 2014

(And we should squash the commits before merging)

@iamphill
Copy link
Author

iamphill commented Sep 8, 2014

@cvrebert Squashed the commits into 1 😄

@hnrch02
Copy link
Collaborator

hnrch02 commented Oct 23, 2014

The unit test doesn't consistently fail on current master.

@iamphill
Copy link
Author

@hnrch02 I can't get the tests to fail? Rebased with master to get latest changes and ran the tests so many times...

@hnrch02
Copy link
Collaborator

hnrch02 commented Oct 23, 2014

That's kinda bad since we then can't detect any regression about this.

@iamphill
Copy link
Author

Tell me about it.
It used to reliably fail when the timing was longer, could put it back to the previous timing?

@hnrch02
Copy link
Collaborator

hnrch02 commented Oct 23, 2014

Yeah, try that. Will be necessary at some point anyways because of #14851.

@iamphill
Copy link
Author

Think I've found the issue with the unit tests. The transitions are disabled on https://github.com/twbs/bootstrap/blob/master/js/tests/index.html#L55

And then in the tooltip.js file it fires the complete func immediately if $.support.transition is false https://github.com/twbs/bootstrap/blob/master/js/tooltip.js#L214-L218

I can reliably recreate the issue & see the fix in the visual tests. Obviously not great... Will see what I can do to get these tests to reliably fail.

@cvrebert cvrebert mentioned this pull request Oct 26, 2014
@hnrch02
Copy link
Collaborator

hnrch02 commented Oct 26, 2014

Yeah, testing without transitions is hard. I'll merge this anyway as we know it fixes the issue. If you come up with a solution though please do share.

@hnrch02 hnrch02 closed this in 9740d8b Oct 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants