fix: aps integration with regards to size mismatch #140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
August 13, 2024 10:12
…size mapping is respected. Also place check on display call for GAM to prevent calling display on ads that are not rendered in the DOM
Trav84
requested review from
masonbraun and
thedaviddias
and removed request for
a team
caffeinated-pixels
approved these changes
Aug 28, 2024
caffeinated-pixels
changed the title
|
@Trav84 The changes have broken two of the unit tests. Are you able to fix? |
|
Hey @caffeinated-pixels I pushed a fix. Sorry about that! There was a missing reference to "this" in the call to get the apsSlotType which was erroring out. Resolved and tested locally and also ran tests locally and they seem happy. |
thedaviddias
pushed a commit
that referenced
this pull request
Aug 28, 2024
## [4.2.14](v4.2.13...v4.2.14) (2024-08-28) ### Bug Fixes * aps integration with regards to size mismatch ([#140](#140)) ([0bb7284](0bb7284))
|
🎉 This PR is included in version 4.2.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The current APS integration does not work correctly if using the GAM size mapping feature. This is because while GAM accepts the size mapping feature, and will request the designated sizes at the designated breakpoints, the current APS integration simply uses the size array of the unit, which includes all potential sizes of the unit.
You can see this where the apsSlot is defined in the defineSlots() function.
This can result in a request for a size that the unit is not currently set up to display. For example a leaderboard unit may be a 320x100 on mobile, a 728x90 on tablet, and a 970x250 on desktop. All of the sizes exist in the "size" array of the unit. That unit may also contain a GAM size mapping definition that states that the 320x100 can only be requested on mobile, 728x90 on tablet, and a 970x250 on desktop.
The APS call, however, will simply include all 3 of those sizes for every request. This can cause breakage on the site as the creative delivered will likely be larger than the size of the ad unit / placeholder. This will also hurt viewability metrics (a 728x90 on mobile won't be fully viewable, etc).
Amazon has a newer method of integration called "simplerGPT" which does utilize GAM size mapping definitions. In fact you do not need to define any slot for APS, it will just use the GPT definition.
You can read about this method here.
I added a simple check for the presence of "simplerGPT" in the config that if present, will utilize the gpt slot definition for the APS calls, instead of the APS unit that is defined. I've defaulted to the older method to avoid any breakage.
Testing this I'm able to verify that APS is being called for the correct sizes at the configured breakpoints, versus the older method which requests all listed sizes.