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

Allow amp-analytics visibilitySpec to tracking elements other than amp-* #4449

Closed
rudygalfi opened this issue Aug 10, 2016 · 15 comments
Closed

Comments

@rudygalfi
Copy link
Contributor

rudygalfi commented Aug 10, 2016

The current amp-analytics visibilitySpec implementation requires elements to be AMP custom elements like amp-img or amp-youtube. For use cases like tracking a feature block on the page (e.g. related articles section), this is insufficient and may inspire hacky beahviors like (mis)using amp elements as visibility trackers.

The proposal to figure out some way to extend the visibilitySpec to non-amp-* elements while keeping perf sane.

cc @avimehta

(This was discussed in #1297.)

@avimehta
Copy link
Contributor

iirc, we decided that using a dummy element at the end of featured article is okay. Lets keep this open though to collect more usecases.

@senthilp
Copy link

We have this use case in eBay AMP pages. We have lot of modules (Top Selling, Top Deals, Sponsored Listings, Guides, Collections etc. ) in the page and most of them are below the fold. Currently they are all HTML <section> elements, which have AMP components amp-img, amp-carousel etc. inside them. In the non-AMP pages we track each module visibility and report them to our tracking system. Our backend systems analyze the visibility tracking data and optimize the module ordering for future visits. It is a constant feedback loop that makes the backend system smarter.

We want a similar tracking mechanism for AMP pages too. It is possible currently but restricted to AMP custom elements. We do not want to replace the <section> elements with some custom AMP element just for this use case. As @avimehta mentioned should we just add a dummy element like

<amp-embed id="topdeals" width=0 height=0 layout="nodisplay"></amp-embed>

at the end of each module and pass this selector #topdeals to the visibility spec? Even here how will the visiblePercentageMin and visiblePercentageMax work as this element has 0 height?.

@dvoytenko
Copy link
Contributor

@senthilp No, we certainly do not want to create fake elements for this purpose and quite contrary we'd really like to expand all intersection observer polyfills to non-AMP elements. It'd take a bit of doing, however. I'll confirm the plans to support this and will chime back in here.

/cc @jridgewell @aghassemi - this is very similar to the conversation we had about measurement/layout layers and extending support to non-AMP elements.

@dvoytenko
Copy link
Contributor

/cc @jridgewell, @cramforce

As we are proceeding with underlying tech implementation, it'd be good to confirm visibility vs state question. Should we report an element's visibility regardless its "loaded" state? Consider this example:

<section id="section1">
  <amp-ad id="ad1"></amp-ad>
</section>

And let's assume we added 50% visibility reporting to both section1 and ad1 (both elements are essentially equivalent). However, the big difference in what we know about these elements. On a surface, reporting a 50% visibility event for ad1 element before it has been loaded is a bit misleading. In the case with section1, we don't have any other information about "loading" state of this element.

So, what do we believe are the right rules here?

@rudygalfi
Copy link
Contributor Author

I think it's wrong to claim visibility when it's still not loaded. I would, however, entertain the idea that the loading of any subcomponent of an element that's being tracked for visibility is sufficient to say it's visible. This is also true at the page level with the visible trigger: Some part of the page has to be loaded, but that doesn't imply that every ad has been loaded.

@dvoytenko
Copy link
Contributor

@rudygalfi And what about the example I give? Are we ok with the possibility that, while section1 and ad1 layouts are one and the same (same position, same size), the visibility even will trigger on section1 immediately and ad1 will wait for some form of "loaded" signal?

@jridgewell
Copy link
Contributor

Should we be waiting for all children to "load" before reporting visibility on section1?

@dvoytenko
Copy link
Contributor

@jridgewell - yes, this is certainly one possible interpretation as well.

@rudygalfi
Copy link
Contributor Author

Can we enumerate the elements that we could give a loading signal for? Presumably it includes amp-img, amp-ad, amp-iframe. Are there any non AMP elements this applies to?

I think it could make sense to call out a certain set of elements that have a loaded requirement before they will report visibility. (Usually visibility analytics hinges on the object actually being visible; otherwise it's worthless.) We could then document which elements this is true for and the rest are essentially based on layout visibility. If you choose to track on a "weak visibility" tag like section in the example above, then the trigger fires as soon as it's positionally visible regardless of the state of its children. On the other hand, elements with "strong visibility" like amp-ad in the example wait to be loaded before triggering.

@dvoytenko
Copy link
Contributor

@rudygalfi Your interpretation, I think, makes the most sense. Generally, each AMP element produces its own loaded signal. amp-img, amp-iframe, amp-youtube, etc - all depend on a fairly comprehensive loading; amp-ad signals as soon as it's rendered; amp-video - as soon as playback can be started. Composite elements, such as amp-carousel, signal almost immediately without waiting for children.

@senthilp
Copy link

senthilp commented Nov 1, 2016

@dvoytenko Does this mean in the visibilitySpec config we would have a flag like week, which indicates that the trigger should happen as soon as the selector is visible, irrespective of the loaded signal generated by its children which may be AMP elements (amp-img, amp-iframe, amp-youtube, etc).

@lannka
Copy link
Contributor

lannka commented Nov 1, 2016

Does it make sense for those elements that have loaded signal todisplay=none before the signal?

@dvoytenko
Copy link
Contributor

@senthilp I think weak property would make sense.

@lannka Why display:none?

@lannka
Copy link
Contributor

lannka commented Nov 1, 2016

@dvoytenko ah, never mind. I think we're already doing this, hiding unloaded resources and showing a placeholder, which doesn't solve the problem.

@zhouyx
Copy link
Contributor

zhouyx commented Jun 21, 2019

Duplicate of #20609

@zhouyx zhouyx marked this as a duplicate of #20609 Jun 21, 2019
@zhouyx zhouyx closed this as completed Jun 21, 2019
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

8 participants