From f3204febaee0fa46b24d447901b5aacfe6183617 Mon Sep 17 00:00:00 2001 From: Marco Ziech Date: Sat, 10 Aug 2013 20:39:55 +0200 Subject: [PATCH 1/4] Tooltip: Register event handlers before content is loaded. Fixes #8740 - Tooltip: Does not hide consistently with dynamically loaded content. --- tests/unit/tooltip/tooltip_events.js | 42 ++++++++++++++++++++++++++++ ui/jquery.ui.tooltip.js | 18 +++++++----- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/tests/unit/tooltip/tooltip_events.js b/tests/unit/tooltip/tooltip_events.js index de16471aedf..7f15eb05fd3 100644 --- a/tests/unit/tooltip/tooltip_events.js +++ b/tests/unit/tooltip/tooltip_events.js @@ -54,4 +54,46 @@ test( "focus events", function() { element.trigger( "focusout" ); }); +// http://bugs.jqueryui.com/ticket/8740 +asyncTest( "content: async callback loses focus before load", function() { + expect( 1 ); + var element = $( "#tooltipped1" ).tooltip({ + content: function( response ) { + element.trigger( "mouseleave" ); + setTimeout(function () { + response( "sometext" ); + setTimeout(function () { + ok(!$( "#" + element.data( "ui-tooltip-id" ) ).is( ":visible" ), "Tooltip should not display" ); + start(); + }); + }); + } + }); + element.trigger( "mouseover" ); + element.tooltip( "destroy" ); +}); + +// https://github.com/jquery/jquery-ui/pull/992/files#r5667799 +asyncTest( "content: close should only be called once, even if content is set multiple times", function() { + expect( 1 ); + var element = $( "#tooltipped1" ).tooltip(); + var closecount = 0; + element.bind( "tooltipopen", function( event ) { + element.tooltip( "option", "content", "one" ); + element.tooltip( "option", "content", "two" ); + element.trigger( "mouseleave" ); + }); + element.bind( "tooltipclose", function( event ) { + closecount++; + if (closecount === 1) { + setTimeout(function () { + equal( closecount, 1, "Close event handler should be called once" ); + element.tooltip( "destroy" ); + start(); + }); + } + }); + element.trigger( "mouseover" ); +}); + }( jQuery ) ); diff --git a/ui/jquery.ui.tooltip.js b/ui/jquery.ui.tooltip.js index 5df93a00245..f239320a6e9 100644 --- a/ui/jquery.ui.tooltip.js +++ b/ui/jquery.ui.tooltip.js @@ -174,6 +174,7 @@ $.widget( "ui.tooltip", { }); } + this._registerCloseHandlers( event, target ); this._updateContent( target, event ); }, @@ -188,13 +189,14 @@ $.widget( "ui.tooltip", { } content = contentOption.call( target[0], function( response ) { - // ignore async response if tooltip was closed already - if ( !target.data( "ui-tooltip-open" ) ) { - return; - } // IE may instantly serve a cached response for ajax requests // delay this call to _open so the other call to _open runs first that._delay(function() { + // ignore async response if tooltip was closed already + if ( !target.data( "ui-tooltip-open" ) ) { + return; + } + // jQuery creates a special event for focusin when it doesn't // exist natively. To improve performance, the native event // object is reused and the type is changed. Therefore, we can't @@ -212,7 +214,7 @@ $.widget( "ui.tooltip", { }, _open: function( event, target, content ) { - var tooltip, events, delayedShow, + var tooltip, delayedShow, positionOption = $.extend( {}, this.options.position ); if ( !content ) { @@ -281,8 +283,10 @@ $.widget( "ui.tooltip", { } this._trigger( "open", event, { tooltip: tooltip } ); + }, - events = { + _registerCloseHandlers: function( event, target ) { + var events = { keyup: function( event ) { if ( event.keyCode === $.ui.keyCode.ESCAPE ) { var fakeEvent = $.Event(event); @@ -291,7 +295,7 @@ $.widget( "ui.tooltip", { } }, remove: function() { - this._removeTooltip( tooltip ); + this._removeTooltip( this._find( target ) ); } }; if ( !event || event.type === "mouseover" ) { From 0520a49f35ac96af7b7a68f11e77738712a70fcf Mon Sep 17 00:00:00 2001 From: Marco Ziech Date: Wed, 14 Aug 2013 23:48:18 +0200 Subject: [PATCH 2/4] Tooltip tests: jshint cleanup --- tests/unit/tooltip/tooltip_events.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/tooltip/tooltip_events.js b/tests/unit/tooltip/tooltip_events.js index 7f15eb05fd3..cd0ec4a5386 100644 --- a/tests/unit/tooltip/tooltip_events.js +++ b/tests/unit/tooltip/tooltip_events.js @@ -76,14 +76,14 @@ asyncTest( "content: async callback loses focus before load", function() { // https://github.com/jquery/jquery-ui/pull/992/files#r5667799 asyncTest( "content: close should only be called once, even if content is set multiple times", function() { expect( 1 ); - var element = $( "#tooltipped1" ).tooltip(); - var closecount = 0; - element.bind( "tooltipopen", function( event ) { + var element = $( "#tooltipped1" ).tooltip(), + closecount = 0; + element.bind( "tooltipopen", function() { element.tooltip( "option", "content", "one" ); element.tooltip( "option", "content", "two" ); element.trigger( "mouseleave" ); }); - element.bind( "tooltipclose", function( event ) { + element.bind( "tooltipclose", function() { closecount++; if (closecount === 1) { setTimeout(function () { From 45a6e90258e9dfb5b2d8c58b3e1978d2b45d258f Mon Sep 17 00:00:00 2001 From: Marco Ziech Date: Sun, 9 Feb 2014 12:21:21 +0100 Subject: [PATCH 3/4] Tooltip: Clean up after merge conflict. Fixes #8740 - Tooltip: Does not hide consistently with dynamically loaded content. --- ui/tooltip.js | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/ui/tooltip.js b/ui/tooltip.js index 736f4ef07a4..08eaead3cb7 100644 --- a/ui/tooltip.js +++ b/ui/tooltip.js @@ -7,11 +7,6 @@ * http://jquery.org/license * * http://api.jqueryui.com/tooltip/ - * - * Depends: - * jquery.ui.core.js - * jquery.ui.widget.js - * jquery.ui.position.js */ (function( factory ) { if ( typeof define === "function" && define.amd ) { @@ -213,10 +208,6 @@ return $.widget( "ui.tooltip", { } content = contentOption.call( target[0], function( response ) { - // ignore async response if tooltip was closed already - if ( !target.data( "ui-tooltip-open" ) ) { - return; - } // IE may instantly serve a cached response for ajax requests // delay this call to _open so the other call to _open runs first that._delay(function() { @@ -333,9 +324,6 @@ return $.widget( "ui.tooltip", { fakeEvent.currentTarget = target[0]; this.close( fakeEvent, true ); } - }, - remove: function() { - this._removeTooltip( this._find( target ) ); } }; @@ -343,7 +331,7 @@ return $.widget( "ui.tooltip", { // tooltips will handle this in destroy. if ( target[ 0 ] !== this.element[ 0 ] ) { events.remove = function() { - this._removeTooltip( tooltip ); + this._removeTooltip( this._find( target ) ); }; } From 556559cbe96c39c9b6b96f0db3d2e1457aaa4d8a Mon Sep 17 00:00:00 2001 From: Marco Ziech Date: Sun, 9 Feb 2014 13:02:33 +0100 Subject: [PATCH 4/4] Tooltip tests: Code style cleanup. Fixes #8740 - Tooltip: Does not hide consistently with dynamically loaded content. --- tests/unit/tooltip/tooltip_events.js | 42 --------------------------- tests/unit/tooltip/tooltip_options.js | 21 ++++++++++++++ 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/tests/unit/tooltip/tooltip_events.js b/tests/unit/tooltip/tooltip_events.js index cd0ec4a5386..de16471aedf 100644 --- a/tests/unit/tooltip/tooltip_events.js +++ b/tests/unit/tooltip/tooltip_events.js @@ -54,46 +54,4 @@ test( "focus events", function() { element.trigger( "focusout" ); }); -// http://bugs.jqueryui.com/ticket/8740 -asyncTest( "content: async callback loses focus before load", function() { - expect( 1 ); - var element = $( "#tooltipped1" ).tooltip({ - content: function( response ) { - element.trigger( "mouseleave" ); - setTimeout(function () { - response( "sometext" ); - setTimeout(function () { - ok(!$( "#" + element.data( "ui-tooltip-id" ) ).is( ":visible" ), "Tooltip should not display" ); - start(); - }); - }); - } - }); - element.trigger( "mouseover" ); - element.tooltip( "destroy" ); -}); - -// https://github.com/jquery/jquery-ui/pull/992/files#r5667799 -asyncTest( "content: close should only be called once, even if content is set multiple times", function() { - expect( 1 ); - var element = $( "#tooltipped1" ).tooltip(), - closecount = 0; - element.bind( "tooltipopen", function() { - element.tooltip( "option", "content", "one" ); - element.tooltip( "option", "content", "two" ); - element.trigger( "mouseleave" ); - }); - element.bind( "tooltipclose", function() { - closecount++; - if (closecount === 1) { - setTimeout(function () { - equal( closecount, 1, "Close event handler should be called once" ); - element.tooltip( "destroy" ); - start(); - }); - } - }); - element.trigger( "mouseover" ); -}); - }( jQuery ) ); diff --git a/tests/unit/tooltip/tooltip_options.js b/tests/unit/tooltip/tooltip_options.js index 17f0a423775..026da4eb32a 100644 --- a/tests/unit/tooltip/tooltip_options.js +++ b/tests/unit/tooltip/tooltip_options.js @@ -71,6 +71,27 @@ asyncTest( "content: sync + async callback", function() { }).tooltip( "open" ); }); +// http://bugs.jqueryui.com/ticket/8740 +asyncTest( "content: async callback loses focus before load", function() { + expect( 1 ); + + var element = $( "#tooltipped1" ).tooltip({ + content: function( response ) { + element.trigger( "mouseleave" ); + setTimeout(function() { + response( "sometext" ); + setTimeout(function() { + ok( !$( "#" + element.data( "ui-tooltip-id" ) ).is( ":visible" ), + "Tooltip should not display" ); + element.tooltip( "destroy" ); + start(); + }); + }); + } + }); + element.trigger( "mouseover" ); +}); + test( "content: change while open", function() { expect( 2 ) ; var element = $( "#tooltipped1" ).tooltip({