Skip to content

Commit

Permalink
🐛Force scrollTop to be remeasured on shadow doc visibility change (#2…
Browse files Browse the repository at this point in the history
…6588)

* Force scrolltop to be remeasured on shadow doc visibility change

* Add comment to fix
  • Loading branch information
kevinkimball committed Feb 3, 2020
1 parent a110d9a commit 653dda3
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/service/viewport/viewport-impl.js
Expand Up @@ -228,6 +228,11 @@ export class ViewportImpl {
// the size has changed between `disconnect` and `connect`.
this.resize_();
}
if (this.scrollTop_) {
// Remeasure scrollTop when resource becomes visible to fix #11983
this./*OK*/ scrollTop_ = null;
this.getScrollTop();
}
} else {
this.binding_.disconnect();
}
Expand Down
21 changes: 21 additions & 0 deletions test/unit/test-viewport.js
Expand Up @@ -395,6 +395,27 @@ describes.fakeWin('Viewport', {}, env => {
expect(binding.disconnect).to.be.calledOnce;
});

it('should update scroll position when visibility changes', () => {
binding = new ViewportBindingDef();
binding.getScrollTop = (() => {
const generator = (function*() {
yield 25;
return 100;
})();
return () => generator.next().value;
})();
viewport = new ViewportImpl(ampdoc, binding, viewer);

// Force scrollTop to be measured
viewport.getScrollTop();
expect(viewport./*OK*/ scrollTop_).to.equal(25);
// Toggle visibility state
changeVisibilityState('prerender');
changeVisibilityState('visible');
// Expect scrollTop to be remeasured
expect(viewport./*OK*/ scrollTop_).to.equal(100);
});

it('should resize only after size has been initialed', () => {
onVisibilityHandlers.length = 0;
changeVisibilityState('visible');
Expand Down

0 comments on commit 653dda3

Please sign in to comment.