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] visiblePercentageMin in visibility spec does not default to 0 #6339

Closed
lannka opened this issue Nov 24, 2016 · 2 comments
Closed

Comments

@lannka
Copy link
Contributor

lannka commented Nov 24, 2016

In the VisibilitySpec, if visiblePercentageMin not specified, visibility trigger fires as soon as the element laid out by runtime (not necessarily in viewport).

Is this a valid use case? Not sure if this metric (# of times an element get laid out) can be useful to pub, as this metric can suddenly change due to any change made in AMP runtime. For example, right now we lay out elements within 3 viewports. If we ever want to reduce this number, the metric will have a sudden drop.

On the other hand, it's confusing because I would think visiblePercentageMin defaults to 0. And I don't see it documented in the spec either.

Should we fix the behavior, or fix the doc?

@dvoytenko
Copy link
Contributor

I agree. We should default it to 0.

@lannka
Copy link
Contributor Author

lannka commented Nov 28, 2016

If everyone agrees that we should default to 0, I will make it in the VisibilityV2 impl.

I'll let @avimehta to decide if we want to fix the current impl.

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

No branches or pull requests

6 participants