Skip to content

Tooltip #387

Closed
wants to merge 3 commits into from

2 participants

@bmsterling

Cleaned up code per scottgonzalez.

Just a note on the reason for:

    tooltip
        .data( "target", target )
        .addClass( this.options.tooltipClass || "" );

The tooptip variable is not set till line 139 and at this point, all the classes have been added to the tooltip. Moving the addClass( this.options.tooltipClass || "" ) out of the _tooltip method and having it live on it's own allows two things, 1) it ensures that the "target" is set so the end user can add their own classes to the tooltip based on what target was interacted with. And 2) it makes sure that, if I add a class with a fixed width as a property, that the position method will have that width to work with.

bmsterling added some commits Jul 7, 2011
@bmsterling bmsterling Position: added a way to add a flipped class based on the direction t…
…he element was flipped
6420e57
@bmsterling bmsterling Tooltip: adding in a table demo where an inner child's content will b…
…e grabbed. Also modified jquery.ui.tooltip.js to attach the target element to the tooltip element for use outside of the widget. Also added the ability to add classed to the tooltip before the position widget is called.
f1e0414
@bmsterling bmsterling Tooltip: Clean up code to match style guide 4addab3
@jzaefferer
jQuery Foundation member

Something is wrong in that modified custom demo (hovering the second cell): http://bassistance.de/i/8633c4.png
Would also be nice if it would look a bit nicer.

@jzaefferer
jQuery Foundation member

Also, could you submit the position changes in a seperate branch/pull request? They aren't related to the tooltip changes, and the commit message should reference the position ticket: http://wiki.jqueryui.com/w/page/25941597/Commit-Message-Style-Guide
http://bugs.jqueryui.com/ticket/5937

@jzaefferer
jQuery Foundation member

And finally, Scott suggested on the forum to add the target to the open and close events. That's much more useful then adding it via data, which we'd have to clean up at some later point.

With that I'm closing this pull request, as its currently too far from actually being mergable. Please let us know if you need help figuring out git/GitHub, your contributions are very welcome!

@jzaefferer jzaefferer closed this Jul 8, 2011
@bmsterling

I will post my follow up to the forum as I'd like to talk thru the whole target thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.