tribute icon indicating copy to clipboard operation
tribute copied to clipboard

Menu re-position on top viewport on edge

Open stromblack opened this issue 5 years ago • 11 comments

How can we reproduce this bug?

  1. set div.mention on bottom page
  2. slide up scroll bar just a little bit and small space less min-height from class tribute-container
  3. and trigger mention

What did you expect to happen? menu re-position on top

Screenshot: screenshot 2019-01-15 17 47 33

stromblack avatar Jan 16 '19 07:01 stromblack

Have the same issue

If there is not enough room left in the viewport to display the list of mentions, then it "jumps" upwards

Test case from https://zurb.github.io/tribute/example/ :

When viewing in full page: image

#tribute-container style:

left: 801px;
position: absolute;
z-index: 10000;
display: block;

When artificially reducing viewport's height by resizing window: image

#tribute-container style:

left: 789.667px;
bottom: 273px;
position: absolute;
z-index: 10000;
display: block;
top: auto;

Would be amazing if someone can have a look at this - unfortunately my JS skills are not sufficient for that..

Thanks

Bunobuno avatar May 14 '19 12:05 Bunobuno

After looking at the code, I don't really understand the logic of handling bottom overflow (ie when the list pops up and goes beyond the bottom edge of the viewport).

if (menuIsOffScreen.bottom) {
   var parentRect = this.tribute.menuContainer ? this.tribute.menuContainer.getBoundingClientRect() : this.getDocument().body.getBoundingClientRect();
   var scrollStillAvailable = parentHeight - (windowHeight - parentRect.top);
   coordinates.top = 'auto';
   coordinates.bottom = scrollStillAvailable + (windowHeight - rect.top);
}

If menus if off-screen through the bottom edge, why not simply display it right at the edge of the viewport?

if (menuIsOffScreen.bottom) {
   coordinates.top = 'auto';
   coordinates.bottom = 0 - window.scrollY;
}

Bunobuno avatar May 29 '19 10:05 Bunobuno

Facing same issue, anyone found a workaround?

chanlito avatar Jun 19 '19 13:06 chanlito

I have found a workaround for #298. Not fully tested yet but I have modified the following in the trubute.js file

tributeChange

It seems to work if you are using a contenteditable tag. You may need to modify code elsewhere too, for example if you are using an input field as the target.

stepwright avatar Sep 02 '19 09:09 stepwright

@stepwright Can you try doing a PR for this?

mrsweaters avatar Sep 06 '19 20:09 mrsweaters

Having the same issue, and it's blocking our go live. Any temporary fix?

sanjogs avatar Sep 10 '19 19:09 sanjogs

Did a PR get submitted for this issue?

rksobchak avatar May 24 '20 15:05 rksobchak

... If menus if off-screen through the bottom edge, why not simply display it right at the edge of the viewport?

if (menuIsOffScreen.bottom) {
   coordinates.top = 'auto';
   coordinates.bottom = 0 - window.scrollY;
}

Disadvantage would be that the menu would be shown on top of the text being entered. Instead, I've so far come up with:

if (menuIsOffScreen.bottom) {
    let yOffset = this.tribute.menuContainer
      ? this.tribute.menuContainer.getBoundingClientRect().top
      : window.pageYOffset
    coordinates.top = "auto"
    coordinates.bottom = (windowHeight - rect.top) - yOffset
}

This seems to work well. See this comment in related, but slightly different(?), issue. There is Codepen + link to the fix. So far it seems to work better in all cases. But there is lot of corner cases, so I might make some of them worse (hopefully not :) ).

mkrauskopf avatar May 26 '20 15:05 mkrauskopf

TLDR: Set position:relative on body.

I believe my code #491 should make this better but there's a gotcha when using absolutely positioned elements on body that's the root cause. When absolutely positioning an element in body using a bottom value, the element is attached to the viewport rather than the body. Or more precisely said, when positioning any element absolutely, it will be positioned relative to the closest parent that is positioned absolute or relative. Intuitively we believe that body qualifies but it does not. Thus absolutely positioning an element in body will actually be relative to the viewport unless body is absolute or relative. This normally doesn't trip you up because the viewport and body both share top/left values, but when positioning something using bottom or right it'll get you. The logic that positions the menu doesn't appreciate this. In fact, there's no easy way to absolutely position something relative to body without requiring users putting position:relative on it. Issues with determining the containing block (i.e. the element that the popup will be positioned relative to) is one of the reasons why some libraries always use a position:fixed on popups.

So in our case, the tribute code says "the input is 400px from the bottom of the body", "lets position:absolute; bottom:400px". But the element is being positioned relative to the viewport so its being shown much higher - its being shown 400px from the viewport bottom.

@mkrauskopf Your code would fix it in this case (when body is the container) but I believe it would cause issues when using a different menuContainer.

IMO the most straightforward way to code popups and deal with positioning is always to put them on body, position them using fixed positioning, and close them if the user scrolls. Users don't really have expectations that dropdowns/popups stay visible after scrolling anyway. Using fixed positioning would simplify that logic. In lieu of doing that, the docs could tell users to put position:relative on body.

cgross avatar Jun 09 '20 18:06 cgross

@cgross I've changed the fix only for the case when the container is not defined. See #477 for details. That seems to work (we are using it in production). But maybe your changes cover also the case mentioned in PR #477. I'll check that later. Thanks.

mkrauskopf avatar Jun 10 '20 06:06 mkrauskopf

Since no patch has been merged, 👍 for the workaround setting up the body's position as relative, but I'm not sure if permanently setting up the body as relative could lead to other weird consequences, so in the end, I add a workaround to the workaround by listening to tribute-active-true event and setting up body's position whenever necessary

const defaultBodyPosition = document.body.style.position

document
  .getElementById("myElement")
  .addEventListener("tribute-active-true", function(e) {
    document.body.style.position = "relative"
  });
  
document
  .getElementById("myElement")
  .addEventListener("tribute-active-false", function(e) {
    document.body.style.position = defaultBodyPosition
  });

haven't dug deep into the code, but I wonder using similar approach we could "recalculate" the correct position?

tejanium avatar May 05 '21 14:05 tejanium