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

amp-analytics calculates wrong scroll triggers for embedded shadow documents #18735

Closed
peterjosling opened this issue Oct 15, 2018 · 3 comments · Fixed by #26451 or #27035
Closed

amp-analytics calculates wrong scroll triggers for embedded shadow documents #18735

peterjosling opened this issue Oct 15, 2018 · 3 comments · Fixed by #26451 or #27035

Comments

@peterjosling
Copy link
Contributor

What's the issue?

amp-analytics ScrollManager calculates scroll positions for scroll triggers based on the viewport scrollHeight/scrollTop. For shadow documents (e.g. in amp-next-page) this means it calculates the current scroll percentage from the entire parent document, even if the shadow document is off screen.

Scroll position should be calculated from the shadow document's body height instead, otherwise you will get completely different analytics behaviour when a document is embedded, vs when used standalone.

@aghassemi
Copy link
Contributor

/to @zhouyx

@aghassemi
Copy link
Contributor

Setting as P2 since amp-next-page is a prioritized feature. /cc @jeffjose

@aghassemi aghassemi added this to the Pending Triage milestone Oct 15, 2018
@lannka lannka assigned torch2424 and unassigned zhouyx Oct 22, 2018
@torch2424
Copy link
Contributor

The analytics team, ( @lannka @zhouyx @calebcordry ) and I decided that it would be good to re-think how we are doing scroll events, and decide what would be the expected behavior on this before we start implementing a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment