Change amp-sticky-ad trigger to end of first scroll. #6597

Closed
jpettitt opened this Issue Dec 9, 2016 · 42 comments

Comments

Projects
None yet
8 participants
@jpettitt
Collaborator

jpettitt commented Dec 9, 2016

Currently "the sticky ad will display after scroll one viewport height from top provided there is at least one more viewport of content available". We'd like to see that change to trigger at the end of the first scroll regardless of length and regardless of additional available content.

Reasoning: We've been running an a/b with a 320x50 top banner and a 320x50 amp-sticky. The top banner (ugly) is generating twice the revenue, mostly due to the sticky not being seen. We'd like to have the sticky appear earlier to try and bring revenue parity with removing top banner.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Dec 9, 2016

Collaborator

to @jasti

Collaborator

zhouyx commented Dec 9, 2016

to @jasti

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Dec 11, 2016

Collaborator

I'm supportive of the suggestion to start request after scroll ends. @zhouyx do you see any issues from a performance stand point?
Regarding article length - @jpettitt Do you have many articles shorter than 2 viewports? The reason we added the requirement is to avoid the high ad density for a relatively short article, assuming the article will most probably have a banner or two.

Collaborator

jasti commented Dec 11, 2016

I'm supportive of the suggestion to start request after scroll ends. @zhouyx do you see any issues from a performance stand point?
Regarding article length - @jpettitt Do you have many articles shorter than 2 viewports? The reason we added the requirement is to avoid the high ad density for a relatively short article, assuming the article will most probably have a banner or two.

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Dec 12, 2016

Collaborator

@jasti very few. That said as I read the current doc it's 2 view ports + and additional viewpoint of content. I think length limitation in general are problematic becasue you get situations where an iphone 4 turned landscape gets an ad and a 6+ in portrait doesn't. This is the same complaint I have about amp-iframe where it has an arbitrary limitation that is impossible to know at page design time. My take - if it's enough content to scroll then show the ad at the end of the first scroll.

Collaborator

jpettitt commented Dec 12, 2016

@jasti very few. That said as I read the current doc it's 2 view ports + and additional viewpoint of content. I think length limitation in general are problematic becasue you get situations where an iphone 4 turned landscape gets an ad and a 6+ in portrait doesn't. This is the same complaint I have about amp-iframe where it has an arbitrary limitation that is impossible to know at page design time. My take - if it's enough content to scroll then show the ad at the end of the first scroll.

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Dec 13, 2016

Collaborator

That's fair. I think we should experiment unless @zhouyx has any performance concerns.
CC @lannka

Collaborator

jasti commented Dec 13, 2016

That's fair. I think we should experiment unless @zhouyx has any performance concerns.
CC @lannka

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Dec 13, 2016

Collaborator

Can't think of any performance issue. Yup we can experiment with this. May add an experiment tag for this? Or simply include the change and experiment in canary?

Collaborator

zhouyx commented Dec 13, 2016

Can't think of any performance issue. Yup we can experiment with this. May add an experiment tag for this? Or simply include the change and experiment in canary?

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Dec 14, 2016

Collaborator

Canary works for me. The sooner we get this the better.

Collaborator

jpettitt commented Dec 14, 2016

Canary works for me. The sooner we get this the better.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Dec 14, 2016

Collaborator

just checked our code. we only check if doc's height is greater than 2 viewport height, and if user is one viewport away from top. So the requesting feature is in from the beginning.

should update the doc to make it clearer.

Collaborator

zhouyx commented Dec 14, 2016

just checked our code. we only check if doc's height is greater than 2 viewport height, and if user is one viewport away from top. So the requesting feature is in from the beginning.

should update the doc to make it clearer.

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Dec 14, 2016

Collaborator

So all that needs to change is trigger on the 1st scroll regardless of length?

Collaborator

jpettitt commented Dec 14, 2016

So all that needs to change is trigger on the 1st scroll regardless of length?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Dec 15, 2016

Collaborator

Yup. Thanks @zhouyx. Let us know when this lands as an experiment.

Collaborator

jasti commented Dec 15, 2016

Yup. Thanks @zhouyx. Let us know when this lands as an experiment.

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Jan 5, 2017

Collaborator

