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

Add 300x50 ad size to mobile-sticky #1403

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

deedeeh
Copy link
Contributor

@deedeeh deedeeh commented Jun 4, 2024

What does this change?

This enables a new ad size 300x50 to mobile-sticky.

I tested in CODE and using a VPN to be in a region that has a mobile-sticky slot. Here is a screenshot from googletag.openConsole() with the new size.

image

Why?

This request was made by AdOps which will give us the opportunity to serve creatives in smaller size.

Copy link

changeset-bot bot commented Jun 4, 2024

🦋 Changeset detected

Latest commit: a42f821

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@guardian/commercial Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jun 4, 2024

Ad load time test results

For consented, top-above-nav took on average 4586.85ms to load.
For consentless, top-above-nav took on average 2797.9ms to load.

Test conditions:

  • 5mbps download speed
  • 1.5mbps upload speed
  • 150ms latency

@deedeeh deedeeh added the [beta] @guardian/commercial Add this label to publish an @guardian/commercial beta npm release from a PR label Jun 4, 2024
Copy link
Contributor

github-actions bot commented Jun 4, 2024

🚀 0.0.0-beta-20240604172725 published to npm as a beta release

@deedeeh deedeeh marked this pull request as ready for review June 5, 2024 09:27
@deedeeh deedeeh requested a review from a team as a code owner June 5, 2024 09:27
@@ -389,7 +389,7 @@ const slotSizeMappings = {
mobile: [adSizes.fluid],
},
'mobile-sticky': {
mobile: [adSizes.mobilesticky, adSizes.empty],
mobile: [adSizes.mobilesticky, adSizes.empty, createAdSize(300, 50)],
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking good 👏

@deedeeh deedeeh merged commit 244b7aa into main Jun 6, 2024
13 checks passed
@deedeeh deedeeh deleted the dina/add-300x50-ad-size-to-mobile-sticky branch June 6, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[beta] @guardian/commercial Add this label to publish an @guardian/commercial beta npm release from a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants