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

Popover falls outside viewport on narrow/mobile screens #7399

Closed
ghost opened this issue Mar 27, 2013 · 21 comments
Closed

Popover falls outside viewport on narrow/mobile screens #7399

ghost opened this issue Mar 27, 2013 · 21 comments
Labels

Comments

@ghost
Copy link

ghost commented Mar 27, 2013

The popover doesn't resize/move when part of it falls outside the viewport.
I think this should happen.
popover-1

This especially is the case when there is a lot of text in the popover...

Can this be fixed?

@cvrebert
Copy link
Collaborator

Are you using v2.3.1? (#6713 improved the handling of such cases.)

@ghost
Copy link
Author

ghost commented Mar 27, 2013

#6713 is the tooltips.
This issue concerns the Popovers. The screenshot was from the bootstrap site.

@cvrebert
Copy link
Collaborator

Popovers are built on top of tooltips. That's why using them requires also including the tooltip JS.

@ghost
Copy link
Author

ghost commented Mar 27, 2013

This is what happens when clicking on a popover link that shows on top (the 'Top' link'):
popover-2
This does show on the screen, but the arrow is now above the 'Bottom' link.

This is what happens on the 'Left' link:
popover-3

@ghost
Copy link
Author

ghost commented Mar 27, 2013

Just an idea:
wouldn't it be a good idea to make position change based on the available room in the viewport?
So if a right positioned tooltip/popover has no room on the right, it shows on the left. If it also doesn't have room there, it shows on the bottom...
Especially on mobile views this would be good, as you don't have room for left/right tooltips/popovers in most cases.

@mdo
Copy link
Member

mdo commented Mar 27, 2013

Of course it would, we just haven't had the time to build it ;).

@ghost
Copy link
Author

ghost commented Mar 27, 2013

Maybe something like this (replacement of lines 1194 ~ 1208
https://gist.github.com/nonumber/5257443

Must be a shorter way to code that, but can't think of it (apart from using separate methods for every if check).

This will try to place the tooltip somewhere else if there is no room, and will default to the 'right' if there is no room on all sides (as the right position seems to have the best text wrapping).
The order of looking for free space is different for every position:
right: left, top, bottom
left: right, top, bottom
top: bottom, right, left
bottom: top, right, left

@dperolio
Copy link

dperolio commented May 6, 2013

Hey, I'm trying to find a solution to this problem as well. Your solution works nonumber? Where exactly do I put it? You said lines 1194 ~ 1208, but I'm not sure where that is. It looks to be around right after this,

var old = $.fn.popover

$.fn.popover = function (option) {
return this.each(function () {

in popover.js; is that correct? Thanks.

@ghost
Copy link
Author

ghost commented May 6, 2013

It's a replacement of lines 145 - 163 of https://github.com/twitter/bootstrap/blob/master/js/bootstrap-tooltip.js:

        actualWidth = $tip[0].offsetWidth
        actualHeight = $tip[0].offsetHeight

        switch (placement) {
          case 'bottom':
            tp = {top: pos.top + pos.height, left: pos.left + pos.width / 2 - actualWidth / 2}
            break
          case 'top':
            tp = {top: pos.top - actualHeight, left: pos.left + pos.width / 2 - actualWidth / 2}
            break
          case 'left':
            tp = {top: pos.top + pos.height / 2 - actualHeight / 2, left: pos.left - actualWidth}
            break
          case 'right':
            tp = {top: pos.top + pos.height / 2 - actualHeight / 2, left: pos.left + pos.width}
            break
        }

        this.applyPlacement(tp, placement)

@dperolio
Copy link

dperolio commented May 6, 2013

@nonumber Thanks for the quick response. I can't seem to get it to work though. It just breaks it for me. :(

http://jsfiddle.net/MV35f/

@ghost
Copy link
Author

ghost commented May 6, 2013

Should be working if you add var to those tpx lines. So:

        var tpb = {top: pos.top + pos.height, left: pos.left + pos.width / 2 - actualWidth / 2}
        var tpt = {top: pos.top - actualHeight, left: pos.left + pos.width / 2 - actualWidth / 2}
        var tpl = {top: pos.top + pos.height / 2 - actualHeight / 2, left: pos.left - actualWidth}
        var tpr = {top: pos.top + pos.height / 2 - actualHeight / 2, left: pos.left + pos.width}

Don't know why it was working just fine on my setup, but not on JSFiddle.

@dperolio
Copy link

dperolio commented May 6, 2013

Very nice, it works. :) Now all we need is full collision detection and correction (top and bottom). :'D

http://jsfiddle.net/MV35f/1/

EDIT: Oh I'm silly. This is a different problem really. Disregard me.

@fat
Copy link
Member

fat commented May 25, 2013

i've written code for this a few times in the issues… i might be open to a pull request for this, but you have to realize its pretty expensive/slow perf wise.

closing this issue for now tho

@fat fat closed this as completed May 25, 2013
@ghost ghost mentioned this issue May 25, 2013
@ghost
Copy link
Author

ghost commented May 25, 2013

PR: #7988

@ghost ghost mentioned this issue May 26, 2013
@mchambaud
Copy link

When placement is set to bottom and there is no room to the right, it will still break.

screen shot 2013-06-04 at 10 34 18 am

@jfall
Copy link

jfall commented Oct 10, 2013

Why is this still broken in Bootstrap 3? It is even broken on the site for mobile: http://getbootstrap.com/javascript/#popovers

@cvrebert
Copy link
Collaborator

@jfall We have the auto placement option now, which somewhat helps with this. Our JS guy had performance and complexity concerns about making the screen-edge-avoidance super-fancy.

@ghost
Copy link
Author

ghost commented Oct 10, 2013

I had a working fix for this: #7996
But for some reason @fat doesn't like it and closed it.

A shame, really.

@jfall
Copy link

jfall commented Oct 11, 2013

@cvrebert thanks for telling me about the "auto" command. What would also be great would be some sort of control for the auto direction as it anchors to the top for all situations. Would be nice to be able to have it for col-sm or col-md have it appear on the right and in col-xs appear on top or bottom.

ma-si pushed a commit to ma-si/x-editable that referenced this issue Feb 19, 2014
…dow edge'

Fix 'bootstrap popover falls off page if editable is too close to window edge'.


Reference:
    twbs/bootstrap#6713
    twbs/bootstrap#7399
    https://gist.github.com/nonumber/5257443
@publicocean0
Copy link

comparing the behaviour of auto replacement of datatime picker with popover there are many emprovements to do: when i resize page for example popover lose the right position;
In addition, auto setting for my opition has a insufficient information because it starts to place where it want not where developer wants. Usually i set a normal position where i want place popover, if it is not possible so then try to re place automatically in different side. A better setting for replacement could be auto:start_side:strategy , where auto_start is the normal side where place it, strategy is moviment sense in the automatic replacement , in the sense of clock or opposite.
example : auto:right:round_to_left replace in the right side if possible else place it in top if possible else place in left side if possible ... (opposite of clock orientation )

@hnrch02
Copy link
Collaborator

hnrch02 commented Aug 19, 2014

@publicocean0 Please open a new issue.

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

No branches or pull requests

8 participants