Hey @zhouyx, checking in on the status for launching as an experiment.

Collaborator

jasti commented Jan 5, 2017

Hey @zhouyx, checking in on the status for launching as an experiment.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Jan 5, 2017

Collaborator

Ah! Sorry this has been stalled so long. I just sent out a PR for this.

Collaborator

zhouyx commented Jan 5, 2017

Ah! Sorry this has been stalled so long. I just sent out a PR for this.

@zhouyx zhouyx closed this in #6907 Jan 9, 2017

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Jan 9, 2017

Collaborator

PR is merged.

Collaborator

zhouyx commented Jan 9, 2017

PR is merged.

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Jan 10, 2017

Collaborator

@jpettitt Would you please test this in canary and let us know if you have any feedback? Thx.

Collaborator

jasti commented Jan 10, 2017

@jpettitt Would you please test this in canary and let us know if you have any feedback? Thx.

@jasti jasti reopened this Jan 10, 2017

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Jan 10, 2017

Collaborator

I'm not seeing this change in dev-channel yet.

Collaborator

jpettitt commented Jan 10, 2017

I'm not seeing this change in dev-channel yet.

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Jan 10, 2017

Collaborator

You will after this Thursday's release.

Collaborator

jasti commented Jan 10, 2017

You will after this Thursday's release.

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Jan 17, 2017

Collaborator

I'm not seeing this in the current dev channel version 1484265443309 - did it go out?

Collaborator

jpettitt commented Jan 17, 2017

I'm not seeing this in the current dev channel version 1484265443309 - did it go out?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Jan 17, 2017

Collaborator

I don't see it. @aghassemi, do you know why this didn't go out with last week's cut?

Collaborator

jasti commented Jan 17, 2017

I don't see it. @aghassemi, do you know why this didn't go out with last week's cut?

@aghassemi

This comment has been minimized.

Show comment
Hide comment
@aghassemi

aghassemi Jan 17, 2017

Member

The code is definitely in but I don't think #6907 is addressing the issue.
There is still the requirement that user needs to scroll equivalent of 1 viewport height before sticky ad shows up, that by itself enforces the fact that page needs to have 2 viewports (otherwise user can never scroll 1 viewport height) #6907 is removing the two viewpoint check, but in reality that has no effect for the reason above.

My test page: https://output.jsbin.com/muyivof works the same in prod and dev channel.

If we want sticky ad to always show up regardless of page height and still provide a good UX, maybe we can do user should scroll min(1 viewport, 50% of page height) as the restriction? @jasti @cramforce thoughts?

Member

aghassemi commented Jan 17, 2017

The code is definitely in but I don't think #6907 is addressing the issue.
There is still the requirement that user needs to scroll equivalent of 1 viewport height before sticky ad shows up, that by itself enforces the fact that page needs to have 2 viewports (otherwise user can never scroll 1 viewport height) #6907 is removing the two viewpoint check, but in reality that has no effect for the reason above.

My test page: https://output.jsbin.com/muyivof works the same in prod and dev channel.

If we want sticky ad to always show up regardless of page height and still provide a good UX, maybe we can do user should scroll min(1 viewport, 50% of page height) as the restriction? @jasti @cramforce thoughts?

@aghassemi

This comment has been minimized.

Show comment
Hide comment
@aghassemi

aghassemi Jan 17, 2017

Member

