fix(overlay): incorrectly calculating centered position on a scrolled page with pushing#13185
Conversation
googlebot
added
the
cla: yes
| const smallestDistanceToViewportEdge = | ||
| Math.min(viewport.bottom - origin.y, origin.y - viewport.left); | ||
| const smallestDistanceToViewportEdge = Math.min( | ||
| viewport.bottom - origin.y + viewport.top, origin.y); |
There was a problem hiding this comment.
Why isn't this Math.min(viewport.bottom - origin.y, origin.y - viewport.top);?
Also, prefer breaking at the higher syntactic level (like it was before)
There was a problem hiding this comment.
Because that ends up including the scroll position again which throws off the position on a scrolled page.
There was a problem hiding this comment.
Could you expand the comment the call that out here?
There was a problem hiding this comment.
Done. Also fixed up the line break.
… page with pushing Fixes a centered flexible overlay with pushing, on a scrolled page, not calculating the position properly. There were a couple of issues: * We were using the `top` viewport offset to calculate along the X axis, as well as `left` to calculate along Y. * We weren't accounting correctly for the scroll position. Fixes angular#11868.
crisbeto
force-pushed
the
11868/flexible-overlay-centered-scrolled
branch
from
6469e37 to
107cab7
Compare
jelbourn
added
pr: lgtm
action: merge
… page with pushing (angular#13185) Fixes a centered flexible overlay with pushing, on a scrolled page, not calculating the position properly. There were a couple of issues: * We were using the `top` viewport offset to calculate along the X axis, as well as `left` to calculate along Y. * We weren't accounting correctly for the scroll position. Fixes angular#11868.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
4 participants
Fixes a centered flexible overlay with pushing, on a scrolled page, not calculating the position properly. There were a couple of issues:
topviewport offset to calculate along the X axis, as well asleftto calculate along Y.Fixes #11868.