Tooltip layout wrong when elements outside of viewport

[Legends]

look here:
http://jsbin.com/yalohofo/1/

The layout of the tooltips gets screwed up, if the tooltips are created when outside of the viewport !

[ericmartel]

This issue seems to be caused by the function getViewportAdjustedDelta, where, on line 354 it does

delta.top = viewportDimensions.top - topEdgeOffset

In this case, topEdgeOffset is negative, so a positive top offset is added to the tooltip, making it fit in screen space.

I would be glad to fix the issue, if the project owners were so kind as to describe the expected behavior.

Here's what I propose:

• Keep the code as is for partially hidden tooltips
• Return a delta of 0, 0 for tooltips that, given their width and height, will not be displayed in the viewport, leaving them out of sight for the user

A nice to have would also be to dynamically reposition tooltips as the user scrolls, but that would be another issue.

What do you think?

[ericmartel]

Given the rounded edges of the Tooltip, adjusting the tooltips that don't fit can result in unpleasant "holes" between the arrow and the tooltip.

I'll try to look into edb221a a bit more, but it seems to me that by default, the delta shouldn't be applied unless the viewport is not the body, and if the delta is applied, the arrow should also be adjusted, which is not currently the case.

[Legends]

You don't need to reposition tooltips when the user scrolls, actually only if the user zooms in/out.
So there is still a lot of work to do....

[hnrch02]

/cc @fat about the questions/proposal of @ericmartel

[ericmartel]

Thanks for the cc

I looked a bit more at it last night, and the current implementation of the viewport is broken.

I'll add unit tests to first bug the fact that the arrows are not placed properly when the tooltip is moved by the viewport and then start by fixing this.

Then, once I am sure both the arrow and the tooltip are behaving in the same way, I'll take scrolling into account in the viewport. I don't believe anymore partially hidden tooltips should be repositionned dynamically, after reading some of the tests, the viewport addition seems to be more to handle things such as padding in containers.

[ericmartel]

Good morning!

Maybe I'm just confused because I'm new to the project / client side JS, but I have a hard time understanding the expected behavior of edb221a

While the documentation says the viewport is another element, there are a lot of assumptions with window.

I made some local changes to make it appropriately reposition the tooltip based on another element, but there's still some references to window in the unit tests that make it fail (I'm still looking into how to display the actual / expected values - very new to qunit / phantom / etc). With this code, @Legends example works fine, and I recreated a test page to validate the expected behavior in the unit tests, and it also seems fine.

@fat / @cvrebert : do you guys know what's the expected behavior of the viewport / viewport padding in the Tooltip library? Do we want to a) prevent the tooltip from touching the viewport element by n pixels or b) do we want to keep the tooltip from touching the edge of the browser by n pixels?

Either way, the current code was taking scrolling into account, which seems like a bad idea.

If you guys want a quick chat, you can easily reach me on twitter: @emartelai

Thanks

[hnrch02]

@ericmartel Maybe the discussions in #11593 and #12328 will give you more insight. And the tooltip viewport example might also be helpful.

[ericmartel]

@hnrch02 Thanks! so a) it is!

Using the viewportPadding, it is still possible to have a disconnection between the arrow and the tooltip though, so my changes wont fix this. I'll also see to remove all references to window as it doesn't make sense to me if the viewport is another element.

Thanks!

[hnrch02]

@ericmartel Looking forward to your PR!

[Legends]

Bootstraps creates the arrows through the CSS3 pseudo classes, which don't scale well and you get these offset in some cases, for example on some zoom levels.
For new browsers I would use SVG's. I have implemented a jquery validation wrapper, where I atttach the arrows as svg's (polygones) and they scale like a charm and no offsets like with :before/:after.
@ericmartel When will teh update be available? :)
One question, how do you guys test the cross browser capability of the code you implement ?

[cvrebert]

One question, how do you guys test the cross browser capability of the code you implement ?

Sauce Labs, or local machines/devices, or local VMs (for IE, typically the gratis modern.ie VMs)
And perhaps something more automated in the future, for simple cases at least; see #11410.

[ericmartel]

@Legends @hnrch02 here's the PR, hopefully it's not too bad, I leaned against unit tests and examples to make sure my changes didn't break anything*

(*) hopefully, but being new at this, I could be wrong :)

[Legends]

@ericmartel where can i download the fix you made or do I have to wait for the next release?

[ericmartel]

@Legends if you know how to build a bootstrap distribution (its explained in the Getting Started guide from getbootstrap.com), you could build this branch: https://github.com/ericmartel/bootstrap/tree/tooltip_positioning_14195

Keep in mind it's a development branch, it's possible that other bugs are in there, this is not an official release. Thanks!

[Legends]

@ericmartel Hi Eric, I've downloaded the zip from
https://github.com/ericmartel/bootstrap/tree/tooltip_positioning_14195
And I've only copied the new bootstrap.js and the new bootstrap.css into my project, but the issue is still existent.
The tooltips get screwed up when I zoom in, but only the tooltips the fall outside the vp.

Try to zoom in here:
http://www.main.somee.com/SignalRChatUIClean/Pages/jQueryValidationIII.aspx

[ericmartel]

@Legends This version of bootstrap.js does not contain the compiled changes in tooltip.js. You have to build the thing using grunt dist from a clone of the branch.

I also didn't test zooming... I can look once I get back home if the zoom is properly handled by my implementation!

[cvrebert]

@ericmartel FWIW, zooming tends to break things generally (as mentioned in our compatibility docs) (and I think this plugin in particular), so don't stress too much if zooming breaks your code.

[Legends]

@ericmartel
Sorry, I am comming from the Microsoft stack, so... i am a newbie regarding OS and github.
I thought, when I go to your brunch and click on download I will get that version..?!
I use VS2012 :)
@cvrebert Can you point me to the compatability docs please...

Zooming should not break the layout.
I took a look at qTip2 and zooming is not a problem there...

[cvrebert]

@Legends The relevant section here is http://getbootstrap.com/getting-started/#support-browser-zooming

[fat]

@ericmartel thanks for all the work on this

as chris said, don't worry about zooming or scrolling

I think the viewport element is supposed to be used to constrain auto positioning.

So for example, if your tooltip is detecting if it is overflowing a boundary it will react. This should 99% of the time be the window… that pr in theory (i believe) allows you to set the boundary element to a different element

[mdo]

Punting as a won't fix—v4 includes a new positioning library for our affix, tooltips, and popovers.