New insert strategy for tooltip breaks tooltips/popovers on table cells

[andriijas]

This change breaks the possibilities of having tooltips and popovers on table cells.

88b1e44#L3L1086

Please let me know the reasoning behind the change of strategy so that I get an understanding of If I may have to change my code.

Thanks

[demike]

It fixes a load of z-index issues + when a modal is closed an open popover did not hide
Can you provide a fiddle, what exactly doesn't work

[hpbuniat]

The change also causes problems, when using a tooltip inside an element with overflow:hidden & position:relative.
http://jsfiddle.net/FKCjj/1/

[andriijas]

http://jsfiddle.net/mvgRC/12/ demonstrates td issues with insertAfter

[andriijas]

Ive been thinking and concluded that its better to wrap text with a span and add the popover there instead of putting popovers on btn groups, tds etc. Or? The new insert strategy is way more powerful after all.

[Yohn]

@andriijas that solution seems to work with <td>s, but not for the btn-group or @hpbuniat's problem
I was trying your solution with popovers and tooltips on button groups here - http://jsfiddle.net/GgmCr/3/ and as you can see whenever I put it on the <span> within the buttons it didnt work =/

[andriijas]

@Yohn that is a bug for sure!

[demike]

I think the new insert strategy needs some tweaking: now it uses insertAfter
It should be appendTo (that means insert it in the selected element) except for input, table and tr

if table, input: --> .insertAfter
if tr: .parent().insertAfter

[Yohn]

@demike that does make sense, and I could see it work with the btn-groups but then the tooltip/popover will be taking on the styles of its parent element.
and I dont think it would fix @hpbuniat's problem either with the container being smaller than the popover or tooltip..
I also found that putting the popover and tooltips within a collapse menu it hides some of it, similar to @hpbuniat -- http://jsfiddle.net/qgyS7/7/

maybe it would be good to decide where it gets appended to manually with an option.. something like

.popover({
  appendTo: $('body')
})

[WilliamStam]

i would be happy with a by default it inserts after the element but with a config option it attaches it to whatever element you specify. like the old way appendTo((element)?element:body) type thing

options
parent, body, #somewhere, etc

for html api

data-container=""

for javascript

popover({
  container: 'parent'
})

parent would then be as it currently is (in 2.2.~) and is the default

if you specify the container it then uses that container if it finds it if not then default back to parent

---

as it is now

$tip
          .detach()
          .css({ top: 0, left: 0, display: 'block' })
          .insertAfter(this.$element)

could do something like

 $tip
          .detach()
          .css({ top: 0, left: 0, display: 'block' })
if (this.options.container=='parent'){
  $tip.insertAfter(this.$element)
} else {
  $tip.appendTo(this.options.container)
}

i would recomend an array type thing tho..

insertAfter = element, self
appendTo = body, document

that way you can control which to make use of depending on the option used

[andriijas]

@WilliamStam I like that approach. Woudl be awesome if you could make a pull request and maybe they will merge it :)

[WilliamStam]

@andriijas no pull requests are being accepted if they dont include tests (i dont know how to make the tests). i also seriously dont know how to code like they have in BS. lemme see if i can do something with it tho

[Yohn]

@WilliamStam the tests are pretty easy, they have examples within the js/tests/unit/ directory that you can follow.. just add the changes to your own test at the bottom and add it to the pull request. I'm also thinking about giving this one a shot.
your idea for going about it looks good though, I'm totally down to try and help solve this problem

[WilliamStam]

@Yohn just a thought... if the whole appendTo thing is cause of the modals then why not do a

var $modal = this.$element.closest(.modal);
if ($modal.length){ 
$tip.appendTo($modal)
} else {
 $tip.insertAfter(this.$element)
}

[Yohn]

I dont think we should put anything within the tooltip plugin referring to the modals itself but that might be a way to actually get it to work correctly without having to use the option for a container..

heres how the tip got shown in the previous version
2.1.1 version

        $tip
          .remove()
          .css({ top: 0, left: 0, display: 'block' })
          .appendTo(inside ? this.$element : document.body)

2.2.2 version

        $tip
          .detach()
          .css({ top: 0, left: 0, display: 'block' })
          .insertAfter(this.$element)

Id have to do some more thinking about how it should go, @fat or @markdotto might have a better idea though

we also need to get the tooltip('toggle') working again in the main branch, #5768 - pull request to fix it

[WilliamStam]

i have a fix.. just trying to figure out how to do the tests...

and needs a bit of testing i suppose

 $tip
          .detach()
          .css({ top: 0, left: 0, display: 'block' })

switch (this.options.container||'parent') {
              case "parent":
                  $tip.insertAfter(this.$element);
                  break;
              case "body":
                  $tip.appendTo(document.body);
                  break;
              default :
                  var container = $(this.options.container);
                  if (container.length){
                      $tip.appendTo(container);
                  } else {
                      $tip.insertAfter(this.$element);
                  }
          }

[WilliamStam]

http://puu.sh/1nsCx test working

[WilliamStam]

ok would somone be so kind as to explain why i get a build failed? then it when clicking details it shows me

Failed — The Travis build failed (Details)

bleh..

i also found out the details its ment to show shows some other project instead of this 1 in opera... it works in chrome tho..

opera: http://puu.sh/1nsGX
chrome: http://puu.sh/1nsHd

i have to go,, so if somone else can get it to work then cool.. do a pull request to them. basicaly just added the tests and the switch i posted earlier

[WilliamStam]

the tests i added.

test("should place tooltips inside the body", function () {
        $.support.transition = false
        var tooltip = $('<a href="#" rel="tooltip" title="Another tooltip"></a>').appendTo('#qunit-fixture').tooltip({container:'body'}).tooltip('show')

        ok($("body > .tooltip").length, 'inside the body')
        ok(!$("#qunit-fixture > .tooltip").length, 'not found in parent')
        tooltip.tooltip('hide')
    })

    test("should place tooltips inside the parent", function () {
        $.support.transition = false
        var tooltip = $('<a href="#" rel="tooltip" title="Another tooltip"></a>').appendTo('#qunit-fixture').tooltip({container:'parent'}).tooltip('show')

        ok($("#qunit-fixture > .tooltip").length, 'inside the parent')
        ok(!$("body > .tooltip").length, 'not found in body')
        tooltip.tooltip('hide')
    })
    test("should place tooltips inside an element", function () {
        $.support.transition = false
        var tooltip = $('<a href="#" rel="tooltip" title="Another tooltip"></a><div id="tooltiphere"></div>').appendTo('#qunit-fixture').tooltip({container:'#tooltiphere'}).tooltip('show')

        ok($("#tooltiphere > .tooltip").length, 'inside the element')
        ok(!$("body > .tooltip").length, 'not in the body')
        ok(!$("#qunit-fixture > .tooltip").length, 'not in the parent')

        tooltip.tooltip('hide')
    })

[WilliamStam]

k pull request done and passing. lets see if BS accepts it.

@Yohn made this fiddle (incase you not following the discussion on the pull request) http://jsfiddle.net/JMskU/17/ shows that the fix will work (hopefully)

[Yohn]

@ajbeaven to follow up with the other thread - I really dont know why they took the "inside" option out.. it had to be with the fix for the modals not displaying the tooltips / popovers..
I didnt know why the inside = /in/.test(placement); was there either until I read that thread, and now it makes sense..
the problem we would have with using the "in left" position would be the tooltip/popover taking on the styles with the parent element, so if it was within the navbar link or a button it could change the style of the popover.. The fix we came up with in this thread fixes that from happening by choosing where the tooltip container would be

$('.modal-body [title]').tooltip({
   container: '.modal-body'
})
$('[title]').tooltip({
  container: 'body'
}) 

and if no container if selected it would be appended after the element that called the tooltip

[Yohn]

@ajbeaven I believe they did have a higher z-index, but for some reason it wasnt working. I'm sure @fat or @markdotto would be able to answer that one better..
I'm gonna look into it some more to see if we can come up with a way to not break the older versions with using "in" on the placement as well

[Yohn]

I found the commit that changed the insert strategy of the tooltips - 88b1e44 and @fat noted it was for the z-index issues

[WilliamStam]

i had problems with the modal closing but the tooltips / popovers staying visible. saw something similar here as well. suppose its that as well as the zindex thing?

[andriijas]

For what its worth this change to tooltips and popovers should probably have been put in 3.0 which is backwards incompatible.

However its in place and I believe the new insert strategy is far superior to the old one. Just wrap your stuff with a span and add the tooltip/popover there instead.