(To clarify we still want #6907, it handles the case where viewport size changes after load due to device rotation, etc.. but #6907 is not relaxing any requirements in regards to amp-sticky-ad requiring 2 viewports of content)

Member

aghassemi commented Jan 17, 2017

(To clarify we still want #6907, it handles the case where viewport size changes after load due to device rotation, etc.. but #6907 is not relaxing any requirements in regards to amp-sticky-ad requiring 2 viewports of content)

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Jan 17, 2017

Collaborator

Thanks. I think doing user should scroll min(1 viewport) as the restriction might be good enough (which implies that there is at least 1 viewport). Any reason why you suggest 50% of page heigh min?

Collaborator

jasti commented Jan 17, 2017

Thanks. I think doing user should scroll min(1 viewport) as the restriction might be good enough (which implies that there is at least 1 viewport). Any reason why you suggest 50% of page heigh min?

@aghassemi

This comment has been minimized.

Show comment
Hide comment
@aghassemi

aghassemi Jan 17, 2017

Member

"user should scroll min 1 viewport" implies "that there is at least 2 viewports of content" which is the current behaviour. If there is only 1 viewport of content, user can not scroll at all (since all content fits the screen and there is no need for scrolling).

My suggestion relaxes this 2 viewport requirement a bit but not by much. Maybe a better approach is:

  • If page <= 1vh (viewport height): no sticky ad, ever.
  • if page >= 1vh but < 1.5vh: any scroll would bring up the sticky ad.
  • if page >= 1.5vh but < 2vh: user needs to scroll 0.5vh to see the ad.
  • if page > 2vh: user needs to scroll 1vh to see the ad. (current behaviour)

This would ensure sticky ad shows up in all cases except when user can not scroll at all (which is when height <= 1vh and in these cases we don't want to show sticky ad anyway since it implies showing the ad at load time without a user interaction).

Member

aghassemi commented Jan 17, 2017

"user should scroll min 1 viewport" implies "that there is at least 2 viewports of content" which is the current behaviour. If there is only 1 viewport of content, user can not scroll at all (since all content fits the screen and there is no need for scrolling).

My suggestion relaxes this 2 viewport requirement a bit but not by much. Maybe a better approach is:

  • If page <= 1vh (viewport height): no sticky ad, ever.
  • if page >= 1vh but < 1.5vh: any scroll would bring up the sticky ad.
  • if page >= 1.5vh but < 2vh: user needs to scroll 0.5vh to see the ad.
  • if page > 2vh: user needs to scroll 1vh to see the ad. (current behaviour)

This would ensure sticky ad shows up in all cases except when user can not scroll at all (which is when height <= 1vh and in these cases we don't want to show sticky ad anyway since it implies showing the ad at load time without a user interaction).

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Jan 17, 2017

Collaborator

Can we just make it "if the user scrolls any amount the sticky ad comes up"? I was in meeting last week where Google ad sales were heavily pushing the amp-sticky. I had to rain on their parade by pointing out how badly it performs. Our a/b study of sticky vs 320x50 top banner showed a 58% revenue drop switching from top banner to sticky. A lot of that was reduced impression count hence the request to make it appear after any scroll. Study attached.

AMP Case Study Top Banner vs. Adhesion Ad.pdf

Collaborator

jpettitt commented Jan 17, 2017

Can we just make it "if the user scrolls any amount the sticky ad comes up"? I was in meeting last week where Google ad sales were heavily pushing the amp-sticky. I had to rain on their parade by pointing out how badly it performs. Our a/b study of sticky vs 320x50 top banner showed a 58% revenue drop switching from top banner to sticky. A lot of that was reduced impression count hence the request to make it appear after any scroll. Study attached.

AMP Case Study Top Banner vs. Adhesion Ad.pdf

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Jan 18, 2017

Collaborator

We'd like to see that change to trigger at the end of the first scroll regardless of length and regardless of additional available content.

Now I get it, what you mean by first scroll is there's no requirement to scroll length. @jpettitt I think I understand you wrong. thanks to @aghassemi for explaining the issue, I will leave the decision to UX.

Collaborator

zhouyx commented Jan 18, 2017

We'd like to see that change to trigger at the end of the first scroll regardless of length and regardless of additional available content.

Now I get it, what you mean by first scroll is there's no requirement to scroll length. @jpettitt I think I understand you wrong. thanks to @aghassemi for explaining the issue, I will leave the decision to UX.

@cramforce

This comment has been minimized.

Show comment
Hide comment
@cramforce

cramforce Jan 18, 2017

Member

I could get behind "user scrolled and has stopped scrolling". @jasti Do we have data about user preference based on when the ad loads?

Member

cramforce commented Jan 18, 2017

I could get behind "user scrolled and has stopped scrolling". @jasti Do we have data about user preference based on when the ad loads?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Jan 18, 2017

Collaborator

@cramforce We didn't test the reverse case, but we did test the sticky to appear as soon as possible and it ranked really high.
As long as the ad itself doesn't cause screen jank, we should definitely try this.
@aghassemi would you be able to take a stab? (Request ad after first scroll stops immaterial of page length).

Collaborator

jasti commented Jan 18, 2017

@cramforce We didn't test the reverse case, but we did test the sticky to appear as soon as possible and it ranked really high.
As long as the ad itself doesn't cause screen jank, we should definitely try this.
@aghassemi would you be able to take a stab? (Request ad after first scroll stops immaterial of page length).

@aghassemi

This comment has been minimized.

Show comment
Hide comment
@aghassemi

aghassemi Jan 18, 2017

Member

Alright, let's put it behind an experiment flag and take it from there. This is definitely a balancing act between UX and revenue. Needless to say, we care about both.

Member

aghassemi commented Jan 18, 2017

Alright, let's put it behind an experiment flag and take it from there. This is definitely a balancing act between UX and revenue. Needless to say, we care about both.

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Feb 3, 2017

Collaborator

@aghassemi - wanted to confirm if you were looking this implementation or did you want @lannka to take over?

Collaborator

jasti commented Feb 3, 2017

@aghassemi - wanted to confirm if you were looking this implementation or did you want @lannka to take over?

@aghassemi

This comment has been minimized.

Show comment
Hide comment
@aghassemi

aghassemi Feb 3, 2017

Member

@jasti I only managed to look into it but didn't get a chance to implement before leaving for vacation. @lannka @zhouyx could you take over? resources-impl already has some logic for detecting scroll stop, it would be nice to refactor that into a common helper to use here as well. One way would be runtime dispatching an amp-scrolling-stopped DOM event that we can listen to using normal event helpers like listenOnce.

Member

aghassemi commented Feb 3, 2017

@jasti I only managed to look into it but didn't get a chance to implement before leaving for vacation. @lannka @zhouyx could you take over? resources-impl already has some logic for detecting scroll stop, it would be nice to refactor that into a common helper to use here as well. One way would be runtime dispatching an amp-scrolling-stopped DOM event that we can listen to using normal event helpers like listenOnce.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Feb 9, 2017

Collaborator

will take over.

Collaborator

zhouyx commented Feb 9, 2017

will take over.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Feb 9, 2017

Collaborator

With the proposal display sticky-ad after scroll stops. It is possible that user scroll slowly while consuming content. What's our idea on that?

Collaborator

zhouyx commented Feb 9, 2017

With the proposal display sticky-ad after scroll stops. It is possible that user scroll slowly while consuming content. What's our idea on that?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Feb 9, 2017

Collaborator

As discussed, we shouldn't have any restrictions on scroll. Any user interaction with the document should trigger loading the ad.

Collaborator

jasti commented Feb 9, 2017

As discussed, we shouldn't have any restrictions on scroll. Any user interaction with the document should trigger loading the ad.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Feb 9, 2017

Collaborator

Thanks @jasti

As we went back and forth on deciding when to display sticky-ad. I want to explain our discussion conclusions more here. The discussion is based on that we already take care of resources scheduling, and achieving good UX is the only concern.

The bad UX is we display sticky-ad to user before then even start to consume a page. (that's mostly like when users are swiping between amp viewer carousel)

As long as user start to consume an article, it is OK to display the ad to them as soon as possible (it's also best to revenue). Display before scrolling stops is also acceptable as long as it doesn't cause screen jank (we will still need to wait for scroll to slows down). This requires us to detect first user interaction with the document to trigger sticky ad loading. The interaction most likely is viewport scrolling, but maybe we should also consider user clicking contents in first viewport.

As we agreed, will put an experiment flag to compare current and proposed approach.

Collaborator

zhouyx commented Feb 9, 2017

Thanks @jasti

As we went back and forth on deciding when to display sticky-ad. I want to explain our discussion conclusions more here. The discussion is based on that we already take care of resources scheduling, and achieving good UX is the only concern.

The bad UX is we display sticky-ad to user before then even start to consume a page. (that's mostly like when users are swiping between amp viewer carousel)

As long as user start to consume an article, it is OK to display the ad to them as soon as possible (it's also best to revenue). Display before scrolling stops is also acceptable as long as it doesn't cause screen jank (we will still need to wait for scroll to slows down). This requires us to detect first user interaction with the document to trigger sticky ad loading. The interaction most likely is viewport scrolling, but maybe we should also consider user clicking contents in first viewport.

As we agreed, will put an experiment flag to compare current and proposed approach.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Feb 10, 2017

Collaborator

update on this issue:

This requires us to detect first user interaction with the document to trigger sticky ad loading. The interaction most likely is viewport scrolling, but maybe we should also consider user clicking contents in first viewport.

After discussion, we agree in most cases viewport scrolling will be the first user interaction. And we decide to only listen to that to balance between display rate and performance.
PR: #7478

Collaborator

zhouyx commented Feb 10, 2017

update on this issue:

This requires us to detect first user interaction with the document to trigger sticky ad loading. The interaction most likely is viewport scrolling, but maybe we should also consider user clicking contents in first viewport.

After discussion, we agree in most cases viewport scrolling will be the first user interaction. And we decide to only listen to that to balance between display rate and performance.
PR: #7478

@lannka lannka modified the milestones: Sprint H2 Feb, Pending Triage Feb 11, 2017

@jasti jasti added this to Sprint Candidate in AMP Advertising Feb 11, 2017

@jasti jasti moved this from Sprint Candidate to Backlog in AMP Advertising Feb 11, 2017

@jasti jasti moved this from Backlog to Sprint Candidate in AMP Advertising Feb 11, 2017

@lannka lannka moved this from Sprint Candidate to In Progress in AMP Advertising Feb 11, 2017

@zhouyx zhouyx closed this in #7478 Feb 15, 2017

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Feb 15, 2017

Collaborator

The PR is in. I am using an experiment flag sticky-ad-early-load. How do we test with the new behavior. Should I enable this feature in 1% canary, aka set the flag to true for all canary version.

Collaborator

zhouyx commented Feb 15, 2017

The PR is in. I am using an experiment flag sticky-ad-early-load. How do we test with the new behavior. Should I enable this feature in 1% canary, aka set the flag to true for all canary version.

@lannka

This comment has been minimized.

Show comment
Hide comment
@lannka

lannka Feb 15, 2017

Collaborator

Yep, we can do 1% canary (to catch any errors/bugs).
Meantime, everyone can opt-in and play with the new experience and give feedback.

Collaborator

lannka commented Feb 15, 2017

Yep, we can do 1% canary (to catch any errors/bugs).
Meantime, everyone can opt-in and play with the new experience and give feedback.

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Feb 15, 2017

Collaborator

FYI: Already turned on the experiment flag in canary #7586
Should be able to see in next week's 1% canary

Collaborator

zhouyx commented Feb 15, 2017

FYI: Already turned on the experiment flag in canary #7586
Should be able to see in next week's 1% canary

@jasti jasti removed this from In Progress in AMP Advertising Feb 22, 2017

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Mar 13, 2017

Collaborator

@jasti This has been in 1% canary for two weeks. Should we go ahead and push the change to prod?

Collaborator

zhouyx commented Mar 13, 2017

@jasti This has been in 1% canary for two weeks. Should we go ahead and push the change to prod?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Mar 13, 2017

Collaborator

@jpettitt have you had a chance to measure revenue impact for this?

Collaborator

jasti commented Mar 13, 2017

@jpettitt have you had a chance to measure revenue impact for this?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Mar 20, 2017

Collaborator

Ping @jpettitt.

Collaborator

jasti commented Mar 20, 2017

Ping @jpettitt.

@jasti jasti reopened this Mar 20, 2017

@jpettitt

This comment has been minimized.

Show comment
Hide comment
@jpettitt

jpettitt Mar 21, 2017

Collaborator

@jasti hard to measure when it's in canary. Given the date it goes to prod I'll be able to give you before/after data.

Collaborator

jpettitt commented Mar 21, 2017

@jasti hard to measure when it's in canary. Given the date it goes to prod I'll be able to give you before/after data.

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Mar 21, 2017

Collaborator

Thanks. @zhouyx would you be able to push it to prod in the upcoming release?

Collaborator

jasti commented Mar 21, 2017

Thanks. @zhouyx would you be able to push it to prod in the upcoming release?

@jasti

This comment has been minimized.

Show comment
Hide comment
@jasti

jasti Mar 29, 2017

Collaborator

This has launched.

Collaborator

jasti commented Mar 29, 2017

This has launched.

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