Skip to content

Tooltip: Register event handlers before content is loaded #1456

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions tests/unit/tooltip/tooltip_options.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,28 @@ 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 ) {
setTimeout(function() {
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" );
});
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was untouched.


test( "content: change while open", function() {
expect( 2 ) ;
var element = $( "#tooltipped1" ).tooltip({
Expand Down
26 changes: 19 additions & 7 deletions ui/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ return $.widget( "ui.tooltip", {
});
}

this._registerCloseHandlers( event, target );
this._updateContent( target, event );
},

Expand All @@ -214,13 +215,16 @@ 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() {

// 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
Expand All @@ -238,7 +242,7 @@ return $.widget( "ui.tooltip", {
},

_open: function( event, target, content ) {
var tooltipData, tooltip, events, delayedShow, a11yContent,
var tooltipData, tooltip, delayedShow, a11yContent,
positionOption = $.extend( {}, this.options.position );

if ( !content ) {
Expand Down Expand Up @@ -316,8 +320,10 @@ return $.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);
Expand All @@ -331,7 +337,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 ).tooltip );
Copy link
Member Author

Choose a reason for hiding this comment

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

This line was changed from this._find( target ) to this._find( target ).tooltip because we now store structured data, not just the tooltip element.

};
}

Expand All @@ -352,6 +358,12 @@ return $.widget( "ui.tooltip", {

// The tooltip may already be closed
if ( !tooltipData ) {

// We set ui-tooltip-open immediately upon open (in open()), but only set the
// additional data once there's actually content to show (in _open()). So even if the
// tooltip doesn't have full data, we always remove ui-tooltip-open in case we're in
// the period between open() and _open().
target.removeData( "ui-tooltip-open" );
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key thing that was needed after rebasing.

return;
}

Expand Down