[WilliamStam]

you still have the issue of overflow hidden / adding a tooltip to for instance the small in a

<h1> welcome <small> user </small></h1>

it applies the h style to the popover.. so as to "superior"... i dont know so much

[fat]

As mentioned, we changed this to fix z-indexing (the api stayed the same… so it didn't warrant a 3.0.0 release). If you want to add a tooltip to a table cell - just wrap in a span/div and you should be all set (apologies on that though, might be worth adding a note to the docs about that).

If you guys want to open a separate issue for the button problem, or anything else please do. thanks!

[vladimirkl]

I don't agree with closing this issue. Wrapping popover in span / div does not solve it. Table cell has padding and popover arrow points to some place inside cell. This can be fixed with css by removing padding from cell, but all this stuff seems to be hackish. I think that best way is to allow us to choose popover container. Single insert strategy does not work everywhere.

[conradfr]

Please, can someone explain to me how to make a tooltip work on a table/td, maybe w/ a jsfiddle ? I may be stupid but I don't get that div wrapping fix. My app is broken for weeks now ...

Edit: nevermind, figured it out. You need to wrap all your td content in a div and move the tooltip data in the div (and update your jquery selector).

[robertmclaws]

I'm sorry, but this problem should never have been introduced in the first place. "Just wrap your TD content in a div" is fine as long as you have control over the content. But if you're using something like the KendoUI Grid, then you have no control whatsoever. Now I get to diff the two files and try to roll the Popover backward while keeping everything else.

Change for the sake of change is not always a bad thing... what was wrong with the way 2.1 did it?

[WilliamStam]

what about the

<h1> this is my heading <small rel="popover">hover here for more info</small> </h1>

i have a project that uses absolute positioning the "new" way ends up in a lot of popovers that get cut off. being able to define its container is definately the better option.

recommendation: the default does the new changes, but at least give us an option to set parent container

[Yohn]

heres what I have so far with setting a container for the tooltips / popovers -- http://jsfiddle.net/R23KB/4/
I wanted to play with it some more to try and fix the modal-body issue with the popovers not following the triggered element, and it getting cut off by the sides, header, and footer when it does follow it..
to get the popover to follow the triggered element, you'd use '.modal-body' for the container
to get the popover / tooltip to not get cut off by the sides, header, and footer use '.modal' for the container..

the problem with them getting cut off by the sides, header, and footer also applies to dropdown menus within modal-body

but as a side note, I do like old way we inserted the tooltips into the page with appendTo() vs insertAfter()

[benatwork99]

I just got bitten by this one as well, cut-off tooltips now. Wrapping the content in a div or span wouldn't solve this.

Also - although the API didn't change wasn't this still a backwards incompatible change as our previous code no longer works the same calling the same API functions? In which case according to semver it should have been a major version bump, right?

[Yohn]

@benatwork99 I can see that being a valid point, and since this thread is closed, I'd recommend bringing it up in a new issue if fat or mdo dont reply here

[ProIcons]

http://jsfiddle.net/FRbhW/1/
Still with your Updated Code (http://jsfiddle.net/R23KB/4/)
it is buggy

[Yohn]

@ProIcons -- http://jsfiddle.net/FRbhW/4/ You have to set the container option when you call .tooltip() and also, that code in the fiddle was an older one we started out with, the new one is in pull request #6378 which got merged into 2.3

[cvrebert]

Just to give a TL;DR for future readers: The solution as of v2.3.x is to set the container option of the tooltip/popover to body. For example:

$(...).tooltip({container: 'body'});

Or using a data-* attribute in the HTML:

data-container="body"

[andrewgunn]

We found more bugs regarding popovers/tooltips in version 2.3.x, so reverted back to 2.2.2. For example, tooltips on the left appear over the element itself (http://twitter.github.io/bootstrap/javascript.html#tooltips) and the dynamic placement of the tip arrow when the element is in one of the right corners is broken (http://codepen.io/andrewgunn/pen/HlyGz). We also had an issue with the popover width being too small (in some scenarios) making it narrow, tall and appear off screen.

As a result, we can't take advantage of the container option and are forced to remove tooltips from th and td elements.

[grekodev]

the solution is this:
put the div after td; Example

 <td class="Sale_td">
     <div  data-trigger="focus" data-toggle="popover">
             [....]
      </div>
 </td>