Skip to content
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

Auto positioning of tooltip #7996

Closed
wants to merge 2 commits into from
Closed

Auto positioning of tooltip #7996

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 26, 2013

This patch changes the positioning of the tooltip when there is no room for the tooptip in the position it would normally show.

As discussed here: #7399

This patch changes the positioning of the tooltip when there is no room for the tooptip in the position it would normally show.

As discussed here: #7399
placement = 'left'
} else if (tpt.top > $(window).scrollTop()) {
placement = 'top'
} else if (tpt.top > $(window).scrollTop()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless my eyes are failing me, this condition is exactly the same as that of the previous else if, which means that the next line is dead code.

@ghost
Copy link
Author

ghost commented May 26, 2013

Really: 'maid' instead of 'made'? I must get some sleep.

@cvrebert
Copy link
Collaborator

cvrebert commented Jul 8, 2013

After rediscovering this PR, I'm now curious how it compares with #8398.

@ghost
Copy link
Author

ghost commented Jul 9, 2013

I think my javascript is a bit 'smarter' as it checks all positions till it finds enough room.
But it is heavier/more overhead.

@fat
Copy link
Member

fat commented Jul 24, 2013

not crzy about your solution - does a lot of unnecessary work calculating all the possible results

@fat fat closed this Jul 24, 2013
@ghost
Copy link
Author

ghost commented Jul 24, 2013

If you have a solid solution that actually works, I would love to see it get into BootStrap :)

@cvrebert
Copy link
Collaborator

@nonumber He just added one: 217eb98

@ghost
Copy link
Author

ghost commented Jul 24, 2013

The issue with that is that it only places the tooltip in one possible other position if there is no room in the target position, and it doesn't check if there is room there.

So if you have your position set to right and there is just not enough room, it places it on the left. Even if there is even less room there.

Also that code is doing calculations that are not used.
So when position is 'right' it still fills vars actualHeight and parentHeight.

@procrastination
Copy link

I update tooltip v2.3.2 with @nonumber 's code. All browsers except IE (currently I am using IE9) work well.
I set default position to be "top".
In IE, when there is no enough room at top, the tooltip won't show. While other browsers will automatically change position to be "bottom" or else.
I tried IE10, tooltips work perfectly. Still cannot figure out what caused the problem on IE9.
Even I updated my bootstrap to be v3.0.0, the tooltip still cannot show when I scrolled down the page and there were no enough room on top for tooltip on IE9 (not compatibility mode).

Sorry, the problem was because I got a stupid typo-> filter: alpha(opacity=100);// before it was filter: alpha(opacity=1) :(

Now the tooltips' auto positioning works great for IE 8 and IE9 !

Thank you for your fantastic job @nonumber @fat !

@ghost
Copy link
Author

ghost commented Aug 8, 2013

Not sure if I should look into the feedback @procrastination gave, as this PR has been closed by @fat.

@fat: What's the status on this?

Sorry to bring up the M-word, but even the old version of Mootools has a great positioning thing going on in their tooltips: http://demos111.mootools.net/Tips
See how it changes position live as you move the mouse towards the edge of the screen (as their tooltips follows the mouse, instead of fixed position on the element).

@niftylettuce
Copy link

@nonumber did you ever get a solid solution for this or some plug in created?

@ghost
Copy link
Author

ghost commented Apr 24, 2015

I have written and used this extra script for some time now:
https://gist.github.com/nonumber/54d9edff3730ed7d25c1

@niftylettuce
Copy link

Thanks @nonumber !

@niftylettuce
Copy link

@nonumber I ran that file through JSHint and got the following errors, can you correct it?

/Users/nexus/test.js: line 0, col 0, ES5 option is now set per default
/Users/nexus/test.js: line 17, col 53, Expected '===' and instead saw '=='.
/Users/nexus/test.js: line 23, col 100, Expected an assignment or function call and instead saw an expression.
/Users/nexus/test.js: line 1, col 1, '$' is not defined.
/Users/nexus/test.js: line 2, col 11, '$' is not defined.
/Users/nexus/test.js: line 37, col 27, '$' is not defined.
/Users/nexus/test.js: line 38, col 45, '$' is not defined.
/Users/nexus/test.js: line 38, col 69, '$' is not defined.
/Users/nexus/test.js: line 39, col 28, '$' is not defined.
/Users/nexus/test.js: line 40, col 45, '$' is not defined.
/Users/nexus/test.js: line 40, col 70, '$' is not defined.
/Users/nexus/test.js: line 45, col 76, 'tooltips_fallback_position' is not defined.
/Users/nexus/test.js: line 50, col 73, 'tooltips_fallback_position' is not defined.
/Users/nexus/test.js: line 55, col 75, 'tooltips_fallback_position' is not defined.
/Users/nexus/test.js: line 60, col 74, 'tooltips_fallback_position' is not defined.

15 errors

@ghost
Copy link
Author

ghost commented Apr 24, 2015

That's because it uses jQuery.
Try now: https://gist.github.com/nonumber/54d9edff3730ed7d25c1

@joncjordan
Copy link

For each case, you may want to add $tip.addClass('top'); or $tip.addClass('left'); so that we can get the arrows displaying properly.

@ghost
Copy link
Author

ghost commented May 23, 2015

No need. That is already being done inside the applyPlacement.

@akrohn
Copy link

akrohn commented Nov 30, 2015

Doesn't work for bootstrap 3.3.6? Error shown on console for bootstrap.js.

Is there a solution that calculates the viewport of a canvas, when the popover is inside this canvas?

@cvrebert
Copy link
Collaborator

cvrebert commented Dec 1, 2015

This feature has been addressed in Bootstrap v4.

@twbs twbs locked and limited conversation to collaborators Dec 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants