-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(Popup): use original position if none fit in viewport #1483
Conversation
I have no idea why the |
src/modules/Popup/Popup.js
Outdated
@@ -228,7 +228,7 @@ export default class Popup extends Component { | |||
|
|||
// Lets detect if the popup is out of the viewport and adjust | |||
// the position accordingly | |||
const positions = _.without(POSITIONS, position) | |||
const positions = _.without(POSITIONS, position).push(position) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line doesn't make sense, you remove position
from array and then push it again 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add it to the end of the array, so that if no suitable position can be found, the selected (last) one is used. Otherwise it would just use the last one in POSITIONS, which is bottom center
.
@MindFreeze any interest in pushing this one over the finish line? I'd be OK merging it if tests were fixed and a new test added that asserts the new behavior. Namely, we'd need a test that says if no other position fits in the viewport then the original position is used. Since this is a very rare edge case, it probably will not get attention from the core team. |
I'll give it a go |
Codecov Report
@@ Coverage Diff @@
## master #1483 +/- ##
=======================================
Coverage 99.74% 99.74%
=======================================
Files 141 141
Lines 2371 2371
=======================================
Hits 2365 2365
Misses 6 6
Continue to review full report at Codecov.
|
@levithomason All done. |
You're awesome, thanks much! |
Released in |
Should fix #1482