Fix hover-popover/tooltip flickering when mouse re-enters #14273

sgonyea · 2014-07-29T04:35:20Z

I'm not exactly happy with the code change I've made to fix this issue, but this at least demonstrates a fix (as well as the bug itself).

JSBin: http://jsbin.com/hajeta/2/

Issues:

Mouse out / mouse in over elements within the popover, the popover will continually flicker (until your mouse hovers over only the popover itself. no child elements)
Mouse out / mouse in, if the user scrolled within the popover -- that scorll position is lost (b/c the popover is recreated repeatedly)

Steps to trigger the flickering bug:

Hover over the text "Hover here!"
Mouse over the tooltip itself
Mouse-out and then quickly mouse back in (specifically, mouse back in over the text itself)
See a flicker. Then mouse up over other text. The flickering continues.

Additionally: You can scroll within the popover and see the flicker / (continual) resetting of the inner scrollbar position.

Hopefully the explanation and the demo make sense.

Thank you very much! Bootstrap is fantastic.

hnrch02 · 2014-07-29T11:05:01Z

Here's a Bin with your changes: http://jsbin.com/lopupi/1

sgonyea · 2014-07-29T17:05:07Z

Thank you for creating that, @hnrch02!

sgonyea · 2014-08-01T18:27:28Z

@cvrebert I hate to be a bother, but is there any chance I might get some quick input on this PR? My mind is still pretty fresh, on the topic, so I'd love to get it into acceptable shape if possible.

Thank you!

cvrebert · 2014-08-01T18:32:44Z

I'm not really qualified to review this in-depth. You'll have to wait for @fat to get around to reviewing this.

sgonyea · 2014-08-04T19:39:47Z

Gotcha! Thanks for taking a quick look, @cvrebert. I may be able to dig this fix in a little deeper near the triggering of the event (though that wasn't clear when I first tried). I'll take another quick stab to see if I can offer something cleaner for @fat to review.

sgonyea · 2014-08-04T20:01:23Z

I was originally triaging this bug in Bootstrap 3.1, and had with this simpler/better fix that now works. I'm going to force-push this much simpler commit in.

sgonyea · 2014-08-04T20:06:04Z

Done. Now it's one-liner with fewer code paths. I don't see any visual glitches that are specific to this change.

cvrebert · 2014-08-04T20:11:41Z

It would be great if you could add either a visual test or unit test for this.

hnrch02 · 2014-08-04T20:15:01Z

js/tooltip.js

@@ -103,6 +103,8 @@
    var self = obj instanceof this.constructor ?
      obj : $(obj.currentTarget).data('bs.' + this.type)

+    if (self && self.hoverState === 'in') return;


Should be

if (self && self.hoverState == 'in') return

cvrebert · 2014-08-04T20:23:16Z

(Filed Medium/phantomjs#209 regarding the unrelated Travis failure.)

hnrch02 · 2014-08-04T20:33:06Z

js/tooltip.js

@@ -151,6 +153,8 @@

      var $tip = this.tip()

+      if ($tip.is(':visible')) return;


sgonyea · 2014-08-04T20:34:20Z

Thanks, @cvrebert.

Also, I had to add one more commit. It seems like there still needs to be a check for whether or not the tooltip is still visible.

I'm wondering if there's a case where this can cause issues when manually triggering the tooltip... If you have a delay AND you quickly hide + show the tooltip.

So I think I need to limit my code changes to the enter/leave methods.

sgonyea · 2014-08-04T20:47:45Z

And now I've got what I think is a final PR. I just realized that hoverState IS being set to null, so I can rely on the in/out states == "still/gonna be visible".

cvrebert · 2014-08-04T20:50:28Z

So, why isn't the check != null then?

sgonyea · 2014-08-04T20:51:30Z

It probably should be. I'll fix.

sgonyea · 2014-08-04T20:57:32Z

Here's a JS bin with the current set of code changes:

http://jsbin.com/tumar/1

I'll work on adding a unit test -- are the visual tests just a manual spot-check? I wasn't sure if my addition would make it too noisy.

Thanks again!

cvrebert · 2014-08-04T21:33:50Z

@sgonyea Yeah, the visual tests are completely manual. (Which is why unit tests are preferred when possible.)

sgonyea · 2014-08-06T00:41:09Z

That was fun. The tests helped reveal that I needed to go back to my original commit, and just rely on this.$tip.is(':visible').

@cvrebert -- I've added some pretty ugly tests and decided to get some feedback. They involve a lot of teeth pulling, so I'll work on cleaning it up soon.

sgonyea · 2014-08-06T20:31:34Z

Oh right, and a JSBin with the current code: http://jsbin.com/tumar/2

hnrch02 · 2014-08-08T01:21:59Z

js/tests/unit/tooltip.js

+  test('should not reload the tooltip on subsequent mouseenter events', function () {
+    var contentData = []
+    var titleHtml = function (skipSave) {
+      var uid = $.fn.bootstrapTooltip.Constructor.prototype.getUID('tooltip').toString()


No need for the toString here.

fat · 2014-08-27T22:18:32Z

everything @hnrch02 said 👍

- is(':visible') seems to be the only reliable check, without a refactoring of how hoverState is used - tests need improvement

sgonyea · 2014-08-29T00:18:26Z

Done. Thanks a lot for the review, @hnrch02! I've made the assertion inside the tooltip a little less strict.

cvrebert · 2014-08-29T00:25:29Z

@fat So, is this now good to go?

Fix hover-popover/tooltip flickering when mouse re-enters

fat · 2014-09-07T17:36:51Z

yep, thanks!

creative3d · 2016-08-17T07:51:29Z

Hello everybody. I've found this issue looking for a fix for this joomla issue: joomla/joomla-cms#10821 . If it's not much of a trouble, please help to fix that issue, or just write some advice to joomla developers because they are not trying to fix it, they are just waiting for you. Or maybe you can provide with a hot fix or something... Thanks.

C-Lodder · 2016-08-17T08:19:18Z

@creative3d - In Joomla we're still using BS2 which is no longer officially supported. To get the issue fixed, we'll either need to do it ourselves or perhaps it has already been fixed by a 3rd party who decided to keep supporting BS2

creative3d · 2016-08-17T08:26:55Z

@C-Lodder, "it has already been fixed" well, it wasn't, as far as I know, I was just able to pay their attention on this from the third attempt two month ago, no movements since then. As for fixing ourselves, I'd like to, but I'm not a developer, have no idea how. Thanks for answering.

C-Lodder · 2016-08-17T08:36:33Z

I'll look into it: joomla/joomla-cms#10821 (comment)

creative3d · 2016-08-17T08:39:12Z

Thanks a lot!

cvrebert added the js label Jul 29, 2014

cvrebert added this to the v3.2.1 milestone Jul 29, 2014

hnrch02 reviewed Aug 4, 2014
View reviewed changes

js/tooltip.js

@@ -151,6 +153,8 @@

var $tip = this.tip()

if ($tip.is(':visible')) return;

Copy link

Collaborator

hnrch02 Aug 4, 2014

Choose a reason for hiding this comment

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

Semicolon.

hnrch02 reviewed Aug 8, 2014
View reviewed changes

sgonyea force-pushed the master branch from dc1f236 to 439d5d9 Compare August 29, 2014 00:14

sgonyea force-pushed the master branch from 439d5d9 to eb67aee Compare August 29, 2014 00:17

Fix hover-tooltip flickering when mouse re-enters

aac0e16

- is(':visible') seems to be the only reliable check, without a refactoring of how hoverState is used - tests need improvement

sgonyea force-pushed the master branch from eb67aee to aac0e16 Compare August 29, 2014 00:17

fat added a commit that referenced this pull request Sep 7, 2014

Merge pull request #14273 from sgonyea/master

c22b270

Fix hover-popover/tooltip flickering when mouse re-enters

fat merged commit c22b270 into twbs:master Sep 7, 2014

cvrebert mentioned this pull request Sep 7, 2014

v3.3.0 ship list #13952

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix hover-popover/tooltip flickering when mouse re-enters #14273

Fix hover-popover/tooltip flickering when mouse re-enters #14273

sgonyea commented Jul 29, 2014

hnrch02 commented Jul 29, 2014

sgonyea commented Jul 29, 2014

sgonyea commented Aug 1, 2014

cvrebert commented Aug 1, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

cvrebert commented Aug 4, 2014

hnrch02 Aug 4, 2014

cvrebert commented Aug 4, 2014

hnrch02 Aug 4, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

cvrebert commented Aug 4, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

cvrebert commented Aug 4, 2014

sgonyea commented Aug 6, 2014

sgonyea commented Aug 6, 2014

hnrch02 Aug 8, 2014

fat commented Aug 27, 2014

sgonyea commented Aug 29, 2014

cvrebert commented Aug 29, 2014

fat commented Sep 7, 2014

creative3d commented Aug 17, 2016

C-Lodder commented Aug 17, 2016

creative3d commented Aug 17, 2016

C-Lodder commented Aug 17, 2016

creative3d commented Aug 17, 2016

		@@ -151,6 +153,8 @@

		var $tip = this.tip()

		if ($tip.is(':visible')) return;

Fix hover-popover/tooltip flickering when mouse re-enters #14273

Fix hover-popover/tooltip flickering when mouse re-enters #14273

Conversation

sgonyea commented Jul 29, 2014

hnrch02 commented Jul 29, 2014

sgonyea commented Jul 29, 2014

sgonyea commented Aug 1, 2014

cvrebert commented Aug 1, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

cvrebert commented Aug 4, 2014

hnrch02 Aug 4, 2014

Choose a reason for hiding this comment

cvrebert commented Aug 4, 2014

hnrch02 Aug 4, 2014

Choose a reason for hiding this comment

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

cvrebert commented Aug 4, 2014

sgonyea commented Aug 4, 2014

sgonyea commented Aug 4, 2014

cvrebert commented Aug 4, 2014

sgonyea commented Aug 6, 2014

sgonyea commented Aug 6, 2014

hnrch02 Aug 8, 2014

Choose a reason for hiding this comment

fat commented Aug 27, 2014

sgonyea commented Aug 29, 2014

cvrebert commented Aug 29, 2014

fat commented Sep 7, 2014

creative3d commented Aug 17, 2016

C-Lodder commented Aug 17, 2016

creative3d commented Aug 17, 2016

C-Lodder commented Aug 17, 2016

creative3d commented Aug 17, 2016