Include child margin-bottom in content height #22718
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dreamofabear
force-pushed
the
bottom-margin-in-content-height
branch
from
d1675aa to
3e3f073
Compare
dreamofabear
changed the title
dreamofabear
force-pushed
the
bottom-margin-in-content-height
branch
from
3e3f073 to
fb0dbe4
Compare
|
/to @jridgewell |
jridgewell
reviewed
Jun 10, 2019
| * @param {!Element} element | ||
| * @return {number} | ||
| */ | ||
| export function marginBottomOfLastChild(win, element) { |
There was a problem hiding this comment.
Where does this algorithm come from?
There was a problem hiding this comment.
Is this how it actually behaves?
There was a problem hiding this comment.
Yup, try it out. Note that it only repros on Safari, not Chrome with emulation.
<!doctype html>
<html ⚡4email>
<head>
<meta charset="utf-8">
<script async src="https://cdn.ampproject.org/v0.js"></script>
<style amp4email-boilerplate>body{visibility:hidden}</style>
<meta name='viewport' content='initial-scale=1'>
<style amp-custom>
.red { background-color: red; }
.blue { background-color: blue; }
.yellow { background-color: yellow; }
html { padding-top:50px; }
#first { margin-top: 100px; }
#last { margin-bottom: 100px; }
</style>
</head>
<body>
<p id="first" >
First element.
</p>
<div id="lipsum">
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec condimentum feugiat nisi, et bibendum est interdum ac. Sed eu pretium lacus. Cras eu laoreet lacus. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Mauris non turpis ipsum. Phasellus sodales dui quam, nec luctus mi euismod nec. Integer vitae nibh leo. Donec leo elit, varius a sapien non, iaculis eleifend odio. In pharetra ultrices magna, at dignissim nisi. Nunc viverra vel nibh ac dictum. Ut iaculis sapien id justo sollicitudin imperdiet. Etiam hendrerit posuere nisi.</p>
<p>Maecenas pretium aliquam neque, eu convallis ipsum lobortis quis. Nunc fermentum vestibulum vulputate. Sed blandit faucibus tellus nec porttitor. Suspendisse faucibus justo sit amet erat convallis pulvinar. Sed mollis, turpis vel faucibus auctor, turpis ipsum efficitur erat, vel tincidunt dolor quam non tortor. In porta porttitor quam ut efficitur. Sed hendrerit condimentum dui in consectetur. Integer a sem egestas, lobortis massa in, consectetur risus. Morbi ut ullamcorper lacus, nec consequat erat. Aenean euismod tristique lorem ac tempor. Curabitur non semper ipsum. Nulla non ante pharetra, viverra urna et, consectetur arcu. Phasellus dapibus consectetur convallis.</p>
<p>Praesent mollis, quam non finibus semper, diam augue elementum lectus, at lobortis justo dolor vel est. Praesent sit amet eleifend lorem. Donec aliquet, dui sit amet rhoncus tempus, dolor ex dignissim ligula, et dictum diam eros nec ex. In ac libero tristique, accumsan mauris et, posuere eros. Cras viverra eros nunc, et egestas odio semper eu. Proin ac vehicula massa. Duis eget ante maximus, condimentum diam in, pretium odio. Proin nec leo ac lorem suscipit faucibus. Nam in est sed magna suscipit congue a id tellus. Fusce ultrices lorem et ullamcorper volutpat. Praesent eu feugiat nunc, convallis finibus ligula. Integer gravida nulla eu viverra ullamcorper.</p>
<p>Mauris condimentum est vitae magna maximus, eu placerat nisi eleifend. Maecenas lobortis, tortor et maximus pellentesque, erat ante pharetra orci, vel efficitur metus elit in urna. Cras et suscipit orci. Praesent sollicitudin vestibulum scelerisque. Nulla justo enim, hendrerit in suscipit sit amet, scelerisque at neque. Nam volutpat ipsum non metus interdum efficitur. Aenean non mauris eget tellus ullamcorper hendrerit et nec nisi. Vivamus ultricies ut purus non ullamcorper. Curabitur eget ornare eros. Nunc fermentum congue venenatis. Nulla id egestas nunc, vel auctor dui. Sed congue nunc vel orci accumsan vehicula. Curabitur ut suscipit neque, vitae gravida diam. Donec congue volutpat massa et sagittis. Duis quis quam gravida, posuere sapien et, rutrum ipsum. Nullam non pretium nisi.</p>
<p>Pellentesque sed auctor massa. Mauris at nisl metus. Duis nec dolor commodo, vestibulum risus a, mollis magna. Fusce sit amet velit et justo imperdiet sollicitudin. Quisque eu facilisis lorem. In massa urna, dapibus id sollicitudin quis, egestas eget tellus. Aliquam blandit ipsum vel maximus vulputate. Sed at accumsan turpis. Sed vel velit odio. Morbi scelerisque mauris quis neque condimentum, non ultrices dui finibus.</p>
<p>Vivamus ultrices urna quis diam vulputate, sit amet condimentum nulla accumsan. Curabitur libero elit, finibus eget neque a, volutpat vestibulum felis. Vestibulum faucibus orci ac turpis sodales venenatis. Nunc erat metus, finibus nec dapibus nec, eleifend eu purus. Nunc euismod sodales sem, non hendrerit dui ullamcorper non. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Morbi id metus velit. In nec nisl ante.</p>
<p>Integer et quam eu magna vestibulum semper sit amet rutrum augue. Vestibulum eget congue quam. Integer placerat congue diam. Etiam rhoncus lorem id ex fringilla, vitae euismod quam posuere. Ut urna turpis, cursus ac dui in, lobortis tristique risus. Cras lacinia ipsum ut erat tincidunt, vel imperdiet ante ornare. Etiam rutrum vitae quam ut vulputate.</p>
<p>Praesent malesuada quis dui sed consequat. Sed suscipit malesuada sem, ac hendrerit neque bibendum a. Curabitur id dapibus justo, eu dapibus ligula. Ut nec bibendum nisl. Donec placerat lorem in tempus sagittis. Quisque dui urna, rutrum vitae dignissim nec, consequat sed ligula. Nam quam mi, condimentum ac rutrum sit amet, posuere quis tellus. Morbi venenatis dictum nibh, at sollicitudin elit finibus vel. Vestibulum a ex malesuada, auctor libero eu, finibus quam. Curabitur maximus nec lacus nec egestas. Interdum et malesuada fames ac ante ipsum primis in faucibus. Vivamus rhoncus magna erat, nec consectetur eros vulputate vel.</p>
<p>Nulla facilisi. Pellentesque ac imperdiet nibh, eu vestibulum neque. Curabitur aliquet dolor ut placerat tristique. Morbi vel pulvinar tortor, pharetra commodo mi. Ut facilisis diam eros, nec suscipit ipsum ultrices ut. Sed sollicitudin fermentum nunc, non varius risus efficitur elementum. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam auctor feugiat malesuada. Pellentesque malesuada diam eros, at tempor nisi feugiat ac. Nulla quis justo eu velit gravida maximus commodo a massa. Maecenas ullamcorper enim ac tempus ultricies. Ut venenatis maximus fermentum.</p>
<p>Nam sed fermentum tortor. Donec felis turpis, auctor a ipsum sed, lacinia varius dolor. Mauris ut ornare sem. Vestibulum iaculis justo ac eros faucibus, in dictum lectus porttitor. Praesent semper, felis ac pulvinar posuere, est turpis cursus arcu, non interdum libero purus sed augue. Nam convallis varius nunc a porta. Etiam arcu dolor, interdum in tellus ut, viverra porta quam. Nulla id nisl nec tortor ornare molestie.</p>
</div>
<p id="last">Last element.</p>
<p></p>
</body>
</html>
jridgewell
approved these changes
Jun 10, 2019
thekorn
pushed a commit
to edelight/amphtml
that referenced
this pull request
Sep 11, 2019
* Include bottom margin in getContentHeight. * Support other viewport bindings. * Dedupe code with helper function. * Restrict to Safari. * Cleaner code. * Add experiment description.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
getContentHeight()is incorrect for some pages on iOS since the margin-bottom of the last child is not included ingetBoundingClientRect().height. To fix this, calculate and add it manually.Gated under
bottom-margin-in-content-heightexperiment. Will enable it in a separate PR.b/134179389