Enable decreasing height above viewport #7649
Conversation
examples/everything.amp.html
Outdated
| @@ -174,6 +174,16 @@ <h1 id="top">AMP #0</h1> | |||
| </amp-anim> | |||
| </amp-carousel> | |||
| </p> | |||
| <h2 id="fakead3p_0">fake3pAd send no-content</h2> | |||
There was a problem hiding this comment.
added for testing. will remove
| @@ -1608,7 +1609,64 @@ describe('Resources changeSize', () => { | |||
| expect(resources.relayoutTop_).to.equal(resource1.layoutBox_.top); | |||
| }); | |||
|
|
|||
| it('should NOT adjust scrolling if size did not increase', () => { | |||
| it('should NOT resize when above vp but cannot adjust scrolling', () => { | |||
There was a problem hiding this comment.
Can you explain to me why this one can't compensate?
There was a problem hiding this comment.
To compensate scrolling brought by height decrease, the best we can do is setting scrollTop to 0. Can't compensate here means scrolling jump can only be compensated by setting scrollTop to a negative value.
In real use cases. Say a document with height 1000, the very top element has height 110, and viewport scrollTop is set to 100. According to the rule The top element is treated to be above viewport. However to avoid scrolling jump, we will need to set scrollTop to -10 in order to collapse this element. Which is not possible so resize should not happen.
In this specific test. viewport scrollTop is mocked to be 2, the resize element top position is -1200, and height is 100. In order to change its height to 0, we will need to set viewport scrollTop to -98 to compensate.
* heroku-test * best-effort * update test * cleanup
We don't adjust scroll if element resize to a smaller height above viewport. This is a feature that we should support.
Unlike increasing, it's possible that I cannot compensate height change through adjusting scroll, because there may not be enough height above. In this PR I only try to resize by best effort. Trying to see if I can reduce size based on already handled request in one mutate.
The new feature will also fix #7481