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

Account for child margin-top in getContentHeight() #21097

Merged
merged 3 commits into from Feb 27, 2019

Conversation

dreamofabear
Copy link

@dreamofabear dreamofabear commented Feb 26, 2019

Follow-up to #20035. Adjusts content height calculation to include the margin-top of children of the scrollingElement.

Tested this on natural, ios-embedwrapper and ios-embed-sd on Chrome and Safari.

Context: b/124618643

/to @jridgewell

@dreamofabear dreamofabear added the Externally Tracked Tracked by Google or other parties label Feb 26, 2019
@dreamofabear dreamofabear merged commit 64b9bbc into ampproject:master Feb 27, 2019
@dreamofabear dreamofabear deleted the content-height-top-gap branch February 27, 2019 16:23
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
* Account for child margin-top in contentHeight calc.

* Fix tests.

* Add unit tests.
bramanudom pushed a commit to bramanudom/amphtml that referenced this pull request Mar 22, 2019
* Account for child margin-top in contentHeight calc.

* Fix tests.

* Add unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Externally Tracked Tracked by Google or other parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants