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

Calculate actual ad views #4885

Merged
merged 2 commits into from Dec 5, 2018
Merged

Calculate actual ad views #4885

merged 2 commits into from Dec 5, 2018

Conversation

@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Nov 9, 2018

Sends a request to the advertising backend when an ad becomes visible in the viewport.

Uses a third-party library verge which has a (more robust than I could write) inViewport function.

@davidfischer davidfischer requested a review from ericholscher Nov 9, 2018
@codecov
Copy link

@codecov codecov bot commented Nov 9, 2018

Codecov Report

Merging #4885 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #4885   +/-   ##
======================================
  Coverage    76.6%   76.6%           
======================================
  Files         158     158           
  Lines       10043   10043           
  Branches     1268    1268           
======================================
  Hits         7693    7693           
  Misses       2008    2008           
  Partials      342     342

@@ -149,6 +149,7 @@ function Promo(data) {
this.div_id = data.div_id || '';
this.html = data.html || '';
this.display_type = data.display_type || '';
this.view_tracking_url = data.view_url;
Copy link
Member

@humitos humitos Nov 12, 2018

Are we already sending this field in our data? Just asking because I don't see any Python code modified in this PR.

Copy link
Contributor Author

@davidfischer davidfischer Nov 12, 2018

You need to look at the related -ext PR readthedocs/readthedocs-ext#200

Copy link
Member

@ericholscher ericholscher left a comment

Simple enough. I don't really understand where the rtdinview event name is defined though.

@davidfischer
Copy link
Contributor Author

@davidfischer davidfischer commented Nov 15, 2018

rtdinview is a jQuery event namespace. These make it easier to get all the events in a namespace.

@davidfischer davidfischer merged commit 5636eb3 into master Dec 5, 2018
3 checks passed
@davidfischer davidfischer deleted the davidfischer/view-tracking-url branch Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants