Allow adding html elements as tooltip content #983
Conversation
…nd jQuery objects to be set as content. Added tests for these scenarios.
|
Thanks Daniel. All feature requests need to go through a ticket, but this seems like a reasonable addition. Can you file a ticket for this at http://bugs.jqueryui.com/newticket? I'll also ping the rest of the team to discuss this. In the meantime, I'll comment on the changes, since I think this will end up landing. |
| @@ -7,3 +7,4 @@ docs | |||
| *.patch | |||
| .DS_Store | |||
| .settings | |||
| *.sw? | |||
There was a problem hiding this comment.
You should add this to your global gitignore via core.excludesfile. I actually didn't realize we had so many files in here, I'm going to rip these out :-P
|
We can't use |
| } | ||
| }).tooltip( "open" ); | ||
| }); | ||
| ); |
There was a problem hiding this comment.
This looks like an error and doesn't need to be there.
There was a problem hiding this comment.
Yeah, looks like this was accidentally pulled from line 98
|
Created a new ticket: http://bugs.jqueryui.com/ticket/9278 |
|
I modified the code to reflect all of the changes you guys posted. The conditions are much simpler. @scottgonzalez should I update the code to do a deep copy of the element being passed and attach that instead? Edit: I guess I don't understand what you mean when you said, "I'm also concerned about what will happen if the same element is used for multiple instances" |
Cloning would require stripping all
The concern is multiple tooltips open at the same time, using the same element.
Did you try with simultaneous tooltips? I'm pretty sure that the current implementation will yank the element out of the first tooltip. |
|
We may just want to document that if you use an element, it must be limited to one tooltip. I would hope it'd be rare for anything else to happen anyway. |
|
I see what you're saying now and you're right about it yanking the element out of the first/previous tooltip(s). I was thinking that there could possibly be a |
|
We definitely won't be adding an option to control another option, so |
|
Fair enough, I spent some time trying to figure out if that would be a silly suggestion but I obviously don't always have the best judgment. |
| content: element, | ||
| open: function( event, ui ) { | ||
| // Getting just the contents of the tooltip wrapper. | ||
| equal( ui.tooltip[ 0 ].firstChild.innerHTML, content ); |
There was a problem hiding this comment.
This comparison looks questionable. Not sure about a better way to test this. Maybe domEqual can be used here. Also applies to the equal call below.
|
Thanks @daniel-o for the initial implementation. |
Can now pass HTMLElement and jQuery objects to the tooltip content option.
Added tests as well. Hopefully those look right.
Pretty much just changed the conditions that are checked for the contentOption variable. I put some of the conditions on separate lines because I thought it increased readability. So if someone could look at the formatting and scold me if I did it wrong, that would be great.