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

Typeahead availability of placement 'bottom-left' or 'top-left' is dismissed by height, not width #1876

Closed
AmadejBukorovic opened this issue Sep 28, 2017 · 1 comment

Comments

@AmadejBukorovic
Copy link

Bug description:

I believe determining the availability of placement, when using secondary placement of 'left' with the primary placement 'top' or 'bottom', is incorrect. It works as long as the left offset of the host element is smaller than the widow height. Given a rectangular window and a far right position of the host element, the positioning breaks.

I should note that I believe this is a general bug in the Positioning class so I presume it's not limited to the Typeahead component. The method in question is named setSecondaryPlacementForTopBottom:

private setSecondaryPlacementForTopBottom(
      hostElemClientRect: ClientRect, targetElemClientRect: ClientRect, primaryPlacement: string,
      availablePlacementArr: Array<string>) {
    let html = document.documentElement;
    // check for left-bottom
    if ((window.innerHeight || html.clientHeight) - hostElemClientRect.left >= targetElemClientRect.width) {
      availablePlacementArr.splice(availablePlacementArr.length, 1, primaryPlacement + '-left');
    }
    if (targetElemClientRect.width <= hostElemClientRect.right) {
      availablePlacementArr.splice(availablePlacementArr.length, 1, primaryPlacement + '-right');
    }
  }  

From my understanding of the code, the first if sentence should use the window.innerWidth or html.clientWidth (and not height, as it is), to determine if there's enough space horizontally, since at this point, it was already determined by the primary placement that there's enough space up or down?

As such, when you set the placement of a typeahead field to be limited to ['bottom-left', 'top-left'], neither is available and the last one is used ('top-left'), even when there's enough space to position it under the host element.

Link to minimally-working plunker that reproduces the issue:

For the plunker to work, please ensure the preview IFRAME is wider than it is taller, but still has enough room for the typeahead's drop down menu to be positioned underneath the input. Ensuring the preview IFRAME is around 1100px wide and 730px tall should exhibit the described behaviour. Type 'al' into the input to get the drop down menu to open with 4 results.

http://plnkr.co/edit/gkh3qxwE1pPMuVMZrRCp?p=preview

Version of Angular, ng-bootstrap, and Bootstrap:

Angular: 4.0.3
ng-bootstrap: 1.0.0-beta.5
Bootstrap: 4.0.0-beta i presume?

@mschoudry
Copy link
Contributor

There are two issues:

  1. Not calculating properly bottom-left position.
    clearly due to the bug of considering height rather than width, as explained in the description above.

  2. Not calculating properly left position.
    default position of created popup window by popup service is causing the scrollbar to appear and disappears as soon as position service sets the top position. This scrollbar causes the wrong calculations.

I am opening a PR with the fixes.

@maxokorokov maxokorokov added this to the 1.0.0-beta.7 milestone Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants