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

Rotate ads on a visibilitychange event #207

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Aug 7, 2024

This allows the ad placement to rotate when the tab regains focus after losing it subject to the normal ad rotation conditions:

  • Ad must have been visible (tab focuses, ad in viewport) for 30+ seconds
  • We only rotate a maximum of 2 additional times per page view (initial ad impression +2 rotations). The total rotations is shared between both hashchange and visibilitychange events.

When a tab loses focus, we send the view time immediately. If that time is less than 30s, then the ad will not immediately rotate when the tab comes back into focus. However, the view time will begin counting again (although it will not send the view time again). If the tab loses/regains focus again and the view time is then 30+, the ad will rotate.

This PR also cleaned up the event listeners a little bit and was a little more defensive in terms of clearing listeners automatically during ad rotation.

@davidfischer davidfischer requested review from a team and ericholscher August 7, 2024 00:05
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@davidfischer davidfischer merged commit 353675a into main Aug 15, 2024
4 checks passed
@davidfischer davidfischer deleted the davidfischer/visibility-change-rotation branch August 15, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants