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-ad and amp-analytics to be rendered in amp-next-page articles. #15807

Open
sarabkohli opened this issue Jun 4, 2018 · 19 comments

Comments

@sarabkohli
Copy link

commented Jun 4, 2018

What's the issue?

We wanted to give our users an infinite scroll type experience and amp-next-page component is exactly what we were looking for. When we implemented it, we saw that the posts that we intended to include were not able to show amp-ad content or amp-analytics content. On the other hand, amp-pixel worked perfectly. So we wanted to request for allowing amp-analytics and amp-ad components to be allowed to be rendered in amp-next-page components.

How do we reproduce the issue?

If this is a bug please provide a public URL and ideally a reduced test case (e.g. on jsbin.com) that exhibits only your issue and nothing else. Then provide step-by-step instructions for reproducing the issue:

  1. Go to https://www.popsugar.com/fitness/How-Eat-Lose-Weight-44625088/amp
  2. Keep an eye out for the amp/amp-next-content GET request as you reach closer the bottom of the post.
  3. Once the post loads, you can inspect the post and find none of the amp-analytics load. Also the amp-iframe for 'bluekai' does not fire either.

Here is screenshot of the components loading in the main article:

original post

Here is the amp-next-page framed article that loads almost the same template but the amp-analytics content is not rendered.
amo-next-page-iframe template on the same post

What browsers are affected?

All browsers

Which AMP version is affected?

This is like bug, but since the amp-next-page component is in experimental stage, this could be considered as a feature request.

amp-next-page: https://github.com/ampproject/amphtml/tree/master/extensions/amp-next-page

@aghassemi

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2018

@aghassemi aghassemi modified the milestones: Backlog Bugs, Pending Triage Jun 4, 2018
@mpatnode

This comment has been minimized.

Copy link

commented Jun 4, 2018

Looks like @peterjosling disabled this intentionally: https://github.com/ampproject/amphtml/blob/master/extensions/amp-next-page/0.1/next-page-service.js#L186
That seriously hinders the value for publishers who want to count page views & visits. Not sure if there's another mechanism we should be using.

@emarchiori

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2018

Thanks for trying out this feature, this feedback is very valuable.

amp-ads not loading is not intentional, we will look into that. Do you have any more information about this?

analytics in the loaded documents is disabled per design, as agreed with amp-analytics, as the configuration of the original document still applies inside loaded documents. Two things about this:

  • There is a new "amp-next-page-scroll" that can be used to log when the user scrolls to the next page.
  • Improvements in next versions will have a way for analytics configurations to differentiate between events in the original doc vs loaded docs, but this is not yet implemented.
@mpatnode

This comment has been minimized.

Copy link

commented Jun 20, 2018

Fair enough. We have fallen back to amp-pixels for tracking the next page for now. One thing to keep in mind is the analytics config needs to be flexible enough to handle dynamic next page selection. So as long as the content of the next page can include the new analytics config, that's fine, though that seems like a complicated ask when you have 3 or 4 different analytics vendors on a page. All of them will need to be dynamically updated. We'll take a look at amp-next-page-scroll to see if that will help.

@emarchiori

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

That is good feedback. We are planning on interating a bit on the analytics behavior.

What about amp-ads? Because in your example the second page does not have amp-ad tags, so I cannot test it, and it was working in other examples. Do you have a page were I can test this?

@mpatnode

This comment has been minimized.

Copy link

commented Jun 21, 2018

Ads appear to be working now. Start here: https://www.popsugar.com/travel/Bermuda-Travel-Planning-2018-44817394/amp and scroll down to the next-page.

@mpatnode

This comment has been minimized.

@aghassemi

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

@mpatnode Thanks for verifying.

@aghassemi

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

Closing issue as fixed.

@aghassemi aghassemi closed this Jun 21, 2018
@aghassemi aghassemi reopened this Jun 21, 2018
@aghassemi

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

actually reopening re: Analytics part :D sorry

@rudygalfi rudygalfi added this to Needs triage in Analytics via automation Jul 23, 2018
@ericlindley-g

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2018

cc/ @jeffjose

@prash-mi

This comment has been minimized.

Copy link

commented Aug 8, 2018

@erwinmombay erwinmombay modified the milestones: Pending Triage, New FRs Aug 21, 2018
@tchirag

This comment has been minimized.

Copy link

commented Sep 13, 2018

Have been following this thread for a while. Just noticed since yesterday page view analytics code has started to fire on amp-next-page. Is that working for others as well?

@mpatnode

This comment has been minimized.

Copy link

commented Sep 13, 2018

Really? What are the semantics? Is it considered a whole new page? (that would be our preference). We took them all out and used pixels instead.

@peterjosling

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2018

This was unintentional - we still have work to do to work out what the semantics should be and to make sure we don't have duplicate triggers firing for scrolls etc.

The previous functionality will be restored by #18026

@tchirag

This comment has been minimized.

Copy link

commented Oct 11, 2018

Really? What are the semantics? Is it considered a whole new page? (that would be our preference). We took them all out and used pixels instead.

Are you able to get the events using pixels? Unfortunately for us, it doesnt get triggered. Also, as mentioned above, GA triggers have stopped working for now.

@mpatnode

This comment has been minimized.

Copy link

commented Oct 11, 2018

Well that was a terrifying moment. The pixels still work fine for us. Feel free to test this link: https://www.google.com/amp/s/www.popsugar.com/celebrity/Jennifer-Garner-Dating-Again-After-Divorce-From-Ben-Affleck-45362924/amp

@nandosangenetto

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2019

@mpatnode how you did to handle with the pixel? I was trying to implement the pixel as a workaround, but I couldn't see new pageviews.

@mpatnode

This comment has been minimized.

Copy link

commented Jul 24, 2019

Nothing too magical. I'm still seeing the pixels fire on the page above, even when served from the CDN. Note that the Chrome Debug tools network search seems a little buggy in that they didn't always show up when I searched by hostname:

Request URL: https://ts.popsugar.com/b/ss/popsugarglobalprod/1/H.26--NS/?&gt=amp&ev=event1%2Cevent2&ns=shopstyle&pageName=savvy%3Aus%3Aarticle%3Asummer-snacks-outdoor-activities-46315084&ch=content&v1=savvy%3Aus%3Amobile&v3=article%3Avhub%3Apss&v5=Unknown&v7=native_boost&v9=46315084&v12=2019-07-23&v98=https%3A%2F%2Fwww.popsugar.com%2Fsmart-living%2FSummer-Snacks-Outdoor-Activities-46315084&v41=46315084&c13=amp&c56=%22Summer%20Snacks%20For%20Outdoor%20Activities%20%7C%20POPSUGAR%20Smart%20Living%22&l1=summer%2Chealthy%20snacks%2Cshoppable%2Cstarkist%2Cnative%20content%2Csnacks%2Cnative%20content
Request Method: GET

I do see we don't fire a GA pixel there, but we do Adobe, Comscore and our local counter. We should probably add GA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Analytics
  
Needs triage
UI - Projects
Awaiting triage
UI
  
Awaiting triage
You can’t perform that action at this time.