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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃摉story-ads: update docs #19308

Merged
merged 4 commits into from Nov 16, 2018
Merged

馃摉story-ads: update docs #19308

merged 4 commits into from Nov 16, 2018

Conversation

calebcordry
Copy link
Member

Trying to get this in before launch. Better docs cleanup coming this week.

Copy link
Contributor

@torch2424 torch2424 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 馃槃

</template>
</amp-story-auto-ads>
```html
<amp-story-page next-page-no-ad id="page-2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing >

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok really done :)

</template>
## Insertion Control
The AMP runtime will determine the best place to insert an ad using our own heuristics.
If there is a specific position in a story that you wish to never show an ad, you
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this is worded weird (at least to me). Can we do something like:

If there is a specific position in a story that you wish to never show an ad, you can add the next-page-no-ad attribute to the <amp-story-page>. The insertion algorithm will then ignore the page following these <amp-story-page> elements as possible places to place an ad.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that sounds too redundant, feel free to just ignore this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made changes.

</template>
## Insertion Control
If there is a specific position in a story that you wish to never show an ad,
you can add the `next-page-no-ad` attribute an `<amp-story-page>`. The insertion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃敟

@calebcordry calebcordry merged commit f3d8d40 into ampproject:master Nov 16, 2018
@calebcordry calebcordry deleted the story-no-ad branch November 16, 2018 00:06
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants