fix(dropdown): align position with or without vertical scrollbar #6008
Conversation
| @@ -195,7 +195,8 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) | |||
| var pos = $position.positionElements($element, self.dropdownMenu, 'bottom-left', true), | |||
| css, | |||
| rightalign, | |||
| scrollbarWidth; | |||
| scrollbarPadding, | |||
| scrollbarWidth = 0; | |||
There was a problem hiding this comment.
Actually, the eslint issue had to do with using tabs instead of spaces (I used Notepad++ which isn't the best tool for the job).
Eslint passes now, but I can change this if you wish.
|
On the contrary, having a test is only a good thing! This PR looks good to me, but can you explain what am I supposed to see in the Plunker from #5942 and this one? |
|
In the Plunker from this PR you should hopefully see nothing out of the ordinary :). The Plunker from #5942 shows that the dropdown with append-to-body and dropdown-menu-right is offset to the right by the width of the scrollbar when the scrollbar isn't visible. This happened because my original fix (that landed in 1.3.3) was very naive and simply assumed that the scrollbar is always present which obviously isn't the case. |
|
I actually am not able to reproduce the issue in Chrome (OS X) - what is your OS that you are seeing the issue on, Windows? |
|
Yes, windows 10. |
|
I think OSX has scrollbars that are overlaid on top, so they probably don't change the window size and affect the right alignment. |
|
Alright, I'll verify when I get home a little later and likely merge then after verification |
| @@ -702,4 +702,33 @@ describe('uib-dropdown', function() { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('using dropdown-append-to-body with dropdown-menu-right class', function() { | |||
There was a problem hiding this comment.
In a comment before this issue, can you mention // issue #5942?
|
Sorry for taking so long - been traveling the past two weekends. This LGTM. |
Fixes #5942.
Plunkr showing the fix:
http://plnkr.co/edit/jlBJjMgpDnR133ByUATF?p=preview
Resubmitted after fixing eslint issues.
Some notes regarding the added test: