Popper-core: Popper does not update its position to fit entirely in viewport/document if it is overflowing

[atomiks]

CodePen demo

https://codepen.io/FezVrasta/pen/QvQQgv

Steps to reproduce the problem

1. Simply resize the browser window and you'll notice part of the Popper begins to overflow the document edge at the boundaries.

What is the expected behavior?

That it is fully visible in the viewport with no overflow on a single update (i.e., it fits within the content of the document excluding itself).

What went wrong?

It overflows.

[FezVrasta]

Have you tried with sheduleUpdate?

[atomiks]

Well if you change the codepen to use scheduleUpdate, the same problem occurs

[FezVrasta]

Why are you disabling the event listeners?

[atomiks]

Event listeners are disabled when a popper is hidden (in Tippy)

[FezVrasta]

Ok, but what I don't get is why you need to update the position of a disabled tooltip?

[atomiks]

Well it needs to recalculate the position of the tooltip when it is shown again, if the browser window was resized (or the element was moved somewhere else in general) it will be at the wrong position..

[FezVrasta]

I see

[atomiks]

Also I should note that it has no trouble positining it correctly when the instance is first created. If you keep the browser window resized and refresh the page, the initial position is correct

[atomiks]

I don't think the title change is right -- what I said initially is what I meant.

I think the debounce function prevents it from properly being moved into the viewport?

Even with events enabled, it doesn't shift enough:
https://i.gyazo.com/551a5a563769d233b8fc45d1d4336c7d.mp4

[FezVrasta]

Ok that's why I say that the codepens should provide the minimum amount of code needed to reproduce the bug.

If the disbleEventListeners isn't needed, why it was there? It generates confusion.

The bug is probably in preventOverflow

[atomiks]

Sorry for the confusion! I didn't mean for it to be that way... hope you can sort the problem out

[atomiks]

The changes you made works, will this be released as 1.9.4?

[FezVrasta]

I have to fix the tests on IE10 before I can merge the fix

[FezVrasta]

1.9.4

[FezVrasta]

The problem with this bug is that we can't know the document size, excluded the space it's taking the popper before it gets re-positioned...

Any attempt to fix it is going to obviously break other parts of the code, because we are using the wrong approach.

Any idea to get the document size minus the space took by the popper (in case it's stretching the document after a resize)?

Right now this issue is fixed but I'm probably going to reintroduce it to fix #302 properly...

I really need help with this one.

[atomiks]

Have you gotten any closer to solving this (without breaking 302)?

[FezVrasta]

Nope sorry. It's very tricky

[atomiks]

Which functions are relevant to this problem?

I can try and see if I can work it out.

[FezVrasta]

I think it's just the getWindowSizes. It should be able to detect the document width/height excluding the potential overflow caused by the popper element.

[atomiks]

Hmm... given that the majority of sites don't have a horizontally-scrolling documentElement, why not just use window.innerWidth instead of the document width?

I haven't really encountered/had trouble with the height issue, it's mainly the width one. So maybe you could have an option, "does the page overflow horizontally or not"?

[FezVrasta]

I'd like to make the library work in all the edge cases, without any manual setting.

[atomiks]

Yeah that's the ideal, but if an option fixes the issue for the majority of cases (at least temporarily until you can work out a full solution given that it's tricky), I'd say it's worthwhile to add in.

[FezVrasta]

That would break the API, I would have to release a major version just to get rid of an hack we introduced. Not ideal at all.

[FezVrasta]

Also, I think the problem can be reproduced on both axis, so that would be a fix only for one of them

[atomiks]

How about calculating the document width and height before the popper has the position styles applied?

[FezVrasta]

What if the document width changes for a reason unrelated to popper?

[atomiks]

Then you'd need to listen to height/width changes of the document/body elements, so that whenever it's updated, Popper can know the new sizes.

[FezVrasta]

But if the document width changes because some content is added to the page we don't have any event to know it, and even if we did, we couldn't know what's the size of the document excluded the popper

[atomiks]

Indeed, you'd need to use a very inefficient polling method/interval.... 😢
It can't really be that this has no solution??

[atomiks]

How about very briefly removing the popper from the document, calculating the document width/height, then appending it back?

[FezVrasta]

We should do that for each update loop.. That would be terrible

[atomiks]

It does sound quite terrible on paper, but I wonder how severe it would be in general use. And you could have an option to turn it on only when necessary... (so keep the behavior off by default), or maybe there's a way to "batch"/optimize it somehow, who knows.

[FezVrasta]

Feel free to experiment with it, if you end up with a good solution I'll be happy to merge it. I just don't want any extra option.

[kybishop]

@atomiks here is how I'm getting around this in my own tooltip library: https://github.com/kybishop/ember-attacher/blob/master/addon/components/attach-popover.js#L252

The above line positions the popper in the top left of the screen on creation. We then manually call update() when the tooltip will be shown, which correctly positions the element.

[FezVrasta]

But how do you handle the window resize?

[kybishop]

Assuming the popover is not larger than the window itself, positioning it in the top left of the screen means the window shrinks to fit its content. Since the popper is all the way to the left, the window shouldn't grow any larger than the popper.

EDIT: This doesn't fix issues where the window is resized, just allows for correct initial positioning

[atomiks]

The window resize isn't a big a deal as the initial update positioning, so I'll see if that solution works well. Thanks!

EDIT: It works nicely. I wonder if the same principle could be applied on each update() cycle too without affecting performance too much?

EDIT2: I just manually added

this.popper.style[getSupportedPropertyName('transform')] = null

inside Popper's function update() {} and not seeing any glitchiness in Chrome 62, and it fixes the issue. However, it slightly affects scroll with a transition applied to the popper, but isn't necessary for scrolling, so maybe you could only use it for the window resize listener.

[alexander-morozov]

I also stuck with the same issue and for me removing html.scrollWidth from getSize method fixes it. Probably something like https://github.com/FezVrasta/popper.js/commit/1b3a82d77a11ce84d767da0f1ac4982774a64979 should fix it.

Can we move includeScroll to external options? By default it can be true for backward compatibility.

[victorting]

I also stuck with the same issue too. Hoping ace to fixes it. Thanks a lot!

[FezVrasta]

1.14.2 includes the fix for this bug