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

fix: added missing CSS unit when positioning the window offset to top #253

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

emeraldsanto
Copy link

Description

This PR simply adds the missing px unit to the calculation of how much to offset from top. Without this, certain browsers will simply interpret the value as 0 which makes the page scroll to the top.

@topolanekmartin
Copy link

On mobile iOS it does not attach top property on body element. This causes the scroll is not returned back to its previous position after scroll is enabled.

…ort changes (#2)

1. Change the target to document.body ➡️ window.top.document.body. (In non-iframe situations window.top resolves to window.) This is because in certain situations we might want to invoke the body scroll lock while inside of an iframe and want to make sure locking extends beyond the iframe.

2. Remove the code that adjusts for viewport changes. While debugging this library I found that this passage of code that attempts to adjust for viewport changes did not work (it was causing the top value to be unset, meaning scroll position was not preserved). I don't think that it ought to be the responsibility of this library to adjust for viewport changes; it should just be to apply a body scroll lock. If they need to, consumers can adjust by listening to changes in visualViewport or using the new CSS display viewport dimensions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants